gortiz commented on code in PR #11205:
URL: https://github.com/apache/pinot/pull/11205#discussion_r1277768278


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxReceiveOperator.java:
##########
@@ -46,41 +42,6 @@ public String toExplainString() {
 
   @Override
   protected TransferableBlock getNextBlock() {
-    if (_errorBlock != null) {
-      return _errorBlock;
-    }
-    if (System.currentTimeMillis() > _context.getDeadlineMs()) {
-      _errorBlock = 
TransferableBlockUtils.getErrorTransferableBlock(QueryException.EXECUTION_TIMEOUT_ERROR);
-      return _errorBlock;
-    }
-
-    // Poll from every mailbox in round-robin fashion:
-    // - Return the first content block
-    // - If no content block found but there are mailboxes not finished, 
return no-op block
-    // - If all content blocks are already returned, return end-of-stream block
-    int numMailboxes = _mailboxes.size();
-    for (int i = 0; i < numMailboxes; i++) {
-      ReceivingMailbox mailbox = _mailboxes.remove();
-      TransferableBlock block = mailbox.poll();
-
-      // Release the mailbox when the block is end-of-stream
-      if (block != null && block.isSuccessfulEndOfStreamBlock()) {
-        _mailboxService.releaseReceivingMailbox(mailbox);
-        _opChainStats.getOperatorStatsMap().putAll(block.getResultMetadata());
-        continue;
-      }
-
-      // Add the mailbox back to the queue if the block is not end-of-stream
-      _mailboxes.add(mailbox);
-      if (block != null) {
-        if (block.isErrorBlock()) {
-          _errorBlock = block;
-        }
-        return block;
-      }
-    }
-
-    return _mailboxes.isEmpty() ? 
TransferableBlockUtils.getEndOfStreamTransferableBlock()
-        : TransferableBlockUtils.getNoOpTransferableBlock();
+    return readBlockBlocking();

Review Comment:
   Now the logic is the same implemented in the parent method. I decided to do 
not implement `getNextBlock` directly in `BaseReceiveOperator` just in case we 
add some extra receive operator later. Also, I think the sorted is clearer 
calling `readBlockBlocking` instead of `super.getNextBlock()`



-- 
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...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to