GrantPSpencer commented on code in PR #2805: URL: https://github.com/apache/helix/pull/2805#discussion_r1633886928
########## meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/ZkMetaClient.java: ########## @@ -207,16 +208,55 @@ public void set(String key, T data, int version) { @Override public T update(String key, DataUpdater<T> updater) { - org.apache.zookeeper.data.Stat stat = new org.apache.zookeeper.data.Stat(); - // TODO: add retry logic for ZkBadVersionException. - try { - T oldData = _zkClient.readData(key, stat); - T newData = updater.update(oldData); - set(key, newData, stat.getVersion()); - return newData; - } catch (ZkException e) { - throw translateZkExceptionToMetaclientException(e); - } + return update(key, updater, false); + } + + @Override + public T update(String key, DataUpdater<T> updater, boolean retryOnFailure) { + final int MAX_RETRY_ATTEMPTS = 3; + int retryAttempts = 0; + boolean retry; + T updatedData = null; + do { + retry = false; + retryAttempts++; + try { + ImmutablePair<T, Stat> tup = getDataAndStat(key); + Stat stat = tup.right; + T oldData = tup.left; + T newData = updater.update(oldData); + set(key, newData, stat.getVersion()); + updatedData = newData; + } catch (MetaClientBadVersionException badVersionException) { + // If exceeded max retry attempts, re-throw exception + if (retryAttempts >= MAX_RETRY_ATTEMPTS) { + LOG.error("Failed to update node at " + key + " after " + MAX_RETRY_ATTEMPTS + " attempts."); + throw badVersionException; + } + // Retry on bad version + retry = true; + } catch (MetaClientNoNodeException noNodeException) { + // If node does not exist, attempt to create it - pass null to updater + T newData = updater.update(null); + if (newData != null) { + try { + create(key, newData); Review Comment: Spoke with @xyuanlu offline. Added createIfAbsent parameter. Base functionality is to not create the node, but will attempt to create leaf node if it is not present. It will not recursively create and will fail on first create() call if parent path does not exist. Added relevant test cases to cover this cc @junkaixue -- 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: reviews-unsubscr...@helix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org