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/10 17:51:57 UTC
[GitHub] [pinot] ankitsultana opened a new pull request, #10888: Close PinotFS after Data Manager Shutdowns
ankitsultana opened a new pull request, #10888:
URL: https://github.com/apache/pinot/pull/10888
When a pinot server is shutting down gracefully, at present it first closes the PinotFS instances and then it waits for all data managers to be destroyed. This could mean that the segments that are getting committed in parallel will not be uploaded to the deep-store.
In this PR I am simply changing the order of these events: making PinotFS close after all segment data managers are destroyed.
This and some other issues are also documented in #10876
--
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] chenboat commented on a diff in pull request #10888: Close PinotFS after Data Manager Shutdowns
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #10888:
URL: https://github.com/apache/pinot/pull/10888#discussion_r1227177967
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -648,6 +642,14 @@ public void stop() {
}
_serverQueriesDisabledTracker.stop();
_realtimeLuceneIndexRefreshState.stop();
+ try {
+ // Close PinotFS after all data managers are shutdown. Otherwise, committing segments might not be able to
Review Comment:
committng segments --> committing 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] ankitsultana commented on a diff in pull request #10888: Close PinotFS after Data Manager Shutdowns
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10888:
URL: https://github.com/apache/pinot/pull/10888#discussion_r1227183104
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -648,6 +642,14 @@ public void stop() {
}
_serverQueriesDisabledTracker.stop();
_realtimeLuceneIndexRefreshState.stop();
+ try {
+ // Close PinotFS after all data managers are shutdown. Otherwise, committing segments might not be able to
Review Comment:
Thanks. fixed the wording.
--
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 #10888: Close PinotFS after Data Manager Shutdowns
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10888:
URL: https://github.com/apache/pinot/pull/10888#issuecomment-1585762601
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10888?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10888](https://app.codecov.io/gh/apache/pinot/pull/10888?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3f9af4e) 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 `n/a`.
```diff
@@ Coverage Diff @@
## master #10888 +/- ##
=============================================
- Coverage 34.47% 0.11% -34.36%
=============================================
Files 2159 2132 -27
Lines 116007 114988 -1019
Branches 17558 17460 -98
=============================================
- Hits 39989 137 -39852
- Misses 72545 114831 +42286
+ Partials 3473 20 -3453
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `?` | |
| unittests2temurin20 | `0.11% <ø> (?)` | |
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.
[see 1130 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10888/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 merged pull request #10888: Close PinotFS after Data Manager Shutdowns
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10888:
URL: https://github.com/apache/pinot/pull/10888
--
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