junkaixue commented on code in PR #2772:
URL: https://github.com/apache/helix/pull/2772#discussion_r1525298995
##########
helix-core/src/main/java/org/apache/helix/controller/dataproviders/BaseControllerDataProvider.java:
##########
@@ -722,78 +698,37 @@ public Set<String> getAllInstances() {
}
/**
- * Return all the live nodes that are enabled and assignable
- *
- * @return A new set contains live instance name and that are marked enabled
- */
- public Set<String> getAssignableEnabledLiveInstances() {
- Set<String> enabledLiveInstances = new
HashSet<>(getAssignableLiveInstances().keySet());
- enabledLiveInstances.removeAll(getDisabledInstances());
-
- return enabledLiveInstances;
- }
-
- /**
- * Return all the live nodes that are enabled
+ * Return all the live nodes that are enabled. If a node is enabled, it is
assignable.
* @return A new set contains live instance name and that are marked enabled
*/
public Set<String> getEnabledLiveInstances() {
Set<String> enabledLiveInstances = new
HashSet<>(getLiveInstances().keySet());
- enabledLiveInstances.removeAll(getDisabledInstances());
+ enabledLiveInstances.retainAll(getEnabledInstances());
return enabledLiveInstances;
}
/**
- * Return all nodes that are enabled and assignable.
- *
- * @return A new set contains instance name and that are marked enabled
- */
- public Set<String> getAssignableEnabledInstances() {
- Set<String> enabledNodes = new HashSet<>(getAssignableInstances());
- enabledNodes.removeAll(getDisabledInstances());
-
- return enabledNodes;
- }
-
- /**
- * Return all nodes that are enabled.
+ * Return all nodes that are enabled. If a node is enabled, it is assignable.
* @return A new set contains instance name and that are marked enabled
*/
public Set<String> getEnabledInstances() {
- Set<String> enabledNodes = new HashSet<>(getAllInstances());
- enabledNodes.removeAll(getDisabledInstances());
-
- return enabledNodes;
+ return new
HashSet<>(_derivedInstanceCache.getInstanceConfigMapByInstanceOperation(
+ InstanceConstants.InstanceOperation.ENABLE).keySet());
}
/**
- * Return all the live nodes that are enabled and assignable and tagged with
given instanceTag.
Review Comment:
This cannot be simply removed. We need support backward compatible. The new
client can just deprecate all enableInstances API.
But for controller it should understand the enable field for a while for
backward compatible. When we remove those deprecated APIs, we can change this
logic as well.
--
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]
