jtuglu1 commented on code in PR #19235:
URL: https://github.com/apache/druid/pull/19235#discussion_r3019663945


##########
processing/src/main/java/org/apache/druid/query/ChainedExecutionQueryRunner.java:
##########
@@ -153,8 +167,32 @@ public Iterable<T> call()
                     )
                 );
 
+            final int totalSegments = futures.size();
             ListenableFuture<List<Iterable<T>>> future = 
Futures.allAsList(futures);
             queryWatcher.registerQueryFuture(query, future);
+            
+            if (completedSegments != null && totalSegments >= samplingWindow 
&& context.hasTimeout()) {
+              for (ListenableFuture<?> f : futures) {
+                f.addListener(
+                    () -> {
+                      if (extrapolationCancelled.get()) {
+                        return;
+                      }
+                      int completed = completedSegments.get();
+                      if (completed >= samplingWindow) {
+                        long elapsedMs = 
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - queryStartNanos);
+                        long extrapolatedMs = elapsedMs * totalSegments / 
completed;
+                        long remainingMs = context.getTimeout() - elapsedMs;
+                        if (extrapolatedMs > context.getTimeout() && 
remainingMs < extrapolatedMs - elapsedMs) {
+                          extrapolationCancelled.set(true);
+                          GuavaUtils.cancelAll(true, future, futures);
+                        }
+                      }
+                    },
+                    Execs.directExecutor()

Review Comment:
   I have some concerns over noisy-neighbor/variability that might cause thing 
to fire more often than not. Another concern is since this is operating on the 
servicing Jetty thread, this might cause interrupts to occur on the thread 
blocking on .get():
   
   I think we should be careful about scheduling async tasks on the main 
executor aside from the primary timeout. The main thread's job is to wait for 
completion of all futures, but if there are other competing tasks it needs to 
service, I want to make sure that:
   
   a) Under contention we cannot possibly get InterruptedException (to go 
service a callback) and bail the processing on a valid query.
   b) We don't delay processing of the future group because we are busy 
servicing a callback for one processing future.
   
   Do we have a way of validating this won't happen?



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

Reply via email to