junkaixue commented on code in PR #2546:
URL: https://github.com/apache/helix/pull/2546#discussion_r1259101535
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/WagedRebalanceUtil.java:
##########
@@ -61,7 +62,11 @@ public static Map<String, Integer> fetchCapacityUsage(String
partitionName,
ResourceConfig resourceConfig, ClusterConfig clusterConfig) {
Map<String, Map<String, Integer>> capacityMap;
try {
- capacityMap = resourceConfig.getPartitionCapacityMap();
+ if (resourceConfig != null) {
+ capacityMap = resourceConfig.getPartitionCapacityMap();
+ } else {
+ capacityMap = new HashMap<>();
+ }
Review Comment:
NIT: can be simplified as to one-liner
capacityMap = resourceConfig == null ? new HashMap<>() :
resourceConfig.getPartitionCapacityMap();
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedInstanceCapacity.java:
##########
@@ -42,79 +49,125 @@ public class WagedInstanceCapacity implements
InstanceCapacityDataProvider {
// Available Capacity per Instance
private final Map<String, Map<String, Integer>> _instanceCapacityMap;
- private final ResourceControllerDataProvider _cache;
+ private final Map<String, Map<String, Set<String>>> _allocatedPartitionsMap;
public WagedInstanceCapacity(ResourceControllerDataProvider clusterData) {
- _cache = clusterData;
_instanceCapacityMap = new HashMap<>();
-
- ClusterConfig clusterConfig = _cache.getClusterConfig();
- for (InstanceConfig instanceConfig :
_cache.getInstanceConfigMap().values()) {
- Map<String, Integer> instanceCapacity =
- WagedValidationUtil.validateAndGetInstanceCapacity(clusterConfig,
instanceConfig);
+ _allocatedPartitionsMap = new HashMap<>();
+ ClusterConfig clusterConfig = clusterData.getClusterConfig();
+ if (clusterConfig == null) {
+ LOG.error("Cluster config is null, cannot initialize instance capacity
map.");
+ return;
+ }
+ for (InstanceConfig instanceConfig :
clusterData.getInstanceConfigMap().values()) {
+ Map<String, Integer> instanceCapacity =
WagedValidationUtil.validateAndGetInstanceCapacity(clusterConfig,
instanceConfig);
_instanceCapacityMap.put(instanceConfig.getInstanceName(),
instanceCapacity);
+ _allocatedPartitionsMap.put(instanceConfig.getInstanceName(), new
HashMap<>());
}
}
- /**
- * Create Default Capacity Map.
- * This is a utility method to create a default capacity map matching
instance capacity map for participants.
- * This is required as non-WAGED partitions will be placed on same instance
and we don't know their actual capacity.
- * This will generate default values of 0 for all the capacity keys.
- */
- private Map<String, Integer> createDefaultParticipantWeight() {
- // copy the value of first Instance capacity.
- Map<String, Integer> partCapacity = new
HashMap<>(_instanceCapacityMap.values().iterator().next());
-
- // Set the value of all capacity to -1.
- for (String key : partCapacity.keySet()) {
- partCapacity.put(key, -1);
+ // Helper methods.
+ private boolean isPartitionInAllocatedMap(String instance, String resource,
String partition) {
Review Comment:
I am OK with this as temporary protective method. Long run, we should based
on the idea behavior construct the logic. Im not sure whether we have the
scenario need to handle double counting. Suggest to have a TODO mark here for
future clean up the logic.
NIT: would be a better naming with "hasPartitionChargedForCapacity"
--
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]
