FrankChen021 commented on code in PR #19397:
URL: https://github.com/apache/druid/pull/19397#discussion_r3181976806
##########
processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java:
##########
@@ -494,21 +534,38 @@ public Sequence<ResultRow> process(
if (intervals.size() != 1) {
throw new IAE("Should only have one interval, got[%s]", intervals);
}
+ }
- final ResourceHolder<ByteBuffer> bufferHolder = bufferPool.take();
+ private Sequence<ResultRow> processWithCursorHolder(
+ GroupByQuery query,
+ CursorFactory cursorFactory,
+ CursorHolder cursorHolder,
+ @Nullable TimeBoundaryInspector timeBoundaryInspector,
+ NonBlockingPool<ByteBuffer> bufferPool,
+ CursorBuildSpec buildSpec
+ )
+ {
+ final GroupByQueryConfig querySpecificConfig =
configSupplier.get().withOverrides(query);
Review Comment:
P2 Close cursorHolder if query config override throws
processWithCursorHolder now receives an already-created CursorHolder, but
computes querySpecificConfig before entering any cleanup-protected block. If
withOverrides throws, for example due to an invalid groupBy query context
value, the CursorHolder returned by makeCursorHolderAsync/makeCursorHolder is
never closed. Move this config resolution before acquiring the holder, or wrap
it so cursorHolder is closed on every pre-sequence failure.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
