xyuanlu commented on code in PR #3043:
URL: https://github.com/apache/helix/pull/3043#discussion_r2165625299


##########
helix-core/src/main/java/org/apache/helix/model/OnlineOfflineSMD.java:
##########
@@ -62,9 +72,8 @@ public static StateModelDefinition build() {
     builder.addTransition(States.OFFLINE.name(), States.ONLINE.name(), 1);
     builder.addTransition(States.OFFLINE.name(), 
HelixDefinedState.DROPPED.name());
 
-    // bounds
-    builder.dynamicUpperBound(States.ONLINE.name(),
-        StateModelDefinition.STATE_REPLICA_COUNT_ALL_REPLICAS);
+    // bounds - uses the instanceCount parameter instead of constant
+    builder.dynamicUpperBound(States.ONLINE.name(), instanceCount);

Review Comment:
   I am not sure about this.
   1. Changing the existing state model is dangerous, would cause some behavior 
change. Especially users are using the state model. Normally we don't change 
any public API behavior.
   2. If we want to introduce a new state model we should define a new one. 
Especially if it is used for test, define one only for test. Dont change public 
API.



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

Reply via email to