zhangmeng916 commented on a change in pull request #1183:
URL: https://github.com/apache/helix/pull/1183#discussion_r462435090
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManagerFactory.java
##########
@@ -65,4 +68,38 @@ public static HelixManager getZKHelixManager(String
clusterName, String instance
return new ZKHelixManager(clusterName, instanceName, type, zkAddr,
stateListener);
}
+ /**
+ * Construct a ZkHelixManager using the HelixManagerProperty instance given.
If a proper
+ * ZkConnectionConfig. HelixManagerProperty given must contain a valid
ZkConnectionConfig.
Review comment:
The doc is not written in a coherent way.
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManagerFactory.java
##########
@@ -65,4 +68,38 @@ public static HelixManager getZKHelixManager(String
clusterName, String instance
return new ZKHelixManager(clusterName, instanceName, type, zkAddr,
stateListener);
}
+ /**
+ * Construct a ZkHelixManager using the HelixManagerProperty instance given.
If a proper
+ * ZkConnectionConfig. HelixManagerProperty given must contain a valid
ZkConnectionConfig.
+ * @param clusterName
+ * @param instanceName
+ * @param type
+ * @param stateListener
+ * @param helixManagerProperty must contain a valid ZkConnectionConfig
+ * @return
+ */
+ public static HelixManager getZKHelixManager(String clusterName, String
instanceName,
+ InstanceType type, HelixManagerStateListener stateListener,
+ HelixManagerProperty helixManagerProperty) {
+ return new ZKHelixManager(clusterName, instanceName, type, null,
stateListener,
+ helixManagerProperty);
+ }
+
+ /**
+ * Construct a ZkHelixManager using the HelixManagerProperty instance given.
If a proper
+ * ZkConnectionConfig is given in HelixManagerProperty, zkAddr field will be
overriden.
+ * @param clusterName
+ * @param instanceName
+ * @param type
+ * @param zkAddr will be overriden if a valid ZkConnectionConfig is given in
helixManagerProperty
Review comment:
This flexibility is a bit weird. I don't know whether users would follow
it. Can we force them to have only one set up instead of using the override,
like to validate the input.
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManagerProperty.java
##########
@@ -20,54 +20,82 @@
*/
import java.util.Properties;
+
import org.apache.helix.model.CloudConfig;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+
/**
- * Hold Helix manager properties. The manager properties further hold Helix
cloud properties
- * and some other properties specific for the manager.
+ * HelixManagerProperty is a general property/config object used for
HelixManager creation.
*/
public class HelixManagerProperty {
private static final Logger LOG =
LoggerFactory.getLogger(HelixManagerProperty.class.getName());
private String _version;
private long _healthReportLatency;
private HelixCloudProperty _helixCloudProperty;
+ private RealmAwareZkClient.RealmAwareZkConnectionConfig _zkConnectionConfig;
+ private RealmAwareZkClient.RealmAwareZkClientConfig _zkClientConfig;
/**
+ * ** Deprecated - HelixManagerProperty should be a general property/config
object used for
+ * HelixManager creation, not tied only to Properties or CloudConfig **
+ *
* Initialize Helix manager property with default value
* @param helixManagerProperties helix manager related properties input as a
map
* @param cloudConfig cloudConfig read from Zookeeper
*/
+ @Deprecated
public HelixManagerProperty(Properties helixManagerProperties, CloudConfig
cloudConfig) {
_helixCloudProperty = new HelixCloudProperty(cloudConfig);
setVersion(helixManagerProperties.getProperty(SystemPropertyKeys.HELIX_MANAGER_VERSION));
setHealthReportLatency(
helixManagerProperties.getProperty(SystemPropertyKeys.PARTICIPANT_HEALTH_REPORT_LATENCY));
}
+ public HelixManagerProperty() {
Review comment:
The original plan was to let users only get the manager property through
`getHelixManagerProperty` with default value, and then allow them to make
modification to override the fields in it. I remember there was some concern
about users accidentally changing it. We can revisit the assumption though.
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManagerProperty.java
##########
@@ -20,54 +20,82 @@
*/
import java.util.Properties;
+
import org.apache.helix.model.CloudConfig;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+
/**
- * Hold Helix manager properties. The manager properties further hold Helix
cloud properties
- * and some other properties specific for the manager.
+ * HelixManagerProperty is a general property/config object used for
HelixManager creation.
*/
public class HelixManagerProperty {
private static final Logger LOG =
LoggerFactory.getLogger(HelixManagerProperty.class.getName());
private String _version;
private long _healthReportLatency;
private HelixCloudProperty _helixCloudProperty;
+ private RealmAwareZkClient.RealmAwareZkConnectionConfig _zkConnectionConfig;
+ private RealmAwareZkClient.RealmAwareZkClientConfig _zkClientConfig;
/**
+ * ** Deprecated - HelixManagerProperty should be a general property/config
object used for
+ * HelixManager creation, not tied only to Properties or CloudConfig **
+ *
* Initialize Helix manager property with default value
* @param helixManagerProperties helix manager related properties input as a
map
* @param cloudConfig cloudConfig read from Zookeeper
*/
+ @Deprecated
public HelixManagerProperty(Properties helixManagerProperties, CloudConfig
cloudConfig) {
_helixCloudProperty = new HelixCloudProperty(cloudConfig);
setVersion(helixManagerProperties.getProperty(SystemPropertyKeys.HELIX_MANAGER_VERSION));
setHealthReportLatency(
Review comment:
fyi, the reason that we have a couple of fields like "version",
"latency" here is just to give an example of how to migrate system properties
to Helix manager property. A "TODO" should have been added here for
completeness in future work.
----------------------------------------------------------------
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]
