You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As per ticket ZOOKEEPER-3414 while executing sync cmd NoNodeException should be thrown when path doesn't exist. To implement this I have used CliWrapperException and KeeperException.NoNodeException in the else condition of exec() present in SyncCommand.java
Please do let me know if the changes made make sense or if I have missed something.
@ravowlga123 Issue has been reported my @maoling .
I had the assumption of this should be implemented on server side. There's no guarantee of somebody not deleting the node after getData call.
I had the assumption of this should be implemented on server side. There's no guarantee of somebody not deleting the node after getData call.
Yes, it is. Implementing on client side really has that side effect, especially a delete after sync by another client, just before getData.
Overall, sync + getData isn't a good api design and some issues in it(e.g sync isn't a quorum operation and cannot get updated data when network partitioned, so I create ZOOKEEPER-3600 to depreciate it by a new Linearizability Read api.)
A base summary of current sync implementation, when the client calls the sync api to a follower server, follower pends the requestA(in the pendingSyncs) and forwards requestA to leader. when leader has no outstanding proposals(outstandingProposals), leader will send a SYNC message to follower, follower will commit requestA util it receives the SYNC from leader.
For the server side fix:
FinalRequestProcessor
case OpCode.sync: {
xxxxxxxxxxxxxxxxxxxxxxxxxx
path = syncRequest.getPath();
DataNode n = zks.getZKDatabase().getNode(path);
if (n == null) {
throw new KeeperException.NoNodeException();
}
rsp = new SyncResponse(path);
xxxxxxxxxxxxxxxxxxxxxxxxxx
}
SyncCommand:
} else if (resultCode == KeeperException.Code.NONODE.intValue()) {
throw new KeeperException.NoNodeException(path);
} else
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As per ticket ZOOKEEPER-3414 while executing sync cmd NoNodeException should be thrown when path doesn't exist. To implement this I have used CliWrapperException and KeeperException.NoNodeException in the else condition of exec() present in SyncCommand.java
Please do let me know if the changes made make sense or if I have missed something.