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 2022/11/16 20:38:32 UTC

[GitHub] [pinot] GSharayu opened a new pull request, #9816: Fix the replication in segment assignment strategy

GSharayu opened a new pull request, #9816:
URL: https://github.com/apache/pinot/pull/9816

   The Balanced and Replica group segment assignment strategy would assume replication to be fetched from only table config as tableConfig.getValidationConfig().getReplicationNumber() irrespective of whether its a realtime table or offline table
   But for real time tables, if we have tableConfig.getValidationConfig().getReplicasPerPartitionNumber(), this should be preferred over table replication
   
   No additional changes are required
   Reference Issue: #9309
   


-- 
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] snleee merged pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
snleee merged PR #9816:
URL: https://github.com/apache/pinot/pull/9816


-- 
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 a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024527281


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   Please tag this PR with the (open) issue https://github.com/apache/pinot/issues/8804.
   
   Since we have a problem in production, we should fix in some way (add TODOs as needed), but fix the overall replication/replicasPerPartition as a part of that issue.



-- 
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] snleee commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
snleee commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024540397


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +52,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (tableConfig.getTableType() == TableType.REALTIME) {

Review Comment:
   Let's add the `TODO` comment that we can clean this up once the table config has the new API that picks up the correct replication depending on the table type.



-- 
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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024612190


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   done



-- 
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] GSharayu commented on pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on PR #9816:
URL: https://github.com/apache/pinot/pull/9816#issuecomment-1317640142

   @mcvsubbu @sajjad-moradi @jugomezv @snleee 


-- 
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] navina commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024504515


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   Is it possible to wrap all the logic for whether to use `replication` or `replicasPerPartition` within the TableConfig itself? That way, `tableConfig.getReplicationNumber` should just return the correct value depending on whether it is offline or realtime table. 



-- 
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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024506869


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   we saw real time tables using balanced strategy in Production. @jugomezv can you please confirm



-- 
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] snleee commented on pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
snleee commented on PR #9816:
URL: https://github.com/apache/pinot/pull/9816#issuecomment-1317701881

   Can we also add the testing case to cover this? We should add the test where we set `replication / numReplicasPerPartition` differently.


-- 
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] snleee commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
snleee commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024547545


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +52,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (tableConfig.getTableType() == TableType.REALTIME) {
+      _replication = validationAndRetentionConfig.getReplicasPerPartitionNumber();
+    }

Review Comment:
   +1 πŸ‘ 



-- 
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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024512709


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   @navina We can wire that in table config but it would also mean cleaning up at all the other places we use this. I would so keep that out of scope for this fix 



-- 
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] sajjad-moradi commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024544432


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +52,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (tableConfig.getTableType() == TableType.REALTIME) {
+      _replication = validationAndRetentionConfig.getReplicasPerPartitionNumber();
+    }

Review Comment:
   nit:
   ```java
   _replication = tableConfig.getTableType() == TableType.REALTIME
       ? validationAndRetentionConfig.getReplicasPerPartitionNumber()
       : validationAndRetentionConfig.getReplicationNumber();
   ```



-- 
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] GSharayu closed pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu closed pull request #9816: Fix the replication in segment assignment strategy
URL: https://github.com/apache/pinot/pull/9816


-- 
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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024506614


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   That was the original thought @navina . @snleee I updated the 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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024611688


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +52,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (tableConfig.getTableType() == TableType.REALTIME) {
+      _replication = validationAndRetentionConfig.getReplicasPerPartitionNumber();
+    }

Review Comment:
   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

