Jackie-Jiang commented on a change in pull request #5631:
URL: https://github.com/apache/incubator-pinot/pull/5631#discussion_r454050108
##########
File path:
pinot-controller/src/test/java/org/apache/pinot/controller/helix/PinotControllerModeTest.java
##########
@@ -248,6 +253,16 @@ public void testPinotOnlyController() {
helixOnlyController.stop();
}
+ private void checkHelixConstraints(HelixAdmin helixAdmin) {
+ ClusterConstraints constraints =
+ helixAdmin.getConstraints(getHelixClusterName(),
ClusterConstraints.ConstraintType.MESSAGE_CONSTRAINT);
+ ConstraintItem item =
constraints.getConstraintItem("MaxStateTransitionsPerInstance");
+ Assert.assertEquals(".*",
item.getAttributeValue(ClusterConstraints.ConstraintAttribute.INSTANCE));
Review comment:
Please reverse the arguments for all the asserts, where the first should
be actual, and the second should be expected.
##########
File path:
pinot-controller/src/test/java/org/apache/pinot/controller/helix/PinotControllerModeTest.java
##########
@@ -248,6 +253,16 @@ public void testPinotOnlyController() {
helixOnlyController.stop();
}
+ private void checkHelixConstraints(HelixAdmin helixAdmin) {
+ ClusterConstraints constraints =
+ helixAdmin.getConstraints(getHelixClusterName(),
ClusterConstraints.ConstraintType.MESSAGE_CONSTRAINT);
+ ConstraintItem item =
constraints.getConstraintItem("MaxStateTransitionsPerInstance");
+ Assert.assertEquals(".*",
item.getAttributeValue(ClusterConstraints.ConstraintAttribute.INSTANCE));
+ Assert
+ .assertEquals("STATE_TRANSITION",
item.getAttributeValue(ClusterConstraints.ConstraintAttribute.MESSAGE_TYPE));
Review comment:
```suggestion
.assertEquals(item.getAttributeValue(ClusterConstraints.ConstraintAttribute.MESSAGE_TYPE),
Message.MessageType.STATE_TRANSITION.name());
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##########
@@ -109,6 +109,8 @@
public static final String CONFIG_OF_BROKER_FLAPPING_TIME_WINDOW_MS =
"pinot.broker.flapping.timeWindowMs";
public static final String CONFIG_OF_SERVER_FLAPPING_TIME_WINDOW_MS =
"pinot.server.flapping.timeWindowMs";
public static final String CONFIG_OF_MINION_FLAPPING_TIME_WINDOW_MS =
"pinot.minion.flapping.timeWindowMs";
+ public static final String CONFIG_OF_HELIX_INSTANCE_MAX_STATE_TRANSITIONS
= "pinot.helix.instance.state.transitions.max";
Review comment:
```suggestion
public static final String
CONFIG_OF_HELIX_INSTANCE_MAX_STATE_TRANSITIONS =
"pinot.helix.instance.maxStateTransitions";
```
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -202,6 +209,20 @@ private void setupHelixSystemProperties() {
CommonConstants.Helix.DEFAULT_FLAPPING_TIME_WINDOW_MS));
}
+ private void setupHelixClusterConstraints() {
+ String maxMessageLimit =
_config.getString(CommonConstants.Helix.CONFIG_OF_HELIX_INSTANCE_MAX_STATE_TRANSITIONS,
Review comment:
```suggestion
String maxStateTransitions =
_config.getString(CommonConstants.Helix.CONFIG_OF_HELIX_INSTANCE_MAX_STATE_TRANSITIONS,
```
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
