Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CURATOR-430] deletingChildrenIfNeeded maybe cannot delete children completely when multi-client delete concurrently #235

Merged
merged 3 commits into from
Sep 24, 2017

Conversation

heziai
Copy link
Contributor

@heziai heziai commented Aug 12, 2017

use curatorFramework.delete().deletingChildrenIfNeeded().forPath(path), this sync api doesn't ignore the NoNodeException, causes the rest of children nodes will not be deleted perhaps.

zookeeper.getChildren(path, null) maybe throw NoNodeException, if the path doesn't exist.

@heziai heziai changed the title deletingChildrenIfNeeded maybe cannot delete children completely when multi-client delete concurrently CURATOR-430 Aug 12, 2017
@heziai heziai changed the title CURATOR-430 [CURATOR-430] deletingChildrenIfNeeded maybe cannot delete children completely when multi-client delete concurrently Aug 12, 2017
@Randgalt
Copy link
Member

Please include a test (something like the code you posted in the issue) for this. A good place to put the test is TestFrameworkEdges.java.

Copy link
Contributor

@lvfangmin lvfangmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me, except a minor comment.

try {
children = zookeeper.getChildren(path, null);
} catch (KeeperException.NoNodeException e) {
// someone else has deleted the node it since we checked
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// someone else has deleted the node since we checked

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha... it's copied

@heziai
Copy link
Contributor Author

heziai commented Aug 17, 2017

Be sure this issue is bug? If yes, could you publish a fixed version based on v2.10.0?

@JunjianS
Copy link

i met the same problem, could u please provide a fixed version ?

@asfgit asfgit merged commit 8daddd2 into apache:master Sep 24, 2017
@Randgalt
Copy link
Member

If yes, could you publish a fixed version based on v2.10.0?

Curator 4.0 is now compatible with both ZK 3.4.x and 3.5.x so the 2.x versions of Curator will not be updated.

@Randgalt
Copy link
Member

Randgalt commented Dec 8, 2018

@hebelala hey - the test for this PR keeps failing and as I read it, I don't see how it ever worked. Can you look at this again? The code client2.delete().forPath("/parent/child" + (childCount / 2)); should actually fail most of the time as the thread has deleted all the children.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants