junkaixue commented on code in PR #3017:
URL: https://github.com/apache/helix/pull/3017#discussion_r2027724281
##########
helix-core/src/main/java/org/apache/helix/util/InstanceUtil.java:
##########
@@ -148,17 +201,57 @@ public static List<InstanceConfig>
findInstancesWithMatchingLogicalId(
.collect(Collectors.toList());
}
+ /**
+ * Finds the instances that have a matching logical ID with the given
instance.
+ *
+ * @param clusterName The cluster name
+ * @param instanceConfig The instance configuration to match
+ * @return A list of matching instances
+ */
+ public static List<InstanceConfig> findInstancesWithMatchingLogicalId(
+ BaseDataAccessor<ZNRecord> baseDataAccessor, String clusterName,
+ InstanceConfig instanceConfig) {
+ HelixDataAccessor helixDataAccessor = new ZKHelixDataAccessor(clusterName,
baseDataAccessor);
+
+ ClusterConfig clusterConfig =
+
helixDataAccessor.getProperty(helixDataAccessor.keyBuilder().clusterConfig());
+ String logicalIdKey =
+
ClusterTopologyConfig.createFromClusterConfig(clusterConfig).getEndNodeType();
+
+ List<InstanceConfig> instanceConfigs =
+
helixDataAccessor.getChildValues(helixDataAccessor.keyBuilder().instanceConfigs(),
true);
+
+ // Retrieve and filter instances with matching logical ID
+ return instanceConfigs.stream().filter(potentialInstanceConfig ->
+
!potentialInstanceConfig.getInstanceName().equals(instanceConfig.getInstanceName())
+ && potentialInstanceConfig.getLogicalId(logicalIdKey)
+
.equals(instanceConfig.getLogicalId(logicalIdKey))).collect(Collectors.toList());
+ }
+
+ private static List<InstanceConfig> findInstancesWithMatchingLogicalId(
+ @Nullable BaseDataAccessor<ZNRecord> baseDataAccessor,
+ @Nullable ConfigAccessor configAccessor, String clusterName,
InstanceConfig instanceConfig) {
+ if (baseDataAccessor != null) {
+ return findInstancesWithMatchingLogicalId(baseDataAccessor, clusterName,
instanceConfig);
+ } else if (configAccessor != null) {
+ return findInstancesWithMatchingLogicalId(configAccessor, clusterName,
instanceConfig);
+ } else {
+ throw new HelixException(
+ "Both BaseDataAccessor and ConfigAccessor cannot be null at the same
time");
+ }
Review Comment:
A better way to make the code more clear is:
if (baseDataAccessor == null && configAccessor == null) {
throw new HelixException(
"Both BaseDataAccessor and ConfigAccessor cannot be null at the
same time");
}
return findInstancesWithMatchingLogicalId(baseDataAccessor != null ?
baseDataAccessor : configAccessor, clusterName, instanceConfig);
This simplify the branch of coding and also return the fail over logic at
first beginning with out further checks. Especially when you have function with
a lot of arguments repeatedly shows up, readability can be reduced.
--
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]
