siddharthteotia commented on a change in pull request #4446: Add support in the 
rebalancer for the user to provide minimum number of serving replicas
URL: https://github.com/apache/incubator-pinot/pull/4446#discussion_r308324988
 
 

 ##########
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/TableRebalancer.java
 ##########
 @@ -212,39 +248,136 @@ private IdealState getNextState(IdealState 
currentState, IdealState targetState,
   }
 
   /**
-   * Updates a segment mapping if needed. In "downtime" mode or if there are 
common elements between source and
-   * target mapping, the segment mapping is set to the target mapping directly.
-   * In a no-downtime, if there are no commmon elements, one element of the 
source mapping is replaced with one
-   * from the target mapping.
+   * Updates a segment mapping if needed. In "downtime" mode.
+   * the segment mapping is set to the target mapping directly.
+   *
+   * In no-downtime mode, we check for couple of things and
+   * keep a certain number of serving replicas for the segment alive
+   * as specified in the rebalance configuration
+   *
+   * (1) if the number of common hosts between current and target
+   * mapping are enough to satisfy more than or equal to the number
+   * of serving replicas we should keep alive then we set to the
+   * target mapping directly (at one go) since this honors the no-downtime
+   * requirement
+   *
+   * (2) however, if there are not enough common hosts then we don't
+   * change the mapping directly as that will result in downtime. The
+   * mapping is updated incrementally (only one change for a given
+   * invocation of this method per segment) by removing a host
+   * from the current mapping and adding a host to it from
+   * target mapping.
+   *
+   * When the caller of this method returns (after going over all
+   * segments once), it persists the new ideal state and then determines
+   * if we have reached the target. If not, the process continues and we come
+   * here again for each method and check for steps (1) or (2) as applicable
+   *
+   * @param segmentId segment id
+   * @param currentIdealStateSegmentHosts map of this segment's hosts 
(replicas)
+   *                                     in current ideal state
+   * @param targetIdealStateSegmentHosts map of this segment's hosts (replicas)
+   *                                     in target ideal state
+   * @param idealStateToUpdate the ideal state that is updated as (caller
+   *                           passes this as a copy of current ideal state)
+   * @param rebalanceUserConfig rebalance configuration
    */
   @VisibleForTesting
-  public void updateSegmentIfNeeded(String segmentId, Map<String, String> 
srcMap, Map<String, String> targetMap,
-      IdealState idealState, Configuration rebalanceUserConfig) {
+  public void updateSegmentIfNeeded(String segmentId, Map<String, String> 
currentIdealStateSegmentHosts,
+      Map<String, String> targetIdealStateSegmentHosts, IdealState 
idealStateToUpdate,
+      Configuration rebalanceUserConfig) {
+
+    LOGGER.info("Will update instance map of segment: {}", segmentId);
 
 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.
 
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