[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336245939
##
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableReplica.java
##
@@ -151,22 +152,24 @@ public static String generateReplicaKey(String
resourceName, String partitionNam
Map partitionCapacity = capacityMap.get(partitionName);
if (partitionCapacity == null) {
- partitionCapacity =
capacityMap.get(ResourceConfig.DEFAULT_PARTITION_KEY);
+ partitionCapacity =
+ capacityMap.getOrDefault(ResourceConfig.DEFAULT_PARTITION_KEY, new
HashMap<>());
}
-if (partitionCapacity == null) {
- LOG.warn("The capacity usage of the specified partition {} is not
configured in the Resource"
- + " Config {}. No default partition capacity is configured either.
Will proceed with"
- + " empty capacity configuration.", partitionName,
resourceConfig.getResourceName());
- partitionCapacity = new HashMap<>();
+
+for (Map.Entry capacityEntry :
clusterConfig.getDefaultPartitionWeightMap()
+.entrySet()) {
+ partitionCapacity.putIfAbsent(capacityEntry.getKey(),
capacityEntry.getValue());
}
List requiredCapacityKeys =
clusterConfig.getInstanceCapacityKeys();
// Remove the non-required capacity items.
partitionCapacity.keySet().retainAll(requiredCapacityKeys);
-// If any required capacity key is not configured in the resource config,
fill the partition
-// capacity map with 0 usage.
-for (String capacityKey : requiredCapacityKeys) {
- partitionCapacity.putIfAbsent(capacityKey, 0);
+// If any required capacity key is not configured in the resource config,
fail the model creating.
+if (!partitionCapacity.keySet().containsAll(requiredCapacityKeys)) {
Review comment:
I think it would be better if we could do the check as early as possible to
skip any redundant computation.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336246498
##
File path:
helix-core/src/test/java/org/apache/helix/model/TestClusterConfig.java
##
@@ -166,19 +166,71 @@ public void testSetInstanceCapacityMap() {
DEFAULT_INSTANCE_CAPACITY_MAP.name()), capacityDataMapString);
}
- @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default Instance Capacity Data is empty")
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data is empty")
public void testSetInstanceCapacityMapEmpty() {
Map capacityDataMap = new HashMap<>();
ClusterConfig testConfig = new ClusterConfig("testConfig");
testConfig.setDefaultInstanceCapacityMap(capacityDataMap);
}
- @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default Instance Capacity Data contains a
negative value: item3 = -3")
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data contains a negative
value: item3 = -3")
public void testSetInstanceCapacityMapInvalid() {
Map capacityDataMap = ImmutableMap.of("item1", 1,
"item2", 2, "item3", -3);
ClusterConfig testConfig = new ClusterConfig("testConfig");
testConfig.setDefaultInstanceCapacityMap(capacityDataMap);
}
+
+ @Test
+ public void testGetPartitionWeightMap() {
+Map weightDataMap = ImmutableMap.of("item1", 1, "item2",
2, "item3", 3);
+
+Map weightDataMapString =
+ImmutableMap.of("item1", "1", "item2", "2", "item3", "3");
+
+ZNRecord rec = new ZNRecord("testId");
+
rec.setMapField(ClusterConfig.ClusterConfigProperty.DEFAULT_PARTITION_WEIGHT_MAP.name(),
+weightDataMapString);
+ClusterConfig testConfig = new ClusterConfig(rec);
+
+
Assert.assertTrue(testConfig.getDefaultPartitionWeightMap().equals(weightDataMap));
+ }
+
+ @Test
+ public void testGetPartitionWeightMapEmpty() {
+ClusterConfig testConfig = new ClusterConfig("testId");
+
+
Assert.assertTrue(testConfig.getDefaultPartitionWeightMap().equals(Collections.emptyMap()));
+ }
+
+ @Test
+ public void testSetPartitionWeightMap() {
+Map weightDataMap = ImmutableMap.of("item1", 1, "item2",
2, "item3", 3);
+
+Map weightDataMapString =
+ImmutableMap.of("item1", "1", "item2", "2", "item3", "3");
+
+ClusterConfig testConfig = new ClusterConfig("testConfig");
+testConfig.setDefaultPartitionWeightMap(weightDataMap);
+
+
Assert.assertEquals(testConfig.getRecord().getMapField(ClusterConfig.ClusterConfigProperty.
+DEFAULT_PARTITION_WEIGHT_MAP.name()), weightDataMapString);
+ }
+
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data is empty")
+ public void testSetPartitionWeightMapEmpty() {
+Map weightDataMap = new HashMap<>();
+
+ClusterConfig testConfig = new ClusterConfig("testConfig");
+testConfig.setDefaultPartitionWeightMap(weightDataMap);
+ }
Review comment:
If you allow an empty map, then I think it would work. I see the change
above. Thanks for making that change to allow an empty map to go through in the
set function.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336166711
##
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##
@@ -705,49 +706,78 @@ public void setInstanceCapacityKeys(List
capacityKeys) {
/**
* Get the default instance capacity information from the map fields.
+ *
* @return data map if it exists, or empty map
*/
public Map getDefaultInstanceCapacityMap() {
-Map capacityData =
-
_record.getMapField(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP.name());
-
-if (capacityData != null) {
- return capacityData.entrySet().stream().collect(
- Collectors.toMap(entry -> entry.getKey(), entry ->
Integer.parseInt(entry.getValue(;
-}
-return Collections.emptyMap();
+return
getDefaultCapacityMap(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP);
}
/**
* Set the default instance capacity information with an Integer mapping.
+ * This information is required by the global rebalancer.
+ * @see
+ *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
+ *
+ * If the instance capacity is not configured in either Instance Config nor
Cluster Config, the
+ * cluster topology is considered invalid. So the rebalancer may stop
working.
* @param capacityDataMap - map of instance capacity data
* @throws IllegalArgumentException - when any of the data value is a
negative number or when the map is empty
+ */
+ public void setDefaultInstanceCapacityMap(Map
capacityDataMap)
+ throws IllegalArgumentException {
+setDefaultCapacityMap(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP,
capacityDataMap);
+ }
+
+ /**
+ * Get the default partition weight information from the map fields.
*
+ * @return data map if it exists, or empty map
+ */
+ public Map getDefaultPartitionWeightMap() {
+return
getDefaultCapacityMap(ClusterConfigProperty.DEFAULT_PARTITION_WEIGHT_MAP);
+ }
+
+ /**
+ * Set the default partition weight information with an Integer mapping.
* This information is required by the global rebalancer.
* @see
- *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
- *
- * If the instance capacity is not configured in neither Instance Config nor
Cluster Config, the
+ *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
+ *
+ * If the partition weight is not configured in either Resource Config nor
Cluster Config, the
* cluster topology is considered invalid. So the rebalancer may stop
working.
+ * @param weightDataMap - map of partition weight data
+ * @throws IllegalArgumentException - when any of the data value is a
negative number or when the map is empty
*/
- public void setDefaultInstanceCapacityMap(Map
capacityDataMap)
+ public void setDefaultPartitionWeightMap(Map weightDataMap)
throws IllegalArgumentException {
-if (capacityDataMap == null || capacityDataMap.size() == 0) {
- throw new IllegalArgumentException("Default Instance Capacity Data is
empty");
-}
+setDefaultCapacityMap(ClusterConfigProperty.DEFAULT_PARTITION_WEIGHT_MAP,
weightDataMap);
+ }
-Map capacityData = new HashMap<>();
+ private Map getDefaultCapacityMap(ClusterConfigProperty
capacityPropertyType) {
+Map capacityData =
_record.getMapField(capacityPropertyType.name());
+if (capacityData != null) {
+ return capacityData.entrySet().stream().collect(
+ Collectors.toMap(entry -> entry.getKey(), entry ->
Integer.parseInt(entry.getValue(;
+}
+return Collections.emptyMap();
+ }
+ public void setDefaultCapacityMap(ClusterConfigProperty capacityPropertyType,
+ Map capacityDataMap) throws IllegalArgumentException {
+if (capacityDataMap == null || capacityDataMap.size() == 0) {
+ throw new IllegalArgumentException("Default capacity data is empty");
Review comment:
`capacityDataMap.size() == 0`
I think this might actually be a valid thing for users to do, so we
shouldn't throw an exception? Are they not allowed to empty out the default
capacity map? They should be allowed to for stricter capacity planning, correct?
Or we provide a separate method for `reset/removeDefaultCapacityMap`?
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
With regards,
Apache Git Services
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336163778
##
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableReplica.java
##
@@ -151,22 +152,24 @@ public static String generateReplicaKey(String
resourceName, String partitionNam
Map partitionCapacity = capacityMap.get(partitionName);
if (partitionCapacity == null) {
- partitionCapacity =
capacityMap.get(ResourceConfig.DEFAULT_PARTITION_KEY);
+ partitionCapacity =
+ capacityMap.getOrDefault(ResourceConfig.DEFAULT_PARTITION_KEY, new
HashMap<>());
}
-if (partitionCapacity == null) {
- LOG.warn("The capacity usage of the specified partition {} is not
configured in the Resource"
- + " Config {}. No default partition capacity is configured either.
Will proceed with"
- + " empty capacity configuration.", partitionName,
resourceConfig.getResourceName());
- partitionCapacity = new HashMap<>();
+
+for (Map.Entry capacityEntry :
clusterConfig.getDefaultPartitionWeightMap()
+.entrySet()) {
+ partitionCapacity.putIfAbsent(capacityEntry.getKey(),
capacityEntry.getValue());
}
List requiredCapacityKeys =
clusterConfig.getInstanceCapacityKeys();
// Remove the non-required capacity items.
partitionCapacity.keySet().retainAll(requiredCapacityKeys);
-// If any required capacity key is not configured in the resource config,
fill the partition
-// capacity map with 0 usage.
-for (String capacityKey : requiredCapacityKeys) {
- partitionCapacity.putIfAbsent(capacityKey, 0);
+// If any required capacity key is not configured in the resource config,
fail the model creating.
+if (!partitionCapacity.keySet().containsAll(requiredCapacityKeys)) {
+ throw new HelixException(String.format(
+ "The required capacity keys %s are not fully configured int the
resource %s partition %s weight map %s.",
Review comment:
int the resource -> typo
Also, I think it would be good if you could put ":" or something so we could
easily tell which one's a variable name. (like resource: %s, partition: %s,
etc..)
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336161572
##
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableReplica.java
##
@@ -151,22 +152,24 @@ public static String generateReplicaKey(String
resourceName, String partitionNam
Map partitionCapacity = capacityMap.get(partitionName);
if (partitionCapacity == null) {
- partitionCapacity =
capacityMap.get(ResourceConfig.DEFAULT_PARTITION_KEY);
+ partitionCapacity =
Review comment:
Let's clearly document the behavior here as a comment - I think what we used
to have as a warn log could be changed to an inline comment here. Also, let's
say that if partition capacity usage is not found in ResourceConfig, we will
try to get it from ClusterConfig.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336169212
##
File path:
helix-core/src/test/java/org/apache/helix/controller/rebalancer/waged/model/TestAssignableReplica.java
##
@@ -102,6 +103,24 @@ public void testConstructReplicaWithResourceConfig()
throws IOException {
Assert.assertEquals(replica.getResourceMaxPartitionsPerInstance(),
maxPartition);
}
+ @Test
Review comment:
Nit: I think adding some JavaDoc here would be nice:
E.g.) Tests that if default partition weight map is configured in
ClusterConfig and NOT in ResourceConfig, AssignableReplica actually will get
the default weight from ClusterConfig even though it's not set in
ResourceConfig.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336163339
##
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableReplica.java
##
@@ -151,22 +152,24 @@ public static String generateReplicaKey(String
resourceName, String partitionNam
Map partitionCapacity = capacityMap.get(partitionName);
if (partitionCapacity == null) {
- partitionCapacity =
capacityMap.get(ResourceConfig.DEFAULT_PARTITION_KEY);
+ partitionCapacity =
+ capacityMap.getOrDefault(ResourceConfig.DEFAULT_PARTITION_KEY, new
HashMap<>());
}
-if (partitionCapacity == null) {
- LOG.warn("The capacity usage of the specified partition {} is not
configured in the Resource"
- + " Config {}. No default partition capacity is configured either.
Will proceed with"
- + " empty capacity configuration.", partitionName,
resourceConfig.getResourceName());
- partitionCapacity = new HashMap<>();
+
+for (Map.Entry capacityEntry :
clusterConfig.getDefaultPartitionWeightMap()
+.entrySet()) {
+ partitionCapacity.putIfAbsent(capacityEntry.getKey(),
capacityEntry.getValue());
}
List requiredCapacityKeys =
clusterConfig.getInstanceCapacityKeys();
Review comment:
Would it be better to just iterate over `requiredCapacityKeys` in the first
place starting in line 159?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336170671
##
File path:
helix-core/src/test/java/org/apache/helix/model/TestClusterConfig.java
##
@@ -166,19 +166,71 @@ public void testSetInstanceCapacityMap() {
DEFAULT_INSTANCE_CAPACITY_MAP.name()), capacityDataMapString);
}
- @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default Instance Capacity Data is empty")
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data is empty")
public void testSetInstanceCapacityMapEmpty() {
Map capacityDataMap = new HashMap<>();
ClusterConfig testConfig = new ClusterConfig("testConfig");
testConfig.setDefaultInstanceCapacityMap(capacityDataMap);
}
- @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default Instance Capacity Data contains a
negative value: item3 = -3")
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data contains a negative
value: item3 = -3")
public void testSetInstanceCapacityMapInvalid() {
Map capacityDataMap = ImmutableMap.of("item1", 1,
"item2", 2, "item3", -3);
ClusterConfig testConfig = new ClusterConfig("testConfig");
testConfig.setDefaultInstanceCapacityMap(capacityDataMap);
}
+
+ @Test
+ public void testGetPartitionWeightMap() {
+Map weightDataMap = ImmutableMap.of("item1", 1, "item2",
2, "item3", 3);
+
+Map weightDataMapString =
+ImmutableMap.of("item1", "1", "item2", "2", "item3", "3");
+
+ZNRecord rec = new ZNRecord("testId");
+
rec.setMapField(ClusterConfig.ClusterConfigProperty.DEFAULT_PARTITION_WEIGHT_MAP.name(),
+weightDataMapString);
+ClusterConfig testConfig = new ClusterConfig(rec);
+
+
Assert.assertTrue(testConfig.getDefaultPartitionWeightMap().equals(weightDataMap));
+ }
+
+ @Test
+ public void testGetPartitionWeightMapEmpty() {
+ClusterConfig testConfig = new ClusterConfig("testId");
+
+
Assert.assertTrue(testConfig.getDefaultPartitionWeightMap().equals(Collections.emptyMap()));
+ }
+
+ @Test
+ public void testSetPartitionWeightMap() {
+Map weightDataMap = ImmutableMap.of("item1", 1, "item2",
2, "item3", 3);
+
+Map weightDataMapString =
+ImmutableMap.of("item1", "1", "item2", "2", "item3", "3");
+
+ClusterConfig testConfig = new ClusterConfig("testConfig");
+testConfig.setDefaultPartitionWeightMap(weightDataMap);
+
+
Assert.assertEquals(testConfig.getRecord().getMapField(ClusterConfig.ClusterConfigProperty.
+DEFAULT_PARTITION_WEIGHT_MAP.name()), weightDataMapString);
+ }
+
+ @Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Default capacity data is empty")
+ public void testSetPartitionWeightMapEmpty() {
+Map weightDataMap = new HashMap<>();
+
+ClusterConfig testConfig = new ClusterConfig("testConfig");
+testConfig.setDefaultPartitionWeightMap(weightDataMap);
+ }
Review comment:
Do we allow users to reset or remove? Or would they have to explicitly set
everything to 0 (which could be quite tedious..)
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r336163644
##
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableReplica.java
##
@@ -151,22 +152,24 @@ public static String generateReplicaKey(String
resourceName, String partitionNam
Map partitionCapacity = capacityMap.get(partitionName);
if (partitionCapacity == null) {
- partitionCapacity =
capacityMap.get(ResourceConfig.DEFAULT_PARTITION_KEY);
+ partitionCapacity =
+ capacityMap.getOrDefault(ResourceConfig.DEFAULT_PARTITION_KEY, new
HashMap<>());
}
-if (partitionCapacity == null) {
- LOG.warn("The capacity usage of the specified partition {} is not
configured in the Resource"
- + " Config {}. No default partition capacity is configured either.
Will proceed with"
- + " empty capacity configuration.", partitionName,
resourceConfig.getResourceName());
- partitionCapacity = new HashMap<>();
+
+for (Map.Entry capacityEntry :
clusterConfig.getDefaultPartitionWeightMap()
+.entrySet()) {
+ partitionCapacity.putIfAbsent(capacityEntry.getKey(),
capacityEntry.getValue());
}
List requiredCapacityKeys =
clusterConfig.getInstanceCapacityKeys();
// Remove the non-required capacity items.
partitionCapacity.keySet().retainAll(requiredCapacityKeys);
-// If any required capacity key is not configured in the resource config,
fill the partition
-// capacity map with 0 usage.
-for (String capacityKey : requiredCapacityKeys) {
- partitionCapacity.putIfAbsent(capacityKey, 0);
+// If any required capacity key is not configured in the resource config,
fail the model creating.
+if (!partitionCapacity.keySet().containsAll(requiredCapacityKeys)) {
Review comment:
Could we do this check before
`partitionCapacity.keySet().retainAll(requiredCapacityKeys);`?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r334763945
##
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##
@@ -705,49 +706,80 @@ public void setInstanceCapacityKeys(List
capacityKeys) {
/**
* Get the default instance capacity information from the map fields.
+ *
* @return data map if it exists, or empty map
*/
public Map getDefaultInstanceCapacityMap() {
-Map capacityData =
-
_record.getMapField(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP.name());
-
-if (capacityData != null) {
- return capacityData.entrySet().stream().collect(
- Collectors.toMap(entry -> entry.getKey(), entry ->
Integer.parseInt(entry.getValue(;
-}
-return Collections.emptyMap();
+return
getDefaultCapacityMap(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP);
}
/**
* Set the default instance capacity information with an Integer mapping.
+ *
* @param capacityDataMap - map of instance capacity data
* @throws IllegalArgumentException - when any of the data value is a
negative number or when the map is empty
- *
- * This information is required by the global rebalancer.
+ * This information is required by the
global rebalancer.
* @see
- *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
- *
+ *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
+ *
* If the instance capacity is not configured in neither Instance Config nor
Cluster Config, the
Review comment:
neither -> either
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #511: More strict partition weight validation while creating the cluster model.
narendly commented on a change in pull request #511: More strict partition
weight validation while creating the cluster model.
URL: https://github.com/apache/helix/pull/511#discussion_r334764101
##
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##
@@ -705,49 +706,80 @@ public void setInstanceCapacityKeys(List
capacityKeys) {
/**
* Get the default instance capacity information from the map fields.
+ *
* @return data map if it exists, or empty map
*/
public Map getDefaultInstanceCapacityMap() {
-Map capacityData =
-
_record.getMapField(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP.name());
-
-if (capacityData != null) {
- return capacityData.entrySet().stream().collect(
- Collectors.toMap(entry -> entry.getKey(), entry ->
Integer.parseInt(entry.getValue(;
-}
-return Collections.emptyMap();
+return
getDefaultCapacityMap(ClusterConfigProperty.DEFAULT_INSTANCE_CAPACITY_MAP);
}
/**
* Set the default instance capacity information with an Integer mapping.
+ *
* @param capacityDataMap - map of instance capacity data
* @throws IllegalArgumentException - when any of the data value is a
negative number or when the map is empty
- *
- * This information is required by the global rebalancer.
+ * This information is required by the
global rebalancer.
* @see
- *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
- *
+ *
https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter
+ *
* If the instance capacity is not configured in neither Instance Config nor
Cluster Config, the
* cluster topology is considered invalid. So the rebalancer may stop
working.
Review comment:
Is this description valid anymore? We aren't throwing an exception anymore.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
