gortiz commented on code in PR #15595:
URL: https://github.com/apache/pinot/pull/15595#discussion_r2051430988
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/HelixSetupUtils.java:
##########
@@ -60,39 +61,56 @@ private HelixSetupUtils() {
private static final Logger LOGGER =
LoggerFactory.getLogger(HelixSetupUtils.class);
- public static HelixManager setupHelixController(String helixClusterName,
String zkPath, String instanceId) {
- setupHelixClusterIfNeeded(helixClusterName, zkPath);
- return HelixControllerMain
- .startHelixController(zkPath, helixClusterName, instanceId,
HelixControllerMain.STANDALONE);
- }
-
- private static void setupHelixClusterIfNeeded(String helixClusterName,
String zkPath) {
- HelixAdmin admin = null;
+ public static void setupHelixClusterWithDefaultConfigs(String zkAddress,
String clusterName,
Review Comment:
I think it will be great to add some javadoc explaining how configs are
going to be used. In fact, reading the code, it is not clear to me.
IICU, if the cluster doesn't exist, this map will be used as the default
config. That is simple, although I think there could be a race condition if two
controllers with different default configs start at the same time. Given the
probability of that happening is very small, I'm ok with that.
When it becomes more complicated is when the cluster was already created and
the defaultConfigs are not equal to the ones already stored in the cluster. In
that case:
1. Properties that are in `defaultConfig` but not in the cluster: The value
from this instance is set in the cluster. I assume other instances will see the
change but may not react to it until next restart.
2. Properties that are in both maps but have different values: The value
from this instance is ignored.
Is that correct? In that case I would recommend to change the log from
`non-default configs` to `ignored configs` or something like that.
--
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
