pkuwm commented on a change in pull request #1066:
URL: https://github.com/apache/helix/pull/1066#discussion_r438323140
##########
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:
This is a Helix Java API interface. Do you think we would need new
interfaces for the Java APIs?
I've carefully thought about having new interfaces. But it bings in 3 new
interfaces for zkclient/BaseDataAccessor/HelixDataAccessor, which causes more
efforts to maintain. And we may need to create all the APIs with expected
session aware.
Currently we are only adding one new API to each of the 3 interfaces. I feel
that this is the easiest way and it won't add too much pollution.
If we decide to go to that direction, that would be another PR. Let's
discuss this more.
##########
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:
This is only one API required to be added to this HelixManager.
HelixManager is no supposed to be a public API, right? We wound't want a new
interface just for this API. I think this one is fine.
But I think we could determine the name of this new API, as Junkai mentioned
we should make it consistent with `isLeader()`.
----------------------------------------------------------------
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]
