Jackie-Jiang commented on a change in pull request #4995: Decouple server 
instance id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r371587599
 
 

 ##########
 File path: 
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
 ##########
 @@ -277,6 +277,26 @@ private void addInstanceTagIfNeeded(String clusterName, 
String instanceName) {
         _helixAdmin.addInstanceTag(clusterName, instanceName, 
UNTAGGED_SERVER_INSTANCE);
       }
     }
+
+    // If the server config has both instance_id and host/port info, overwrite 
the host/port info in zk. Without the
+    // overwrite, Helix will extract host/port from the instance_id instead of 
use those in config.
+    // Use serverConf instead of _serverConf as the latter has been modified.
+    if (serverConf.containsKey(CONFIG_OF_INSTANCE_ID) && 
serverConf.containsKey(KEY_OF_SERVER_NETTY_HOST)) {
+      // Internally, Helix use instanceId to derive Hostname and Port. To 
decouple them, explicitly set the hostname/port
+      // field in zk.
+      
instanceConfig.setHostName(_serverConf.getString(KEY_OF_SERVER_NETTY_HOST));
+      
instanceConfig.setPort(Integer.toString(_serverConf.getInt(KEY_OF_SERVER_NETTY_PORT,
 DEFAULT_SERVER_NETTY_PORT)));
+      // Use setProperty instead of _helixAdmin.setInstanceConfig because the 
latter explicitly forbids instance host
+      // port modification.
+      if(_helixManager.getHelixDataAccessor().setProperty(
+          
_helixManager.getHelixDataAccessor().keyBuilder().instanceConfig(instanceName), 
instanceConfig)) {
+        LOGGER.info("Updated server hostname/port successfully for server id 
{} to {}:", instanceName, instanceConfig);
+      } else {
+        LOGGER.error("Failed to update hostname/port for instance: {}", 
instanceName);
+        // Treat this is as a fatal error.
+        System.exit(1);
 
 Review comment:
   Don't use `System.exit()` for production code. Throwing an exception is good 
enough to abort

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to