You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "klsince (via GitHub)" <gi...@apache.org> on 2023/04/19 04:16:19 UTC
[GitHub] [pinot] klsince opened a new pull request, #10642: update target tier for segments if tierConfigs is provided
klsince opened a new pull request, #10642:
URL: https://github.com/apache/pinot/pull/10642
SegmentRelocator should check and update segment target tier when tierConfigs is provided, so that segment gets its right target tier set in its ZK metadata to make features like index config tier overwrites (https://github.com/apache/pinot/pull/10553) work properly. Previously, target tier was only updated for the multi-volume feature.
--
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 #10642: update target tier for segments if tierConfigs is provided
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10642:
URL: https://github.com/apache/pinot/pull/10642#issuecomment-1514129085
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10642?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 [#10642](https://codecov.io/gh/apache/pinot/pull/10642?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9f40e4f) into [master](https://codecov.io/gh/apache/pinot/commit/f2afe21b9962a0d676a7887edc999bcbee6c441d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2afe21) will **decrease** coverage by `56.45%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10642 +/- ##
=============================================
- Coverage 70.31% 13.87% -56.45%
+ Complexity 6495 439 -6056
=============================================
Files 2106 2052 -54
Lines 113381 110887 -2494
Branches 17090 16792 -298
=============================================
- Hits 79728 15381 -64347
- Misses 28059 94251 +66192
+ Partials 5594 1255 -4339
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `13.87% <0.00%> (-0.01%)` | :arrow_down: |
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/10642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...roller/helix/core/relocation/SegmentRelocator.java](https://codecov.io/gh/apache/pinot/pull/10642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vU2VnbWVudFJlbG9jYXRvci5qYXZh) | `49.12% <0.00%> (-17.74%)` | :arrow_down: |
... and [1660 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10642/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] npawar commented on a diff in pull request #10642: update target tier for segments if tierConfigs is provided
Posted by "npawar (via GitHub)" <gi...@apache.org>.
npawar commented on code in PR #10642:
URL: https://github.com/apache/pinot/pull/10642#discussion_r1171946228
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/relocation/SegmentRelocator.java:
##########
@@ -175,10 +175,6 @@ private void rebalanceTable(String tableNameWithType) {
relocate = true;
LOGGER.info("Relocating COMPLETED segments for table: {}", tableNameWithType);
}
- if (_enableLocalTierMigration) {
Review Comment:
multi volume feature was being controlled by this flag?
--
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] klsince commented on a diff in pull request #10642: update target tier for segments if tierConfigs is provided
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #10642:
URL: https://github.com/apache/pinot/pull/10642#discussion_r1171949861
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/relocation/SegmentRelocator.java:
##########
@@ -175,10 +175,6 @@ private void rebalanceTable(String tableNameWithType) {
relocate = true;
LOGGER.info("Relocating COMPLETED segments for table: {}", tableNameWithType);
}
- if (_enableLocalTierMigration) {
Review Comment:
on the server side, this TierBasedSegmentDirectoryLoader should be used, instead of the default loader. The former one sets the correct tier on the segment for the controller side logic to check.
--
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] klsince commented on a diff in pull request #10642: update target tier for segments if tierConfigs is provided
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #10642:
URL: https://github.com/apache/pinot/pull/10642#discussion_r1171949392
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/relocation/SegmentRelocator.java:
##########
@@ -175,10 +175,6 @@ private void rebalanceTable(String tableNameWithType) {
relocate = true;
LOGGER.info("Relocating COMPLETED segments for table: {}", tableNameWithType);
}
- if (_enableLocalTierMigration) {
Review Comment:
this flag controls the controller side logic - checking segment current vs. target tier and issue segment reload msg.
--
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] npawar merged pull request #10642: update target tier for segments if tierConfigs is provided
Posted by "npawar (via GitHub)" <gi...@apache.org>.
npawar merged PR #10642:
URL: https://github.com/apache/pinot/pull/10642
--
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