You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/07/01 01:42:44 UTC
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5631: Add the message constraint on all instances in Helix
Jackie-Jiang commented on a change in pull request #5631:
URL: https://github.com/apache/incubator-pinot/pull/5631#discussion_r448067581
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -97,6 +103,7 @@
private static final Long DATA_DIRECTORY_MISSING_VALUE = 1000000L;
private static final Long DATA_DIRECTORY_EXCEPTION_VALUE = 1100000L;
private static final String METADATA_EVENT_NOTIFIER_PREFIX = "metadata.event.notifier";
+ private static final String MAX_MESSAGES_PER_INSTANCE_NAME = "MaxMessagesPerInstance";
Review comment:
```suggestion
private static final String MAX_STATE_TRANSITIONS_PER_INSTANCE = "MaxStateTransitionsPerInstance";
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -202,6 +209,18 @@ private void setupHelixSystemProperties() {
CommonConstants.Helix.DEFAULT_FLAPPING_TIME_WINDOW_MS));
}
+ private void setupHelixClusterConstraints() {
+ String maxMessageLimit = _config.getString(CommonConstants.Helix.CONFIG_OF_HELIX_INSTANCE_MAX_MESSAGES,
+ CommonConstants.Helix.DEFAULT_HELIX_INSTANCE_MAX_MESSAGES);
+ Map<ClusterConstraints.ConstraintAttribute, String> constraintAttributes = Maps.newHashMap();
Review comment:
No need to use guava for this. We have experienced several backward-incompatible issues with guava.
```suggestion
Map<ClusterConstraints.ConstraintAttribute, String> constraintAttributes = new HashMap()<>;
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -202,6 +209,18 @@ private void setupHelixSystemProperties() {
CommonConstants.Helix.DEFAULT_FLAPPING_TIME_WINDOW_MS));
}
+ private void setupHelixClusterConstraints() {
+ String maxMessageLimit = _config.getString(CommonConstants.Helix.CONFIG_OF_HELIX_INSTANCE_MAX_MESSAGES,
+ CommonConstants.Helix.DEFAULT_HELIX_INSTANCE_MAX_MESSAGES);
+ Map<ClusterConstraints.ConstraintAttribute, String> constraintAttributes = Maps.newHashMap();
+ constraintAttributes.put(ClusterConstraints.ConstraintAttribute.INSTANCE, ".*");
+ constraintAttributes.put(ClusterConstraints.ConstraintAttribute.MESSAGE_TYPE, "STATE_TRANSITION");
Review comment:
```suggestion
constraintAttributes.put(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_MESSAGES = "pinot.helix.instance.messages.max";
Review comment:
Recommend renaming it for clarity and add some comments on how it works (limit the maximum number of state transitions for each instance):
```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
##########
@@ -350,6 +377,9 @@ private void setUpPinotController() {
_realtimeSegmentsManager = null;
}
+ // setup constraints
+ setupHelixClusterConstraints();
Review comment:
We should not set this for Pinot controller but only for Helix controller
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org