Jackie-Jiang commented on a change in pull request #4929: Fix the issue of 
server not registering state model factory before connecting the Helix manager
URL: https://github.com/apache/incubator-pinot/pull/4929#discussion_r359101097
 
 

 ##########
 File path: 
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
 ##########
 @@ -121,35 +120,33 @@ public HelixServerStarter(String helixClusterName, 
String zkServer, Configuratio
       _serverConf.addProperty(CONFIG_OF_INSTANCE_ID, _instanceId);
     }
 
-    LOGGER.info("Connecting Helix components");
+    LOGGER.info("Initializing Helix manager");
     setupHelixSystemProperties();
     // Replace all white-spaces from list of zkServers.
     _zkServers = zkServer.replaceAll("\\s+", "");
     _helixManager =
         HelixManagerFactory.getZKHelixManager(helixClusterName, _instanceId, 
InstanceType.PARTICIPANT, _zkServers);
-    _helixManager.connect();
-    _helixAdmin = _helixManager.getClusterManagmentTool();
-    addInstanceTagIfNeeded(helixClusterName, _instanceId);
-    ZkHelixPropertyStore<ZNRecord> propertyStore = 
_helixManager.getHelixPropertyStore();
 
-    LOGGER.info("Starting server instance");
+    LOGGER.info("Initializing server instance and registering state model 
factory");
     Utils.logVersions();
-    ServerConf serverInstanceConfig = 
DefaultHelixStarterServerConfig.getDefaultHelixServerConfig(_serverConf);
     // Need to do this before we start receiving state transitions.
     ServerSegmentCompletionProtocolHandler
         
.init(_serverConf.subset(SegmentCompletionProtocol.PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER));
-    _serverInstance = new ServerInstance();
-    _serverInstance.init(serverInstanceConfig, propertyStore);
-    _serverInstance.start();
-
-    // Register state model factory
-    SegmentFetcherAndLoader fetcherAndLoader =
-        new SegmentFetcherAndLoader(_serverConf, 
_serverInstance.getInstanceDataManager(), propertyStore);
+    ServerConf serverInstanceConfig = 
DefaultHelixStarterServerConfig.getDefaultHelixServerConfig(_serverConf);
+    _serverInstance = new ServerInstance(serverInstanceConfig, _helixManager);
+    InstanceDataManager instanceDataManager = 
_serverInstance.getInstanceDataManager();
+    SegmentFetcherAndLoader fetcherAndLoader = new 
SegmentFetcherAndLoader(_serverConf, instanceDataManager);
     StateModelFactory<?> stateModelFactory =
-        new SegmentOnlineOfflineStateModelFactory(_instanceId, 
_serverInstance.getInstanceDataManager(),
-            fetcherAndLoader, propertyStore);
+        new SegmentOnlineOfflineStateModelFactory(_instanceId, 
instanceDataManager, fetcherAndLoader);
     _helixManager.getStateMachineEngine()
         
.registerStateModelFactory(SegmentOnlineOfflineStateModelFactory.getStateModelName(),
 stateModelFactory);
+    // Start the server instance after connecting to the ZK but before 
processing state transitions
 
 Review comment:
   Done

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