junkaixue commented on code in PR #2607:
URL: https://github.com/apache/helix/pull/2607#discussion_r1336444726
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/ZkMetaClient.java:
##########
@@ -115,6 +117,67 @@ public void create(String key, Object data,
MetaClientInterface.EntryMode mode)
}
}
+ @Override
+ public void recursiveCreate(String key, T data, EntryMode mode) {
+ // Function named recursiveCreate to match naming scheme, but actual work
is iterative
+ iterativeCreate(key, data, mode, -1);
+ }
+
+ @Override
+ public void recursiveCreateWithTTL(String key, T data, long ttl) {
+ iterativeCreate(key, data, EntryMode.TTL, ttl);
+ }
+
+ private void iterativeCreate(String key, T data, EntryMode mode, long ttl) {
+ List<String> nodePaths = separateIntoUniqueNodePaths(key);
+ int i = 0;
+ // Ephemeral nodes cant have children, so change mode when creating parents
+ EntryMode parentMode = (EntryMode.EPHEMERAL.equals(mode) ?
+ EntryMode.PERSISTENT : mode);
+
+ // Iterate over paths, starting with full key then attempting each
successive parent
+ // Try /a/b/c, if parent /a/b, does not exist, then try to create parent,
etc..
+ while (i < nodePaths.size()) {
+ // If parent exists or there is no parent node, then try to create the
node
+ // and break out of loop on successful create
+ if (i == nodePaths.size() - 1 || _zkClient.exists(nodePaths.get(i+1))) {
+ try {
+ if (EntryMode.TTL.equals(mode)) {
+ createWithTTL(nodePaths.get(i), data, ttl);
+ } else {
+ create(nodePaths.get(i), data, i == 0 ? mode : parentMode);
+ }
+ // Race condition may occur where a node is created by another thread
in between loops.
+ // We should not throw error if this occurs for parent nodes, only for
the full node path.
+ } catch (MetaClientNodeExistsException e) {
+ if (i != 0) {
+ throw e;
+ }
+ }
+ break;
+ // Else try to create parent in next loop iteration
+ } else {
+ i++;
+ }
+ }
+
+ // Reattempt creation of children that failed due to NoNodeException
+ while (--i >= 0) {
Review Comment:
is this i ++ or --I? We should start with 0, right?
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/ZkMetaClient.java:
##########
@@ -115,6 +117,67 @@ public void create(String key, Object data,
MetaClientInterface.EntryMode mode)
}
}
+ @Override
+ public void recursiveCreate(String key, T data, EntryMode mode) {
+ // Function named recursiveCreate to match naming scheme, but actual work
is iterative
+ iterativeCreate(key, data, mode, -1);
+ }
+
+ @Override
+ public void recursiveCreateWithTTL(String key, T data, long ttl) {
+ iterativeCreate(key, data, EntryMode.TTL, ttl);
+ }
+
+ private void iterativeCreate(String key, T data, EntryMode mode, long ttl) {
+ List<String> nodePaths = separateIntoUniqueNodePaths(key);
+ int i = 0;
+ // Ephemeral nodes cant have children, so change mode when creating parents
+ EntryMode parentMode = (EntryMode.EPHEMERAL.equals(mode) ?
+ EntryMode.PERSISTENT : mode);
+
+ // Iterate over paths, starting with full key then attempting each
successive parent
+ // Try /a/b/c, if parent /a/b, does not exist, then try to create parent,
etc..
+ while (i < nodePaths.size()) {
+ // If parent exists or there is no parent node, then try to create the
node
+ // and break out of loop on successful create
+ if (i == nodePaths.size() - 1 || _zkClient.exists(nodePaths.get(i+1))) {
+ try {
+ if (EntryMode.TTL.equals(mode)) {
+ createWithTTL(nodePaths.get(i), data, ttl);
+ } else {
+ create(nodePaths.get(i), data, i == 0 ? mode : parentMode);
+ }
+ // Race condition may occur where a node is created by another thread
in between loops.
+ // We should not throw error if this occurs for parent nodes, only for
the full node path.
+ } catch (MetaClientNodeExistsException e) {
+ if (i != 0) {
+ throw e;
+ }
+ }
+ break;
+ // Else try to create parent in next loop iteration
+ } else {
+ i++;
+ }
+ }
+
+ // Reattempt creation of children that failed due to NoNodeException
Review Comment:
Update this comment.
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/ZkMetaClient.java:
##########
@@ -115,6 +117,67 @@ public void create(String key, Object data,
MetaClientInterface.EntryMode mode)
}
}
+ @Override
+ public void recursiveCreate(String key, T data, EntryMode mode) {
+ // Function named recursiveCreate to match naming scheme, but actual work
is iterative
+ iterativeCreate(key, data, mode, -1);
+ }
+
+ @Override
+ public void recursiveCreateWithTTL(String key, T data, long ttl) {
+ iterativeCreate(key, data, EntryMode.TTL, ttl);
+ }
+
+ private void iterativeCreate(String key, T data, EntryMode mode, long ttl) {
+ List<String> nodePaths = separateIntoUniqueNodePaths(key);
+ int i = 0;
+ // Ephemeral nodes cant have children, so change mode when creating parents
+ EntryMode parentMode = (EntryMode.EPHEMERAL.equals(mode) ?
+ EntryMode.PERSISTENT : mode);
+
+ // Iterate over paths, starting with full key then attempting each
successive parent
+ // Try /a/b/c, if parent /a/b, does not exist, then try to create parent,
etc..
+ while (i < nodePaths.size()) {
+ // If parent exists or there is no parent node, then try to create the
node
+ // and break out of loop on successful create
+ if (i == nodePaths.size() - 1 || _zkClient.exists(nodePaths.get(i+1))) {
+ try {
+ if (EntryMode.TTL.equals(mode)) {
+ createWithTTL(nodePaths.get(i), data, ttl);
+ } else {
+ create(nodePaths.get(i), data, i == 0 ? mode : parentMode);
+ }
+ // Race condition may occur where a node is created by another thread
in between loops.
+ // We should not throw error if this occurs for parent nodes, only for
the full node path.
+ } catch (MetaClientNodeExistsException e) {
+ if (i != 0) {
+ throw e;
+ }
Review Comment:
It makes the API complicated behavior. I would suggest to consider if there
is exist nodes, then we just stop disregard it is root race condition or not.
Otherwise, the API behavior would be a little bit complicate. We can just
let user to retry create it recursively if it is root race condition.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
