LakshSingla commented on code in PR #15420:
URL: https://github.com/apache/druid/pull/15420#discussion_r1408780032
##########
processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java:
##########
@@ -127,17 +123,32 @@ public GroupingEngine(
*
* @return broker resource
*/
- public GroupByQueryResources prepareResource(GroupByQuery query)
+ public static GroupByQueryResources prepareResource(
+ GroupByQuery query,
+ BlockingPool<ByteBuffer> mergeBufferPool,
+ boolean usesGroupByMergingQueryRunner,
+ GroupByQueryConfig groupByQueryConfig
+ )
{
- final int requiredMergeBufferNum =
GroupByQueryResources.countRequiredMergeBufferNum(query);
+
+ final int requiredMergeBufferNumForToolchestMerge =
+
GroupByQueryResources.countRequiredMergeBufferNumForToolchestMerge(query);
+
+ final int requiredMergeBufferNumForMergingQueryRunner =
+ usesGroupByMergingQueryRunner
+ ?
GroupByQueryResources.countRequiredMergeBufferNumForMergingQueryRunner(groupByQueryConfig,
query)
+ : 0;
Review Comment:
Yes it is pretty bad. What's also unsettling is that we now wanna make sure
that GroupByQueryMergingRunnerV2 expects the merge buffer from someone above in
the chain, which enforces a contract b/w two different query runners, with only
a generic responseContext that holds them.
Merge buffers do need a redesign, and I hope the changes introduced in the
patch become obsolete.
--
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-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
