You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ankitsultana (via GitHub)" <gi...@apache.org> on 2023/05/22 18:44:27 UTC
[GitHub] [pinot] ankitsultana opened a new pull request, #10794: [multistage] Avoid Busy Waiting in Broker
ankitsultana opened a new pull request, #10794:
URL: https://github.com/apache/pinot/pull/10794
At present the broker can run into a busy waiting loop if all the receiving mailbox are initialized but there's at least one mailbox which hasn't returned EOS yet.
This leads to that query hogging one entire CPU core which leads to obvious issues.
I guess long-term we would want to use a scheduler for the broker as well (or get rid of polling and use an event based approach), but meanwhile I think it might be good to fix this issue. We saw this in our prod clusters once recently.
We have known about this issue for a while now so thinking it's best to put a fix in: https://github.com/apache/pinot/issues/10131
cc: @walterddr
--
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 #10794: [multistage] Avoid Busy Waiting in Broker
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10794:
URL: https://github.com/apache/pinot/pull/10794#discussion_r1201467266
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -231,6 +232,9 @@ private static List<DataBlock> reduceMailboxReceive(MailboxReceiveOperator mailb
"Received error query execution result block: " + transferableBlock.getDataBlock().getExceptions());
}
if (transferableBlock.isNoOpBlock()) {
+ // If all mailbox return no-op block, then we sleep for some time before retrying. This can increase the latency
+ // of the query a little bit but it will prevent the broker from running into a busy-waiting loop.
+ Thread.sleep(10);
Review Comment:
the best way to run this is a "pipelinebreaker" which is described as MVP in #10779 .
--
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] ankitsultana commented on pull request #10794: [multistage] Avoid Busy Waiting in Broker
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #10794:
URL: https://github.com/apache/pinot/pull/10794#issuecomment-1592086279
Closing this as @walterddr mentioned that this should be fixed as part of the pipeline breaker work
--
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] ankitsultana commented on a diff in pull request #10794: [multistage] Avoid Busy Waiting in Broker
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10794:
URL: https://github.com/apache/pinot/pull/10794#discussion_r1202507747
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -231,6 +232,9 @@ private static List<DataBlock> reduceMailboxReceive(MailboxReceiveOperator mailb
"Received error query execution result block: " + transferableBlock.getDataBlock().getExceptions());
}
if (transferableBlock.isNoOpBlock()) {
+ // If all mailbox return no-op block, then we sleep for some time before retrying. This can increase the latency
+ // of the query a little bit but it will prevent the broker from running into a busy-waiting loop.
+ Thread.sleep(10);
Review Comment:
Until we have that approach finalized, can we merge this to fix master?
We can revert this and fix this properly with the pipeline breaker PR?
--
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 #10794: [multistage] Avoid Busy Waiting in Broker
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10794:
URL: https://github.com/apache/pinot/pull/10794#issuecomment-1557894186
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10794?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10794](https://app.codecov.io/gh/apache/pinot/pull/10794?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0a0368c) into [master](https://app.codecov.io/gh/apache/pinot/commit/c78026da7372d8ab4c71360df20c32c48a29da50?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c78026d) will **decrease** coverage by `46.27%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10794 +/- ##
=============================================
- Coverage 70.33% 24.07% -46.27%
+ Complexity 6486 49 -6437
=============================================
Files 2159 2143 -16
Lines 116044 115612 -432
Branches 17569 17514 -55
=============================================
- Hits 81623 27830 -53793
- Misses 28727 84841 +56114
+ Partials 5694 2941 -2753
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.07% <0.00%> (+0.03%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10794?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../pinot/query/service/dispatch/QueryDispatcher.java](https://app.codecov.io/gh/apache/pinot/pull/10794?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvc2VydmljZS9kaXNwYXRjaC9RdWVyeURpc3BhdGNoZXIuamF2YQ==) | `0.00% <0.00%> (-93.04%)` | :arrow_down: |
... and [1625 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10794/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=apache)
--
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] ankitsultana closed pull request #10794: [multistage] Avoid Busy Waiting in Broker
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana closed pull request #10794: [multistage] Avoid Busy Waiting in Broker
URL: https://github.com/apache/pinot/pull/10794
--
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