agavra commented on code in PR #9887:
URL: https://github.com/apache/pinot/pull/9887#discussion_r1041199197
##########
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:
correct - it will live in the `_available` queue and never move to `_ready`
also, just to make it clear (I think it may not have been with my first
comment), the mailbox doesn't technically need to be closed properly so long as
we get the EOS blocks from all open ones at the same time (which happens in the
happy 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]
