capistrant commented on code in PR #20805:
URL: https://github.com/apache/druid/pull/20805#discussion_r4433452453


##########
multi-stage-query/src/main/java/org/apache/druid/msq/querykit/ReadableInputQueue.java:
##########
@@ -331,47 +352,123 @@ private ListenableFuture<NumberedReadableInput> 
nextChannelInput()
   @Nullable
   private ListenableFuture<NumberedReadableInput> nextSegmentInput()
   {
-    // Pick a loadahead future, preferring ones that are already loaded.
-    ListenableFuture<NumberedReadableInput> selectedLoadaheadFuture = null;
     synchronized (this) {
-      for (ListenableFuture<NumberedReadableInput> f : pendingNextInputs) {
-        if (selectedLoadaheadFuture == null || f.isDone()) {
-          selectedLoadaheadFuture = f;
-          if (f.isDone()) {
-            break;
-          }
-        }
-      }
-
-      if (selectedLoadaheadFuture != null) {
-        pendingNextInputs.remove(selectedLoadaheadFuture);
+      while (!pendingNextInputs.isEmpty()) {
+        final ListenableFuture<NumberedReadableInput> selectedFuture = 
selectPendingInput();
+        final LoadableSegment selectedSegment = 
pendingNextInputs.remove(selectedFuture);

Review Comment:
   I have waffled back and forth on this my thought was that since we are 
processing intervals in sorted order and pruning based on interval, if you 
prune one you are about to prune everything and complete the stage. so it is 
wasteful but the queue is about to drain fast anyways. Either way I will take a 
look to see about this discard path.



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