TisonKun commented on a change in pull request #376:
URL: https://github.com/apache/curator/pull/376#discussion_r568269227
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
##########
@@ -1147,12 +1235,35 @@ public String call() throws Exception
if ( setDataIfExists )
{
Stat setStat =
client.getZooKeeper().setData(path, data, setDataIfExistsVersion);
- if(storingStat != null)
+ if ( storingStat != null )
{
DataTree.copyStat(setStat,
storingStat);
}
createdPath = path;
}
+ else if ( idempotent &&
!createMode.isSequential() )
Review comment:
ditto warning suggestion here, or we can check whenever an operation
triggered.
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
##########
@@ -642,6 +657,10 @@ else if ( (rc ==
KeeperException.Code.NODEEXISTS.intValue()) && setDataIfExists
{
backgroundSetData(client, operationAndData,
operationAndData.getData().getPath(), backgrounding);
}
+ else if ( (rc ==
KeeperException.Code.NODEEXISTS.intValue()) && idempotent &&
!createMode.isSequential() )
Review comment:
Can we log a warning when user set `idempotent` with sequential node?
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
##########
@@ -87,6 +93,8 @@ public CreateBuilderImpl(CuratorFrameworkImpl client,
CreateMode createMode, Bac
this.createParentsAsContainers = createParentsAsContainers;
this.compress = compress;
this.setDataIfExists = setDataIfExists;
+ // TODO implement this in CURATOR-589
Review comment:
remove before merging
##########
File path: curator-examples/src/main/java/framework/CrudExamples.java
##########
@@ -119,6 +145,24 @@ public static void guaranteedDelete(CuratorFramework
client, String path) t
client.delete().guaranteed().forPath(path);
}
+ public static void idempotentDelete(CuratorFramework client, String
path, int currentVersion) throws Exception
Review comment:
nit: why not `deleteIdempotent`
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java
##########
@@ -210,6 +231,48 @@ public Stat forPath(String path) throws Exception
return this;
}
+ private void backgroundCheckIdempotent(final CuratorFrameworkImpl client,
final OperationAndData<PathAndBytes> mainOperationAndData, final String path,
final Backgrounding backgrounding)
+ {
+ final AsyncCallback.DataCallback dataCallback = new
AsyncCallback.DataCallback()
+ {
+ @Override
+ public void processResult(int rc, String path, Object ctx, byte[]
data, Stat stat)
+ {
+ if ( rc == KeeperException.Code.OK.intValue() )
+ {
+ if ( failNextIdempotentCheckForTesting )
+ {
+ failNextIdempotentCheckForTesting = false;
+ rc =
KeeperException.Code.CONNECTIONLOSS.intValue();
+ }
+ else if ( !idempotentSetMatches(stat.getVersion(),
mainOperationAndData.getData().getData(), data) )
+ {
+ rc = KeeperException.Code.BADVERSION.intValue();
+ }
+ }
+ CuratorEvent event = new CuratorEventImpl(client,
CuratorEventType.SET_DATA, rc, path, null, ctx, stat, null, null, null, null,
null);
Review comment:
```suggestion
final CuratorEvent event = new CuratorEventImpl(client,
CuratorEventType.SET_DATA, rc, path, null, ctx, stat, null, null, null, null,
null);
```
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java
##########
@@ -51,6 +63,8 @@ public SetDataBuilderImpl(CuratorFrameworkImpl client,
Backgrounding backgroundi
this.backgrounding = backgrounding;
this.version = version;
this.compress = compress;
+ // TODO implement this in CURATOR-589
Review comment:
ditto removal suggestion here.
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java
##########
@@ -36,13 +39,22 @@
private Backgrounding backgrounding;
private int version;
private boolean compress;
+ private boolean idempotent;
Review comment:
ditto default `false` suggest here.
##########
File path:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
##########
@@ -57,12 +57,17 @@
private boolean compress;
private boolean setDataIfExists;
private int setDataIfExistsVersion = -1;
+ private boolean idempotent;
Review comment:
how about just set a default `false` here?
----------------------------------------------------------------
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]
