Copilot commented on code in PR #19268:
URL: https://github.com/apache/pinot/pull/19268#discussion_r3790499410
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/mailbox/MailboxServiceTest.java:
##########
@@ -560,6 +563,12 @@ public void testRemoteBufferFull()
for (int i = 0; i < ReceivingMailbox.DEFAULT_MAX_PENDING_BLOCKS; i++) {
sendingMailbox.send(OperatorTestUtil.block(DATA_SCHEMA, new
Object[]{"0"}));
}
+ // Wait until the buffer is full before sending the next block, so that
the remaining deadline is spent parked on
+ // the full buffer instead of racing the block delivery. This also
guarantees the error block comes from the
+ // buffer-full timeout rather than from the deadline cancelling the stream
mid-delivery.
Review Comment:
This wait does not guarantee the overflow path described here. It only
proves the first five blocks reached the receiver; the following
`sendingMailbox.send(...)` returns after a client-side gRPC `onNext`, before
the receiver necessarily enters `offerData`. If the deadline expires in that
gap, `MailboxContentObserver.onError` installs an INTERNAL error and supplies
the sixth callback, so every assertion below still passes without exercising
the buffer-full timeout. Please also assert that the received `ErrorMseBlock`
contains `QueryErrorCode.EXECUTION_TIMEOUT` (and ideally the buffer-full
message), or synchronize on the overflowing offer actually being parked.
--
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]
