dongxiaoman commented on a change in pull request #7064:
URL: https://github.com/apache/incubator-pinot/pull/7064#discussion_r660199645



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##########
@@ -495,4 +499,104 @@ public IdealState apply(@Nullable IdealState idealState) {
       String tenant) {
     return new HashSet<>(getInstancesConfigsWithTag(instanceConfigs, 
TagNameUtils.getBrokerTagForTenant(tenant)));
   }
+
+  /**
+   * Add default tags to instance if the instance has no tags Note for the 
getDefaultTags: it is a
+   * lambda so it may not be invoked if instance already has tags. * For 
example () ->
+   * ImmutableList.of("Default_tenant")
+   *
+   * @param helixManager The helix manager for update
+   * @param instanceId the Helix instance Id
+   * @param getDefaultTags the default tags lambda. Something like () -> 
ImmutableList.of("Default_tenant") to provide default tags
+   */
+  public static void addDefaultTags(
+      HelixManager helixManager, String instanceId, Supplier<List<String>> 
getDefaultTags) {
+    InstanceConfig instanceConfig = getInstanceConfig(helixManager, 
instanceId);
+    // Add default instance tags if not exist
+    List<String> instanceTags = instanceConfig.getTags();
+    if (instanceTags == null || instanceTags.size() == 0) {
+      List<String> defaultTags = getDefaultTags == null ? null : 
getDefaultTags.get();
+      if (!CollectionUtils.isEmpty(defaultTags)) {
+        defaultTags.forEach(instanceConfig::addTag);
+        LOGGER.info("Updating instance tags {} for instance: {}", 
instanceTags, instanceId);
+        updateInstanceConfig(helixManager, instanceConfig);
+      }
+    }
+  }
+
+  /**
+   * Returns the instance config for a specific instance ID
+   * @param helixManager the Helix manager
+   * @param instanceId the unique ID for instance
+   * @return An InstanceConfig that we can update
+   */
+  public static InstanceConfig getInstanceConfig(HelixManager helixManager, 
String instanceId) {
+    HelixAdmin admin = helixManager.getClusterManagmentTool();
+    String clusterName = helixManager.getClusterName();
+    return admin.getInstanceConfig(clusterName, instanceId);
+  }
+
+  /**
+   * Update instance config into Helix properly
+   * @param helixManager the HelixManager for access
+   * @param instanceConfig the updated Helix Config
+   */
+  public static void updateInstanceConfig(HelixManager helixManager, 
InstanceConfig instanceConfig) {
+    // NOTE: Use HelixDataAccessor.setProperty() instead of 
HelixAdmin.setInstanceConfig() because the latter explicitly
+    // forbids instance host/port modification
+    HelixDataAccessor helixDataAccessor = helixManager.getHelixDataAccessor();
+    Preconditions.checkState(
+        
helixDataAccessor.setProperty(helixDataAccessor.keyBuilder().instanceConfig(instanceConfig.getId()),
 instanceConfig),
+        "Failed to update instance config");
+  }
+
+  /**
+   * Update Helix Host and Name if the values are reasonable.
+   * @param helixManager the Helix Manager to get control
+   * @param instanceId the Helix instance Id
+   * @param hostName the Host name
+   * @param hostPort the Host port
+   * @return true if it is updated
+   */
+  public static boolean updateHostNamePort(HelixManager helixManager, String 
instanceId, String hostName, int hostPort) {
+    InstanceConfig instanceConfig = getInstanceConfig(helixManager, 
instanceId);
+    if (Strings.isNullOrEmpty(hostName)) {
+      LOGGER.info("host is empty, skip updating helix host.");
+      return false;
+    }
+    boolean updated = false;
+    // Update host and port if needed
+    if (!String.valueOf(hostPort).equals(instanceConfig.getPort())) {
+      instanceConfig.setPort(String.valueOf(hostPort));
+      updated = true;
+    }
+    if (!hostName.equals(instanceConfig.getHostName())) {
+      instanceConfig.setHostName(hostName);
+      updated = true;
+    }
+    updateInstanceConfig(helixManager, instanceConfig);
+    return updated;
+  }
+  /**
+   * Update Helix Host and Name if the values are reasonable.
+   * @param helixManager the Helix Manager to get control
+   * @param instanceId the Helix instance Id
+   * @param hostName the Host name
+   * @param hostPort the Host port
+   * @return true if it is updated
+   */
+  public static boolean updateHostNamePort(HelixManager helixManager, String 
instanceId, String hostName, String hostPort) {
+    if (Strings.isNullOrEmpty(hostPort)) {

Review comment:
       Right, all changed to `log.warn` which makes more sense




-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to