junkaixue commented on code in PR #2680:
URL: https://github.com/apache/helix/pull/2680#discussion_r1372495856
##########
helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java:
##########
@@ -235,7 +239,22 @@ private Response batchGetStoppableInstances(String
clusterId, JsonNode node, boo
OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class,
String.class));
if (!orderOfZone.isEmpty() && random) {
String message =
- "Both 'orderOfZone' and 'random' parameters are set. Please
specify only one option.";
+ "Both 'zone_order' and 'random' parameters are set. Please
specify only one option.";
+ _logger.error(message);
+ return badRequest(message);
+ }
+ }
+
+ if
(node.get(InstancesAccessor.InstancesProperties.to_be_stopped_instances.name())
!= null) {
Review Comment:
Are we assuming this will be something passed by user? If it is already
reconstructed by our code, then we don't need this check.
##########
helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/StoppableInstancesSelector.java:
##########
@@ -118,10 +152,22 @@ public void getStoppableInstancesCrossZones() {
*
* @param random Indicates whether to randomize the order of zones.
*/
- public void calculateOrderOfZone(boolean random) {
+ public void calculateOrderOfZone(List<String> instances, boolean random) {
Review Comment:
I don't see any place to use this random
##########
helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java:
##########
@@ -235,7 +239,22 @@ private Response batchGetStoppableInstances(String
clusterId, JsonNode node, boo
OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class,
String.class));
if (!orderOfZone.isEmpty() && random) {
Review Comment:
This will break the backward compatibility. User can have non zone selected
by let us to choose in order first one.
##########
helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java:
##########
@@ -370,7 +370,8 @@ public static boolean isInstanceStable(HelixDataAccessor
dataAccessor, String in
* @param instanceName
Review Comment:
Add java doc to explain what the to be stopped instances are.
--
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]
