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-715. bottom up existence check for mkdirs #506

Merged

Conversation

pfcoperez
Copy link
Contributor

@pfcoperez pfcoperez commented Aug 31, 2024

This PR tackles the problem reported in https://issues.apache.org/jira/browse/CURATOR-715.

Namely, that for ZooKeeper trees where upper nodes have restricted READ ACLs, it is not possible to use creatingParentsIfNeeded without changing the ACLs.

This is because the implementation prior to this patch iterated from root node to the newly created one, checking existence of all intermediate nodes.

This patch changes that approach from top-down to bottom-up and the first ancestor that exists is where the validation stops thus avoiding exists checks on the higher nodes with more restricted access.

This is only relevant since https://issues.apache.org/jira/browse/ZOOKEEPER-2590

@madhava-sridhar
Copy link

I would vote up the fix , this is real blocker to anybody upgrading zookeeper to version with fix for ZOOKEEPER-2590

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@apache/curator-committers can you please take a look ?

@eolivelli
Copy link
Contributor

Hopefully we can include this in the next release, looking for a second reviewer
thanks

@pfcoperez
Copy link
Contributor Author

Hopefully we can include this in the next release, looking for a second reviewer thanks

Thank you @eolivelli! I've just pushed code style fix after executing mvn spotless:apply with d886a8a .

@kezhuw kezhuw merged commit 9fe79d8 into apache:master Sep 12, 2024
10 checks passed
@kezhuw
Copy link
Member

kezhuw commented Sep 12, 2024

@pfcoperez Merged. Thank you for your contribution!

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

Looks reasonable.

@tisonkun tisonkun changed the title CURATOR-715/mkdirs/bottom up existence check CURATOR-715. bottom up existence check for mkdirs Sep 12, 2024
@pfcoperez
Copy link
Contributor Author

Thank you @eolivelli @kezhuw and @tisonkun for your reviews 🙇

Do you have any plans of releasing before the end of the year?

@eolivelli
Copy link
Contributor

Please ask on [email protected]
We can cut a release

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