mcvsubbu commented on a change in pull request #4533: [Instance Assignment] 
De-couple assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314769606
 
 

 ##########
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java
 ##########
 @@ -23,37 +23,42 @@
 import org.apache.commons.configuration.Configuration;
 import org.apache.helix.HelixManager;
 import org.apache.pinot.common.config.TableConfig;
+import org.apache.pinot.common.utils.InstancePartitionsType;
+import org.apache.pinot.controller.helix.core.assignment.InstancePartitions;
 
 
 /**
- * Strategy to assign segment to instances or rebalance all segments in a 
table.
+ * Interface for segment assignment and table rebalancing.
 
 Review comment:
   Our documentation states that SegmentAssignmentStrategy is public. You 
should keep the existing interface and introduce a new one to suit the needs.
   
https://pinot.readthedocs.io/en/latest/customizations.html#segment-assignment-strategies
   
   Otherwise it is an incompatible change

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to