[multistage] Add Callbacks for Complete Events#10564
[multistage] Add Callbacks for Complete Events#10564walterddr merged 3 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10564 +/- ##
============================================
- Coverage 70.29% 64.36% -5.93%
- Complexity 6462 6473 +11
============================================
Files 2103 2052 -51
Lines 112767 110960 -1807
Branches 16979 16800 -179
============================================
- Hits 79265 71420 -7845
- Misses 27947 34372 +6425
+ Partials 5555 5168 -387
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 526 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java
Outdated
Show resolved
Hide resolved
| @Override | ||
| public void onCompleted() { | ||
| _isCompleted.set(true); | ||
| _gotMailCallback.accept(_mailboxId); |
There was a problem hiding this comment.
shouldn't we also wake up onError as well. we need also to populate errors upstream?
There was a problem hiding this comment.
We are doing that already right? (L145 above)
There was a problem hiding this comment.
hmm. interesting i see some errors not populating back correctly issue. maybe it is a different problem (remove error vs. local error?)
There was a problem hiding this comment.
One open issue right now is that in case of an error we can potentially return "CANCELLED" error instead of the actual error block due to the race condition we talked about in the past. Are you seeing something else?
There was a problem hiding this comment.
that's probably what's happening. let me double check the code and make sure it was the case. was it similar to the root cause for: #10555 ?
There was a problem hiding this comment.
Nope for #10555 the issue was with the sorting logic in the mailbox receive operator.
|
tagging @Jackie-Jiang to take a look as well |
3 participants
Complete events should also be able to wake up an OpChain.