clintropolis commented on code in PR #18148:
URL: https://github.com/apache/druid/pull/18148#discussion_r2345351989
##########
processing/src/main/java/org/apache/druid/query/QueryContexts.java:
##########
@@ -44,6 +44,7 @@ public class QueryContexts
public static final String PRIORITY_KEY = "priority";
public static final String LANE_KEY = "lane";
public static final String TIMEOUT_KEY = "timeout";
+ public static final String PER_SEGMENT_TIMEOUT_KEY = "segmentTimeout";
Review Comment:
i wonder if the naming of this property should include something about the
processing pool since it only applies to queries that run on the pool, not sure
what a great name would be though... naming is the worst, and this is probably
fine too
##########
processing/src/main/java/org/apache/druid/query/ForwardingQueryProcessingPool.java:
##########
@@ -33,10 +38,19 @@
public class ForwardingQueryProcessingPool extends
ForwardingListeningExecutorService implements QueryProcessingPool
{
private final ListeningExecutorService delegate;
+ private final ScheduledExecutorService timeoutService;
+ public ForwardingQueryProcessingPool(ExecutorService executorService,
ScheduledExecutorService timeoutService)
Review Comment:
maybe consider doing this in a bit more of an 'opt-in' manner where we allow
`timeoutService` to be nullable. Like the processing module would check for 0
threads in `numTimeoutThreads`, and if 0 pass null into this, and here we could
`log.warn` or something if `submitRunnerTask ` while `timeoutService` is null
as a way to disable this functionality.
--
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]