[GitHub] [pinot] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024611832


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +52,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (tableConfig.getTableType() == TableType.REALTIME) {

Review Comment:
   added a todo and a unit test.



-- 
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 #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9816:
URL: https://github.com/apache/pinot/pull/9816#issuecomment-1317680379

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/9816?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 [#9816](https://codecov.io/gh/apache/pinot/pull/9816?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (212da59) into [master](https://codecov.io/gh/apache/pinot/commit/47c8f184365d6ffde870f94285747b9e009c7c15?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47c8f18) will **increase** coverage by `3.59%`.
   > The diff coverage is `87.00%`.
   
   > :exclamation: Current head 212da59 differs from pull request most recent head 0d53147. Consider uploading reports for the commit 0d53147 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #9816      +/-   ##
   ============================================
   + Coverage     60.58%   64.18%   +3.59%     
   + Complexity     5281     4972     -309     
   ============================================
     Files          1949     1910      -39     
     Lines        104632   102611    -2021     
     Branches      15847    15609     -238     
   ============================================
   + Hits          63389    65858    +2469     
   + Misses        36540    31973    -4567     
   - Partials       4703     4780      +77     
   ```
   
   | Flag | Coverage Ξ” | |
   |---|---|---|
   | integration2 | `?` | |
   | unittests1 | `67.84% <86.45%> (+0.03%)` | :arrow_up: |
   | unittests2 | `15.77% <87.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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9816?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [...apache/pinot/common/function/FunctionRegistry.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25SZWdpc3RyeS5qYXZh) | `84.21% <ΓΈ> (-2.64%)` | :arrow_down: |
   | [...not/query/planner/logical/RelToStageConverter.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JlbFRvU3RhZ2VDb252ZXJ0ZXIuamF2YQ==) | `81.48% <ΓΈ> (+1.85%)` | :arrow_up: |
   | [...che/pinot/query/planner/logical/RexExpression.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JleEV4cHJlc3Npb24uamF2YQ==) | `78.66% <ΓΈ> (+2.66%)` | :arrow_up: |
   | [...ache/pinot/query/planner/logical/StagePlanner.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1N0YWdlUGxhbm5lci5qYXZh) | `100.00% <ΓΈ> (ΓΈ)` | |
   | [...e/pinot/query/runtime/operator/FilterOperator.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9GaWx0ZXJPcGVyYXRvci5qYXZh) | `76.00% <ΓΈ> (+16.00%)` | :arrow_up: |
   | [...ery/runtime/operator/operands/FunctionOperand.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9vcGVyYW5kcy9GdW5jdGlvbk9wZXJhbmQuamF2YQ==) | `100.00% <ΓΈ> (ΓΈ)` | |
   | [...query/runtime/operator/MailboxReceiveOperator.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9NYWlsYm94UmVjZWl2ZU9wZXJhdG9yLmphdmE=) | `79.36% <50.00%> (-0.97%)` | :arrow_down: |
   | [...query/runtime/operator/operands/FilterOperand.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9vcGVyYW5kcy9GaWx0ZXJPcGVyYW5kLmphdmE=) | `83.90% <50.00%> (+16.05%)` | :arrow_up: |
   | [...ot/query/runtime/executor/RoundRobinScheduler.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9leGVjdXRvci9Sb3VuZFJvYmluU2NoZWR1bGVyLmphdmE=) | `87.50% <87.50%> (ΓΈ)` | |
   | [...uery/runtime/executor/OpChainSchedulerService.java](https://codecov.io/gh/apache/pinot/pull/9816/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9leGVjdXRvci9PcENoYWluU2NoZWR1bGVyU2VydmljZS5qYXZh) | `95.12% <95.12%> (ΓΈ)` | |
   | ... and [612 more](https://codecov.io/gh/apache/pinot/pull/9816/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) | |
   
   :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] GSharayu commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024506869


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   But we saw real time tables using balanced strategy in Production. @jugomezv can you please confirm



-- 
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] GSharayu closed pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
GSharayu closed pull request #9816: Fix the replication in segment assignment strategy
URL: https://github.com/apache/pinot/pull/9816


-- 
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] snleee commented on a diff in pull request #9816: Fix the replication in segment assignment strategy

Posted by GitBox <gi...@apache.org>.
snleee commented on code in PR #9816:
URL: https://github.com/apache/pinot/pull/9816#discussion_r1024500603


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/strategy/BalancedNumSegmentAssignmentStrategy.java:
##########
@@ -51,6 +51,10 @@ public void init(HelixManager helixManager, TableConfig tableConfig) {
     SegmentsValidationAndRetentionConfig validationAndRetentionConfig = tableConfig.getValidationConfig();
     Preconditions.checkState(validationAndRetentionConfig != null, "Validation Config is null");
     _replication = validationAndRetentionConfig.getReplicationNumber();
+    // Number of replicas per partition of low-level consumers check is for the real time tables only
+    if (validationAndRetentionConfig.getReplicasPerPartition() != null) {

Review Comment:
   We need to check if the table is offline or real-time and then decide where to pull the replication data.



-- 
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