gortiz commented on code in PR #18519:
URL: https://github.com/apache/pinot/pull/18519#discussion_r3266796779
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -102,7 +128,13 @@ public void send(MseBlock.Data data) {
@Override
public void send(MseBlock.Eos block, List<DataBuffer> serializedStats) {
- if (sendInternal(block, serializedStats)) {
+ // EOS blocks (success or error) carry control-plane info — including the
original error code on the error path —
+ // and must always reach the receiver, so they bypass the back-pressure
gate. Bypassing also disables the
+ // cooperative termination poll inside [#awaitReady]; without it, a
terminate signal raised while the sender is
+ // mid-way through pushing an error EOS would unwind [#sendInternal] with
a TerminationException, leave
+ // [#_senderSideClosed] false, and let [#cancel] run and overwrite the
original error code with
+ // QUERY_CANCELLATION on the receiver side.
+ if (sendInternal(block, serializedStats, /* bypassReady */ true)) {
LOGGER.debug("Completing mailbox: {}", _id);
_contentObserver.onCompleted();
Review Comment:
Both addressed in commit `573356ee74` ("Fix race between EOS-send /
data-send and concurrent cancel").
1. Success path now sets `_senderSideClosed = true` **before**
`_contentObserver.onCompleted()` (`GrpcSendingMailbox.java:142-143`) with an
inline comment explaining the race window.
2. `sendContent` now re-checks termination after `awaitReady` returns: `if
(!bypassReady && isTerminated()) return;` before
`_contentObserver.onNext(content)` (`GrpcSendingMailbox.java:344`). The bypass
path (cancel/close) still pushes through unconditionally as you'd expect.
The fully-serialized-under-`_readyLock` variant is left as a follow-up — the
post-awaitReady re-check shrinks the window enough that it's no longer
load-bearing for correctness, just a narrowed-by-construction race against a
concurrent cancel that's racing the same observer regardless.
--
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]
