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/06/11 06:35:58 UTC
[GitHub] [pinot] ankitsultana opened a new pull request, #10892: [Draft] Skip Zk Call in Segment Post Commit Phase
ankitsultana opened a new pull request, #10892:
URL: https://github.com/apache/pinot/pull/10892
As part of graceful shutdown, we wait for each consumer thread to stop.
If a segment is committing, we will wait for the commit to finish before continuing with the shutdown (assuming commit happens within the grace period).
At present after the `SegmentCommitter` returns success, we call `replaceLLSegment` which makes a ZK call. However, as part of graceful shutdown we stop the HelixManager before destroying the segment data managers so the ZK client is already closed.
We don't need to refetch the table-config and schema because they are already present in the segment data manager and this PR essentially makes that change.
The goal is to ensure the commit completes cleanly during server shutdown.
cc: @Jackie-Jiang
--
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] mcvsubbu commented on pull request #10892: [Draft] Skip Zk Call in Segment Post Commit Phase
Posted by "mcvsubbu (via GitHub)" <gi...@apache.org>.
mcvsubbu commented on PR #10892:
URL: https://github.com/apache/pinot/pull/10892#issuecomment-1595473885
> Before actually fixing this issue, I want to discuss should we allow committing segment during the gracefully shutdown period? We need a clear cutting point beyond which there should be no commit allowed. When we invoke the cutting point method, existing commit can go through, but no new commit will be accepted. The method can be blocked until the ongoing commit finishes. Then we can safely disconnect from Helix.
+1
The protocol handles the case where the commit does not succeed. The servers try again, with other (or the newly restarted) controller to complete the segment. And, the segment is _not_ rebuilt in the servers.
--
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 #10892: [Draft] Skip Zk Call in Segment Post Commit Phase
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10892:
URL: https://github.com/apache/pinot/pull/10892#issuecomment-1586049348
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10892?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10892](https://app.codecov.io/gh/apache/pinot/pull/10892?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (2a2e348) into [master](https://app.codecov.io/gh/apache/pinot/commit/b76f24cb30c7c92c2839faac5ea6a97ac01f2044?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b76f24c) will **decrease** coverage by `34.36%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10892 +/- ##
=============================================
- Coverage 34.47% 0.11% -34.36%
=============================================
Files 2159 2132 -27
Lines 116007 114992 -1015
Branches 17558 17460 -98
=============================================
- Hits 39989 137 -39852
- Misses 72545 114835 +42290
+ Partials 3473 20 -3453
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `?` | |
| unittests2temurin11 | `0.11% <0.00%> (?)` | |
| unittests2temurin20 | `0.11% <0.00%> (?)` | |
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/10892?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/10892?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `0.00% <0.00%> (-62.94%)` | :arrow_down: |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/10892?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
... and [1128 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10892/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] Jackie-Jiang commented on pull request #10892: [Draft] Skip Zk Call in Segment Post Commit Phase
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #10892:
URL: https://github.com/apache/pinot/pull/10892#issuecomment-1595422857
Before actually fixing this issue, I want to discuss should we allow committing segment during the gracefully shutdown period? We need a clear cutting point beyond which there should be no commit allowed. When we invoke the cutting point method, existing commit can go through, but no new commit will be accepted. The method can be blocked until the ongoing commit finishes. Then we can safely disconnect from Helix.
--
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