Support for Helix Auto Registration #2982
Conversation
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Outdated
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Outdated
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Outdated
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Outdated
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Show resolved
Hide resolved
| @@ -96,7 +101,14 @@ public DataNodeConfigSource getDataNodeConfigSource(ClusterMapConfig clusterMapC | |||
| * @return a new instance of {@link HelixManager}. | |||
| */ | |||
| HelixManager buildZKHelixManager(String clusterName, String instanceName, InstanceType instanceType, String zkAddr) { | |||
| return HelixManagerFactory.getZKHelixManager(clusterName, instanceName, instanceType, zkAddr); | |||
| HelixManagerProperty participantHelixProperty = new HelixManagerProperty.Builder().setDefaultInstanceConfigBuilder( | |||
There was a problem hiding this comment.
You need to create a flag in config to switch b/w old logic and new logic.
There was a problem hiding this comment.
Also you can add a comment explaining what is autoregistration.
There was a problem hiding this comment.
There is no need for flag here as we are not changing the way we connect to helix. In this we are providing an InstanceConfig as extra configuration while connection
There was a problem hiding this comment.
This change will be tested in EI for sometime. We will need prod deployments to go through while wait for validation. And if we encounter some issue, we can just turn off the the flag.
There was a problem hiding this comment.
There is no auto registration at this point. Auto registration will have two requisites after this enabling cloud config and taking dependency on helix-cloud in closed.
Now for this specific change, it is change to the factory method on how we instantiate the HelixManager. I have added an additional test as safety check.
I don't think there should be any issue due to this but I can hold off from merging until I test on perf
There was a problem hiding this comment.
sure, check if this is not getting used in VCR too.
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/HelixFactory.java
Outdated
Show resolved
Hide resolved
ambry-clustermap/src/main/java/com/github/ambry/clustermap/ClusterMapUtils.java
Show resolved
Hide resolved
Summary
Support for Helix Auto Registration. This will enable Ambry DataNodes to auto-join Helix cluster. AutoRegistration changes were available in
[email protected]which is only Java11 compatible so I had to pin down dependency of this version to Java8 supported snapshotTesting
./gradlew clean build