You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/06/15 09:25:02 UTC
[GitHub] [incubator-pinot] KKcorps opened a new pull request #7058: Add new partition group metadata at the time of segment commit
KKcorps opened a new pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058
Creates new partition groups while creating commitingSegment metadata instead of waiting till the RealtimeSegmentValidationManager runs
e.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E, then metadata/idealstate entries for D, E are created along with the committing partition's entries.
--
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.
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#issuecomment-861382903
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?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 [#7058](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (05b976d) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/a1c9b631381a25ddd6d3164d6a9ce337c3939b9f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a1c9b63) will **decrease** coverage by `8.09%`.
> The diff coverage is `84.09%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7058 +/- ##
============================================
- Coverage 73.38% 65.29% -8.10%
- Complexity 12 92 +80
============================================
Files 1453 1500 +47
Lines 72032 73770 +1738
Branches 10427 10640 +213
============================================
- Hits 52863 48166 -4697
- Misses 15643 22200 +6557
+ Partials 3526 3404 -122
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.29% <84.09%> (-0.12%)` | :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/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `68.43% <84.09%> (-11.31%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...startree/executor/StarTreeAggregationExecutor.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ager/realtime/PeerSchemeSplitSegmentCommitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUGVlclNjaGVtZVNwbGl0U2VnbWVudENvbW1pdHRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [703 more](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a1c9b63...05b976d](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-pinot] codecov-commenter edited a comment on pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#issuecomment-861382903
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?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 [#7058](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (521a0e6) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/a1c9b631381a25ddd6d3164d6a9ce337c3939b9f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a1c9b63) will **increase** coverage by `0.24%`.
> The diff coverage is `88.88%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7058 +/- ##
============================================
+ Coverage 73.38% 73.63% +0.24%
- Complexity 12 91 +79
============================================
Files 1453 1477 +24
Lines 72032 72820 +788
Branches 10427 10474 +47
============================================
+ Hits 52863 53622 +759
- Misses 15643 15734 +91
+ Partials 3526 3464 -62
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.90% <68.88%> (-0.23%)` | :arrow_down: |
| unittests | `65.57% <88.88%> (+0.17%)` | :arrow_up: |
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/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `80.25% <88.88%> (+0.52%)` | :arrow_up: |
| [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...i/config/table/ingestion/BatchIngestionConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9CYXRjaEluZ2VzdGlvbkNvbmZpZy5qYXZh) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [...ot/spi/config/table/ingestion/IngestionConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9Jbmdlc3Rpb25Db25maWcuamF2YQ==) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [...g/apache/pinot/segment/spi/memory/CleanerUtil.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL21lbW9yeS9DbGVhbmVyVXRpbC5qYXZh) | `33.87% <0.00%> (-23.28%)` | :arrow_down: |
| [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `40.42% <0.00%> (-19.15%)` | :arrow_down: |
| [...e/pinot/core/transport/InstanceRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvSW5zdGFuY2VSZXF1ZXN0SGFuZGxlci5qYXZh) | `62.31% <0.00%> (-16.26%)` | :arrow_down: |
| [.../apache/pinot/core/transport/DataTableHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvRGF0YVRhYmxlSGFuZGxlci5qYXZh) | `88.00% <0.00%> (-12.00%)` | :arrow_down: |
| [...pache/pinot/core/query/utils/idset/EmptyIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS91dGlscy9pZHNldC9FbXB0eUlkU2V0LmphdmE=) | `25.00% <0.00%> (-10.72%)` | :arrow_down: |
| [...altime/ServerSegmentCompletionProtocolHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL1NlcnZlclNlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2xIYW5kbGVyLmphdmE=) | `49.52% <0.00%> (-8.58%)` | :arrow_down: |
| ... and [368 more](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a1c9b63...521a0e6](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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.
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r667380336
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
Two questions:
- You are not actually updating the idealstate here, right?
- If you were to add code to update the idealstate, how would that work with a background job that is also trying to add new partitions simultaneously?
--
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] [incubator-pinot] KKcorps commented on pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
KKcorps commented on pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#issuecomment-877667542
@mcvsubbu can you please take a look at this
--
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r669006297
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
In that case, if the background job beats the segment commit logic, then there will already be a consuming segment created for the new partition, and a call to updateInstanceStatesaForNewConsumingSegment() will result in an exception. Just be sure to handle the exception correctly. It is better to throw a specific exception, so that you can catch that particular exception here.
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#issuecomment-861382903
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?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 [#7058](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (05b976d) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/a1c9b631381a25ddd6d3164d6a9ce337c3939b9f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a1c9b63) will **increase** coverage by `0.22%`.
> The diff coverage is `84.09%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7058 +/- ##
============================================
+ Coverage 73.38% 73.60% +0.22%
- Complexity 12 92 +80
============================================
Files 1453 1500 +47
Lines 72032 73770 +1738
Branches 10427 10640 +213
============================================
+ Hits 52863 54301 +1438
- Misses 15643 15936 +293
- Partials 3526 3533 +7
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `42.04% <63.63%> (-0.09%)` | :arrow_down: |
| unittests | `65.29% <84.09%> (-0.12%)` | :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/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `79.92% <84.09%> (+0.19%)` | :arrow_up: |
| [...segment/index/readers/ValidDocIndexReaderImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvVmFsaWREb2NJbmRleFJlYWRlckltcGwuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/controller/ControllerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyU3RhcnRlci5qYXZh) | `5.71% <0.00%> (-68.43%)` | :arrow_down: |
| [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `7.40% <0.00%> (-61.18%)` | :arrow_down: |
| [...in/java/org/apache/pinot/minion/MinionStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uU3RhcnRlci5qYXZh) | `18.18% <0.00%> (-57.26%)` | :arrow_down: |
| [...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==) | `8.33% <0.00%> (-45.13%)` | :arrow_down: |
| [...i/config/table/ingestion/BatchIngestionConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9CYXRjaEluZ2VzdGlvbkNvbmZpZy5qYXZh) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [...ot/spi/config/table/ingestion/IngestionConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9Jbmdlc3Rpb25Db25maWcuamF2YQ==) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | :arrow_down: |
| [...g/apache/pinot/segment/spi/memory/CleanerUtil.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL21lbW9yeS9DbGVhbmVyVXRpbC5qYXZh) | `33.87% <0.00%> (-23.28%)` | :arrow_down: |
| ... and [494 more](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a1c9b63...05b976d](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-pinot] KKcorps commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r669859367
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
Thanks a lot. Will do that!
--
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] [incubator-pinot] KKcorps commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r668578046
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
From my testing, this code does update the ideal state. Is there anything you see as missing.
As far as background job is concerned, I'll look into it. I believe taking a lock on some object is the only way
--
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r668903399
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
- The code updates ideal state, but it is not written back to zookeeper anywhere.
- It is possible (under some race conditions) that the background job runs in another controller. Need to be careful here. Good thing is, I have added code for idealstate update (PR #6483) so that we don't create two segments with the same seq number but different timestamp. It covers all existing code path, but since this is a new code path, you should make sure that is covered. In that case, the idealstate update will fail with an exception, and you need to handle it correctly.
--
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] [incubator-pinot] codecov-commenter commented on pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#issuecomment-861382903
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?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 [#7058](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (521a0e6) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/a1c9b631381a25ddd6d3164d6a9ce337c3939b9f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a1c9b63) will **decrease** coverage by `7.81%`.
> The diff coverage is `88.88%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7058 +/- ##
============================================
- Coverage 73.38% 65.57% -7.82%
- Complexity 12 91 +79
============================================
Files 1453 1477 +24
Lines 72032 72820 +788
Branches 10427 10474 +47
============================================
- Hits 52863 47754 -5109
- Misses 15643 21716 +6073
+ Partials 3526 3350 -176
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.57% <88.88%> (+0.17%)` | :arrow_up: |
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/incubator-pinot/pull/7058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `68.81% <88.88%> (-10.92%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [600 more](https://codecov.io/gh/apache/incubator-pinot/pull/7058/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a1c9b63...521a0e6](https://codecov.io/gh/apache/incubator-pinot/pull/7058?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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.
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7058: Add new partition group metadata at the time of segment commit
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r669004482
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
lock.unlock();
}
- // TODO: also create the new partition groups here, instead of waiting till the {@link RealtimeSegmentValidationManager} runs
+ // Creates new partition groups here instead of waiting till the {@link RealtimeSegmentValidationManager} runs
// E.g. If current state is A, B, C, and newPartitionGroupMetadataList contains B, C, D, E,
- // then create metadata/idealstate entries for D, E along with the committing partition's entries.
- // Ensure that multiple committing segments don't create multiple new segment metadata and ideal state entries for the same partitionGroup
+ // then metadata/idealstate entries for D, E are created along with the committing partition's entries.
+
+ addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, idealState, numReplicas, streamConfig,
+ newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, instancePartitionsMap);
// Trigger the metadata event notifier
_metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
}
+ /**
+ * Method is kept synchronised so that multiple committing segments don't create multiple new segment metadata
+ * and ideal state entries for the same partitionGroup
+ */
+ private synchronized void addNewPartitionGroups(String realtimeTableName, TableConfig tableConfig,
Review comment:
Ah ok, my bad. Yes idealstate is getting updated.
--
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