xyuanlu commented on code in PR #2755:
URL: https://github.com/apache/helix/pull/2755#discussion_r1487032870
##########
helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java:
##########
@@ -165,37 +170,36 @@ private void
addSwapInInstancesToBestPossibleState(Map<String, Resource> resourc
}
// If the swap-in node is live and enabled, do assignment with
the following logic:
- // 1. If the swap-out instance's replica is a topState, set
the swap-in instance's replica
- // to the topState if the StateModel allows for another
replica with the topState to be added.
- // Otherwise, set the swap-in instance's replica to the
secondTopState.
- // 2. If the swap-out instance's replica is a secondTopState,
set the swap-in instance's replica
+ // 1. If the swap-out instance's replica is a secondTopState,
set the swap-in instance's replica
// to the same secondTopState.
- if
(stateMap.get(swapOutInstance).equals(stateModelDef.getTopState())) {
+ // 2. If the swap-out instance's replica is any other state
and is in the preferenceList,
+ // set the swap-in instance's replica to the topState if the
StateModel allows for another replica with the topState
+ // to be added. We can make this assumption because if there
is assignment to the swapOutInstance, it must be either
+ // a topState or a secondTopState.
+ if (stateMap.containsKey(swapOutInstance) &&
stateModelDef.getSecondTopStates()
+ .contains(stateMap.get(swapOutInstance))) {
+ // If the swap-out instance's replica is a secondTopState,
set the swap-in instance's replica
+ // to the same secondTopState.
+
stateMap.put(swapOutToSwapInInstancePairs.get(swapOutInstance),
+ stateMap.get(swapOutInstance));
+ } else {
String topStateCount =
stateModelDef.getNumInstancesPerState(stateModelDef.getTopState());
if (topStateCount.equals(
StateModelDefinition.STATE_REPLICA_COUNT_ALL_CANDIDATE_NODES)
|| topStateCount.equals(
StateModelDefinition.STATE_REPLICA_COUNT_ALL_REPLICAS)) {
- // If the swap-out instance's replica is a topState and
the StateModel allows for
- // another replica with the topState to be added, set the
swap-in instance's replica
- // to the topState.
+ // If the StateModel allows for another replica with the
topState to be added,
Review Comment:
nit: I think the comment can be 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
