jackjlli commented on code in PR #8820:
URL: https://github.com/apache/pinot/pull/8820#discussion_r888402755
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -751,6 +751,20 @@ public ServerInstance getServerInstance() {
return _serverInstance;
}
+ /**
+ * Helper method to set system resource info into instance config.
+ *
+ * @param helixAdmin Helix Admin
+ * @param helixClusterName Name of Helix cluster
+ * @param instanceId Id of instance for which to set the system resource info
+ * @param systemResourceMap Map containing system resource info
+ */
+ protected void setInstanceResourceInfo(HelixAdmin helixAdmin, String
helixClusterName, String instanceId,
+ Map<String, String> systemResourceMap) {
+ InstanceConfig instanceConfig =
helixAdmin.getInstanceConfig(helixClusterName, instanceId);
Review Comment:
You don't need to fetch instance config again. Please refer to the other
logic in the same method.
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -346,7 +346,7 @@ private void updateInstanceConfigIfNeeded(ServerConf
serverConf) {
Map<String, String> systemResourceInfoMap = new
SystemResourceInfo().toMap();
if
(!systemResourceInfoMap.equals(znRecord.getMapField(Instance.SYSTEM_RESOURCE_INFO_KEY)))
{
Review Comment:
The existing map from ZNRecord can contain more information than the default
map generated from `SystemResourceInfo` class. It'd be good to check whether
the new kv pairs from the new map equal to the ones appeared from ZNRecord.
--
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]
