61yao commented on code in PR #9887:
URL: https://github.com/apache/pinot/pull/9887#discussion_r1040065008
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxReceiveOperator.java:
##########
@@ -165,11 +172,13 @@ protected TransferableBlock getNextBlock() {
}
}
- // if we opened at least one mailbox, but still got to this point, then
that means
- // all the mailboxes we opened returned null but were not yet closed -
early terminate
- // with a noop block. Otherwise, we have exhausted all data from all
mailboxes and can
- // return EOS
- return openMailboxCount > 0 ?
TransferableBlockUtils.getNoOpTransferableBlock()
+ // there are two conditions in which we should return EOS: (1) there were
+ // no mailboxes to open (this shouldn't happen because the second condition
+ // should be hit first, but is defensive) (2) every mailbox that was opened
+ // returned an EOS block. in every other scenario, there are mailboxes that
+ // are not yet exhausted and we should wait for more data to be available
+ return openMailboxCount > 0 && openMailboxCount > eosMailboxCount
Review Comment:
Just a note: This basically means if we don't close mailbox correctly, the
corresponding opchain will got stuck in ready q forever.
--
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]
