You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/03/16 01:21:26 UTC
[GitHub] [pinot] walterddr opened a new pull request, #10432: [multistage] fix mailbox cancel race condition
walterddr opened a new pull request, #10432:
URL: https://github.com/apache/pinot/pull/10432
see #10424 for more details
this is another attempt to address the problem discovered in #10417
- comparing to approach #10425 this one doesn't do a fix delay instead will try to leverage the onNext sequential nature to guarantee delivery of at least 1 message, thus not causing the un-started stream issue.
- this approach could block when onNext() is slow and thus potentially slow down the closing of an exception channel.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10432: [multistage] fix mailbox cancel race condition
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10432:
URL: https://github.com/apache/pinot/pull/10432#issuecomment-1471167654
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10432?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10432](https://codecov.io/gh/apache/pinot/pull/10432?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (265dc3b) into [master](https://codecov.io/gh/apache/pinot/commit/11b6bcda50469898e96a03aa6a0c92bbcbe6463c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (11b6bcd) will **decrease** coverage by `38.72%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10432 +/- ##
=============================================
- Coverage 63.23% 24.51% -38.72%
+ Complexity 5068 49 -5019
=============================================
Files 2028 2028
Lines 110632 110635 +3
Branches 16847 16847
=============================================
- Hits 69955 27126 -42829
- Misses 35505 80680 +45175
+ Partials 5172 2829 -2343
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.51% <0.00%> (+0.12%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10432?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/query/mailbox/GrpcSendingMailbox.java](https://codecov.io/gh/apache/pinot/pull/10432?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9HcnBjU2VuZGluZ01haWxib3guamF2YQ==) | `0.00% <0.00%> (-81.58%)` | :arrow_down: |
... and [1299 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10432/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 merged pull request #10432: [multistage] fix mailbox cancel race condition
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #10432:
URL: https://github.com/apache/pinot/pull/10432
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10432: [multistage] fix mailbox cancel race condition
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10432:
URL: https://github.com/apache/pinot/pull/10432#discussion_r1137981097
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -72,6 +75,7 @@ public void send(TransferableBlock block)
@Override
public void complete()
throws Exception {
+ // TODO: should wait for _mailboxContentStreamObserver.onNext() finish before calling onComplete().
Review Comment:
currently it is guaranteed that onComplete is called after all onNext() --> implicitly via the OpChain scheduler
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10432: [multistage] fix mailbox cancel race condition
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10432:
URL: https://github.com/apache/pinot/pull/10432#discussion_r1137981097
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -72,6 +75,7 @@ public void send(TransferableBlock block)
@Override
public void complete()
throws Exception {
+ // TODO: should wait for _mailboxContentStreamObserver.onNext() finish before calling onComplete().
Review Comment:
I think currently it is guaranteed that onComplete is called after all onNext() --> implicitly via the OpChain scheduler. however we should still fix this systematically
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org