narendly commented on a change in pull request #1033:
URL: https://github.com/apache/helix/pull/1033#discussion_r431483789
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
##########
@@ -1256,14 +1256,18 @@ protected void
checkLiveInstancesObservation(List<LiveInstance> liveInstances,
for (String instance : curInstances.keySet()) {
if (lastInstances == null || !lastInstances.containsKey(instance)) {
- try {
- manager.addCustomizedStateRootChangeListener(this, instance);
- logger.info(manager.getInstanceName() + " added customized root
change listener for"
- + " " + instance
- + ", listener: " + this);
- } catch (Exception e) {
- logger.error("Fail to add customized root change listener for
instance: " + instance,
- e);
+ PropertyKey propertyKey =
+ keyBuilder.customizedStatesRoot(instance);
+ if (propertyKey.getPath() != null) {
Review comment:
You're creating a PropertyKey object with the given instance name and
checking whether it's null? How would it be null if you've just created the
path? In other words, when would this condition ever evaluate to false?
Did you mean to check whether the path exists in ZooKeeper? If so, shouldn't
you do an exist() check with a ZkClient-based API?
Unless I'm mistaken and we actually somehow pre-populate the KeyBuilder with
a whitelist of instances for the Customized State feature and returns null if
the instance doesn't show up in the list?
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
