MarkGaox commented on code in PR #2635:
URL: https://github.com/apache/helix/pull/2635#discussion_r1337921496
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedInstanceCapacity.java:
##########
@@ -54,7 +55,15 @@ public WagedInstanceCapacity(ResourceControllerDataProvider
clusterData) {
return;
}
for (InstanceConfig instanceConfig :
clusterData.getInstanceConfigMap().values()) {
- Map<String, Integer> instanceCapacity =
WagedValidationUtil.validateAndGetInstanceCapacity(clusterConfig,
instanceConfig);
+ Map<String, Integer> instanceCapacity;
+ try {
+ instanceCapacity =
WagedValidationUtil.validateAndGetInstanceCapacity(clusterConfig,
instanceConfig);
+ } catch (HelixException ex) {
+ // We don't want to throw exception here, it would be OK if no
resource is using Waged.
+ // Waged rebalancer will fail in later pipeline stage only for waged
resource. So it won't block other resources.
+ LOG.error("Failed to initialize instance capacity map. Instance
capacity map is not property set up", ex);
+ return;
Review Comment:
+1 here. This is under the assumption that if the instanceCapacity is not
available then `getInstanceAvailableCapacity`, `isInstanceCapacityAvailable`,
and `checkAndReduceInstanceCapacity` won't be called. It's risky. Could replace
all `_instanceCapacityMap.get()` with `getOrDefault()` to give instances that
doesn't exist in INSTANCE_CAPACITY_MAP a default value of 0. In this way, if
the INSTANCE_CAPACITY_MAP is not available, no WAGED-related resources can
allocate partition on this instance.
--
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]
