rahulrane50 commented on code in PR #2340:
URL: https://github.com/apache/helix/pull/2340#discussion_r1073928319
##########
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/ResourceMonitor.java:
##########
@@ -128,6 +129,8 @@ public ResourceMonitor(String clusterName, String
resourceName, ObjectName objec
_numOfPartitionsInExternalView = new
SimpleDynamicMetric("ExternalViewPartitionGauge", 0L);
_numOfPartitions = new SimpleDynamicMetric("PartitionGauge", 0L);
_numPendingStateTransitions = new
SimpleDynamicMetric("PendingStateTransitionGauge", 0L);
+ _rebalanceThrottledByErrorPartitionGauge =
Review Comment:
I think you missed to reset it's value in resetResourceStateGauges() method?
##########
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/ResourceMonitor.java:
##########
@@ -371,11 +374,12 @@ public void updateStateHandoffStats(MonitorState
monitorState, long totalDuratio
public void updateRebalancerStats(long numPendingRecoveryRebalancePartitions,
long numPendingLoadRebalancePartitions, long
numRecoveryRebalanceThrottledPartitions,
- long numLoadRebalanceThrottledPartitions) {
+ long numLoadRebalanceThrottledPartitions, boolean
rebalanceThrottledByErrorPartitio) {
Review Comment:
neat typo: rebalanceThrottledByErrorPartitions
##########
helix-core/src/test/java/org/apache/helix/controller/stages/TestIntermediateStateCalcStage.java:
##########
@@ -272,6 +273,128 @@ public void testWithClusterConfigChange() {
}
}
+ @Test
+ public void testThrottleByErrorPartition() {
+ String resourcePrefix = "resource";
+ int nResource = 3;
+ int nPartition = 3;
+ int nReplica = 3;
+
+ String[] resources = new String[nResource];
+ for (int i = 0; i < nResource; i++) {
+ resources[i] = resourcePrefix + "_" + i;
+ }
+
+ preSetup(resources, nReplica, nReplica);
+ event.addAttribute(AttributeName.RESOURCES.name(),
+ getResourceMap(resources, nPartition, "OnlineOffline"));
+ event.addAttribute(AttributeName.RESOURCES_TO_REBALANCE.name(),
+ getResourceMap(resources, nPartition, "OnlineOffline"));
+ ClusterStatusMonitor monitor = new ClusterStatusMonitor(_clusterName);
+ monitor.active();
+ event.addAttribute(AttributeName.clusterStatusMonitor.name(), monitor);
+
+ // Initialize best possible state and current state
+ BestPossibleStateOutput bestPossibleStateOutput = new
BestPossibleStateOutput();
+ MessageOutput messageSelectOutput = new MessageOutput();
+ CurrentStateOutput currentStateOutput = new CurrentStateOutput();
+ IntermediateStateOutput expectedResult = new IntermediateStateOutput();
+
+ _clusterConfig.setErrorOrRecoveryPartitionThresholdForLoadBalance(0);
Review Comment:
For my understanding, we have set this value 0 here mean there always be
downward rebalance because all positive values values will be greater than 0.
If that's the correct understanding then don't we want to test for positive
threshold value?
--
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: reviews-unsubscr...@helix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
