GrantPSpencer commented on code in PR #2805:
URL: https://github.com/apache/helix/pull/2805#discussion_r1626333586
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/ZkMetaClient.java:
##########
@@ -207,16 +208,37 @@ 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);
- }
+ boolean retry;
+ T updatedData = null;
+ do {
+ retry = false;
+ 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) {
+ // 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);
+ updatedData = newData;
+ } catch (MetaClientNodeExistsException nodeExistsException) {
+ // If node now exists, then retry update
+ retry = true;
+ } catch (ZkException e) {
+ throw translateZkExceptionToMetaclientException(e);
+ }
+ }
+ }
+ } while (retry);
+ return updatedData;
Review Comment:
This changes the behavior, but brings it in line with how updater behaves
for helix zkClient, which I think is what customers would expect the behavior
to be.
I think having another method with shouldRetry parameter is a good solution,
but we'll need to update the javadocs for the method to clarify default
behavior is no retry (shouldRetry = false). Let me know what you think is the
best approach
--
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
