NealSun96 commented on a change in pull request #1129:
URL: https://github.com/apache/helix/pull/1129#discussion_r458464421
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/topology/Topology.java
##########
@@ -204,8 +172,9 @@ private Node createClusterTree() {
InstanceConfig insConfig = _instanceConfigMap.get(instanceName);
try {
LinkedHashMap<String, String> instanceTopologyMap =
-
computeInstanceTopologyMap(_clusterConfig.isTopologyAwareEnabled(),
instanceName,
- insConfig, _clusterTopologyKeys);
+
computeInstanceTopologyMapHelper(_clusterConfig.isTopologyAwareEnabled(),
instanceName,
Review comment:
I believe you meant to call `computeInstanceTopologyMap()` with a false
in the end here, instead of the concrete function itself?
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/topology/Topology.java
##########
@@ -230,20 +199,73 @@ private boolean isInstanceEnabled(ClusterConfig
clusterConfig, String instanceNa
|| !clusterConfig.getDisabledInstances().containsKey(instanceName)));
}
+ /**
+ * Populate faultZone, endNodetype and and a LinkedHashMap containing
pathKeys default values for
+ * clusterConfig.Topology. The LinkedHashMap will be empty if
clusterConfig.Topology is unset.
+ *
+ * @return an Instance of Topology.ClusterTopologyConfig.
+ */
+ private static ClusterTopologyConfig getClusterTopologySetting(ClusterConfig
clusterConfig) {
+
+ ClusterTopologyConfig clusterTopologyConfig = new ClusterTopologyConfig();
+ if (clusterConfig.isTopologyAwareEnabled()) {
+ String topologyDef = clusterConfig.getTopology();
+ if (topologyDef != null) {
+ String[] topologyKeys = topologyDef.trim().split("/");
+ int lastValidTypeIdx = 0;
+ for (int i = 0; i < topologyKeys.length; i++) {
+ if (topologyKeys[i].length() != 0) {
+ clusterTopologyConfig.topologyKeyDefaultValue
+ .put(topologyKeys[i], DEFAULT_DOMAIN_PREFIX + topologyKeys[i]);
+ lastValidTypeIdx = i;
+ }
+ }
+ if (clusterTopologyConfig.topologyKeyDefaultValue.size() == 0) {
+ throw new IllegalArgumentException("Invalid cluster topology
definition " + topologyDef);
+ }
+ clusterTopologyConfig.endNodeType = topologyKeys[lastValidTypeIdx];
+ String faultZoneType = clusterConfig.getFaultZoneType();
+ if (faultZoneType == null) {
+ clusterTopologyConfig.faultZoneType =
clusterTopologyConfig.endNodeType;
+ } else if
(!clusterTopologyConfig.topologyKeyDefaultValue.containsKey(faultZoneType)) {
+ throw new HelixException(String
+ .format("Invalid fault zone type %s, not present in topology
definition %s.",
+ faultZoneType, clusterConfig.getTopology()));
+ } else {
+ clusterTopologyConfig.faultZoneType = faultZoneType;
+ }
+ } else {
+ // Use default cluster topology definition, i,e. /root/zone/instance
+ clusterTopologyConfig.endNodeType = Types.INSTANCE.name();
+ clusterTopologyConfig.faultZoneType = Types.ZONE.name();
+ }
+ } else {
+ clusterTopologyConfig.endNodeType = Types.INSTANCE.name();
+ clusterTopologyConfig.faultZoneType = Types.INSTANCE.name();
+ }
+ return clusterTopologyConfig;
+ }
+
/**
* This function returns a LinkedHashMap<String, String> object representing
* the topology path for an instance.
* LinkedHashMap is used here since the order of the path needs to be
preserved
* when creating the topology tree.
*
+ * @param clusterTopologyKeyDefaultValue a LinkedHashMap where keys are
cluster topology path and
+ * values are their corresponding
default value. The entries
+ * are ordered by
ClusterConfig.topology setting.
+ * @param faultZoneForEarlyQuit this flag is set to true only if caller
wants the path
Review comment:
This comment should be for `computeInstanceTopologyMap()`? That public
function needs an entire javadoc also.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
