You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/10/28 08:03:22 UTC
[GitHub] [druid] gianm opened a new pull request, #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
gianm opened a new pull request, #13274:
URL: https://github.com/apache/druid/pull/13274
This consideration is important, because otherwise we can run out of memory due to large statistics-tracking objects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
cryptoe commented on PR #13274:
URL: https://github.com/apache/druid/pull/13274#issuecomment-1302128796
We might have to update this line `Each sketch uses at most approximately 300 MB` in `[docs/multi-stage-query/concepts.md]`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a diff in pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
gianm commented on code in PR #13274:
URL: https://github.com/apache/druid/pull/13274#discussion_r1013008436
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/WorkerMemoryParameters.java:
##########
@@ -282,13 +283,19 @@ private static long memoryPerBundle(
)
{
final int bundleCount = numWorkersInJvm + numProcessingThreadsInJvm;
- return (long) (maxMemoryInJvm * USABLE_MEMORY_FRACTION) / bundleCount;
+
+ // Need to subtract memoryForStatisticsTracking off the top, since this is reserved for statistics collection.
+ final long memoryForStatisticsTracking = (long) numWorkersInJvm * StageDefinition.PARTITION_STATS_MAX_BYTES;
+ final long memoryForBundles = (long) (maxMemoryInJvm * USABLE_MEMORY_FRACTION - memoryForStatisticsTracking);
Review Comment:
There's a reasonable error that occurs later on; see WorkerMemoryParametersTest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13274:
URL: https://github.com/apache/druid/pull/13274#discussion_r1012907059
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/WorkerMemoryParameters.java:
##########
@@ -272,23 +347,78 @@ static int computeMaxWorkers(
final long bundleMemory = memoryPerBundle(maxMemoryInJvm, numWorkersInJvm, numProcessingThreadsInJvm);
// Inverse of memoryNeededForInputChannels.
- return Ints.checkedCast((bundleMemory - PROCESSING_MINIMUM_BYTES) / STANDARD_FRAME_SIZE - 1);
+ return Math.max(0, Ints.checkedCast((bundleMemory - PROCESSING_MINIMUM_BYTES) / STANDARD_FRAME_SIZE - 1));
}
+ /**
+ * Maximum number of workers that may exist in the current JVM.
+ */
+ private static int computeNumWorkersInJvm(final Injector injector)
+ {
+ final AppenderatorsManager appenderatorsManager = injector.getInstance(AppenderatorsManager.class);
+
+ if (appenderatorsManager instanceof UnifiedIndexerAppenderatorsManager) {
+ // CliIndexer
+ return injector.getInstance(WorkerConfig.class).getCapacity();
+ } else {
+ // CliPeon
+ return 1;
+ }
+ }
+
+ /**
+ * Maximum number of concurrent processors that exist in the current JVM.
+ */
+ private static int computeNumProcessorsInJvm(final Injector injector)
+ {
+ return injector.getInstance(Bouncer.class).getMaxCount();
+ }
+
+ /**
+ * Compute the memory allocated to each worker. Includes anything that exists outside of processing bundles.
+ *
+ * Today, we only look at one thing: the amount of memory taken up by
+ * {@link org.apache.druid.msq.statistics.ClusterByStatisticsCollector}. This is the single largest source of memory
+ * usage outside processing bundles.
+ */
+ private static long memoryPerWorker(
+ final long maxMemoryInJvm,
+ final int numWorkersInJvm
+ )
+ {
+ final long usableMemory = (long) (maxMemoryInJvm * USABLE_MEMORY_FRACTION);
+ final long memoryForWorkers = (long) Math.min(
Review Comment:
Nice. This is really cool and will go a long way in fixing the memory issues of sketches.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/kernel/controller/ControllerStageTracker.java:
##########
@@ -216,6 +219,10 @@ void start()
*/
void finish()
{
+ if (resultKeyStatisticsCollector != null) {
+ resultKeyStatisticsCollector.clear();
Review Comment:
Nice catch !!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
gianm commented on PR #13274:
URL: https://github.com/apache/druid/pull/13274#issuecomment-1302391887
Pushed an update fixing merge conflicts, updating `docs/multi-stage-query/concepts.md`, and adjusting logic so the accounting for lookups happens on both controller and worker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe merged pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
cryptoe merged PR #13274:
URL: https://github.com/apache/druid/pull/13274
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13274:
URL: https://github.com/apache/druid/pull/13274#discussion_r1008101118
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/WorkerMemoryParameters.java:
##########
@@ -282,13 +283,19 @@ private static long memoryPerBundle(
)
{
final int bundleCount = numWorkersInJvm + numProcessingThreadsInJvm;
- return (long) (maxMemoryInJvm * USABLE_MEMORY_FRACTION) / bundleCount;
+
+ // Need to subtract memoryForStatisticsTracking off the top, since this is reserved for statistics collection.
+ final long memoryForStatisticsTracking = (long) numWorkersInJvm * StageDefinition.PARTITION_STATS_MAX_BYTES;
+ final long memoryForBundles = (long) (maxMemoryInJvm * USABLE_MEMORY_FRACTION - memoryForStatisticsTracking);
Review Comment:
Should we add a > 0 check here? That way the user knows that he needs to give more memory to the task
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on pull request #13274: MSQ: Consider PARTITION_STATS_MAX_BYTES in WorkerMemoryParameters.
Posted by GitBox <gi...@apache.org>.
cryptoe commented on PR #13274:
URL: https://github.com/apache/druid/pull/13274#issuecomment-1305285436
🚀
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org