jiajunwang commented on a change in pull request #1066:
URL: https://github.com/apache/helix/pull/1066#discussion_r437912409
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManager.java
##########
@@ -425,6 +426,20 @@ void
addExternalViewChangeListener(org.apache.helix.ExternalViewChangeListener l
*/
boolean isLeader();
+ /**
+ * Checks whether the cluster manager is leader and sets its ZK session in
param
+ * {@link InstanceLeaderSession} if and only if it is leader.
+ *
+ * @param instanceLeaderSession To include ZK session ID of the cluster
manager in return
+ *
+ * @return true if the instance is a leader of the cluster and Zk session of
the cluster
+ * manager is returned in param {@link InstanceLeaderSession}
+ */
+ default boolean isInstanceLeader(InstanceLeaderSession
instanceLeaderSession) {
Review comment:
Same here
##########
File path: helix-core/src/main/java/org/apache/helix/HelixDataAccessor.java
##########
@@ -188,6 +188,22 @@
*/
<T extends HelixProperty> boolean[] createChildren(List<PropertyKey> keys,
List<T> children);
+ /**
+ * Adds multiple children to a parent. If successful, the children will be
created by the expected
+ * ZK session. If current ZK session does not match expected session, the
creation operation will
+ * fail.
+ *
+ * @param keys property keys
+ * @param children list of children znodes to be created
+ * @param expectedSession expected ZK session to create the children znodes
+ * @return array where true means the child was added and false means it was
not
+ */
+ default <T extends HelixProperty> boolean[] createChildren(List<PropertyKey>
keys, List<T> children,
Review comment:
same here
##########
File path: helix-core/src/main/java/org/apache/helix/BaseDataAccessor.java
##########
@@ -95,6 +95,24 @@
*/
boolean[] createChildren(List<String> paths, List<T> records, int options);
+ /**
+ * Use it when creating children under a parent node with an expected ZK
session.
+ * <p>
+ * This will use async api for better performance. If the children already
exist it will return
+ * false.
+ *
+ * @param paths the paths to the children ZNodes
+ * @param records List of data to write to each of the path
+ * @param options Set the type of ZNode see the valid values in {@link
AccessOption}
+ * @param expectedSession The expected ZK session to create children
+ * @return For each child: true if creation succeeded, false otherwise (e.g.
if the child exists)
+ */
+ default boolean[] createChildren(List<String> paths, List<T> records, int
options,
Review comment:
Instead of polluting the interface, shall we have a new interface called
sessionAwareZkClient or something like that? Note that an implementation can
implement multiple interfaces. So we just need to let the right class implement
that interface. And this one won't need to have this ugly and not supported
method.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/AttributeName.java
##########
@@ -40,5 +40,6 @@
PipelineType,
LastRebalanceFinishTimeStamp,
ControllerDataProvider,
- STATEFUL_REBALANCER
+ STATEFUL_REBALANCER,
+ CONTROLLER_LEADER_SESSION
Review comment:
This can be more generic. Just call it event_session?
As you can imagine, this can be used in multiple ways.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
##########
@@ -401,6 +402,8 @@ void checkConnected(long timeout) {
throw new HelixException(
"HelixManager is not connected within retry timeout for cluster " +
_clusterName);
}
+
+ _sessionId = ZKUtil.toHexSessionId(_zkclient.getSessionId());
Review comment:
This may cause some issues. If the session is updated randomly, a new
session may be populated before new session handling is done.
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -2119,8 +2126,41 @@ private String getHexSessionId() {
* 2. create mode is EPHEMERAL or EPHEMERAL_SEQUENTIAL
*/
private boolean isSessionAwareOperation(String expectedSessionId, CreateMode
mode) {
- return expectedSessionId != null && !expectedSessionId.isEmpty() && (
- mode == CreateMode.EPHEMERAL || mode ==
CreateMode.EPHEMERAL_SEQUENTIAL);
+ return expectedSessionId != null && !expectedSessionId.isEmpty() &&
mode.isEphemeral();
+ }
+
+ private ZooKeeper getExpectedZookeeper(final String expectedSessionId) {
Review comment:
This is a great idea!
----------------------------------------------------------------
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]
