[GitHub] [druid] adarshsanjeev commented on a diff in pull request #14117: Fix memory calculations for WorkerMemoryParameters for machines with relatively less heap space

["adarshsanjeev (via GitHub)" <gi...@apache.org>]

adarshsanjeev commented on code in PR #14117:
URL: https://github.com/apache/druid/pull/14117#discussion_r1181389556


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/WorkerMemoryParameters.java:
##########
@@ -309,8 +316,12 @@ public static WorkerMemoryParameters createInstance(
         )
     );
 
-    // Apportion max frames to all processors equally, then subtract one to account for an output frame.
-    final int superSorterMaxChannelsPerProcessor = maxNumFramesForSuperSorter / superSorterMaxActiveProcessors - 1;
+    final int isSmallWorker = usableMemoryInJvm < SMALL_WORKER_CAPACITY_THRESHOLD_BYTES ? 1 : 0;
+    // Apportion max frames to all processors equally, then subtract one to account for an output frame and one to account
+    // for the durable storage's output frame in the supersorter

Review Comment:
   Does this sound better? I think the comment is a bit confusing as in, it doesn't explain why these two frames does not need to be reserved for large workers, as they would use them too. We should add that to the comment as well.
   
   ```suggestion
       // for the durable storage's output frame in the supersorter. The durable storage's output frame is only considered if the worker is small to be more conservative with memory requirements.
   ```



-- 
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