Skip to content

ZOOKEEPER-4846 Failure to reload database due to missing ACL - #2222

Merged
anmolnar merged 1 commit into
apache:masterfrom
anmolnar:ZOOKEEPER-4846
Feb 11, 2025
Merged

ZOOKEEPER-4846 Failure to reload database due to missing ACL#2222
anmolnar merged 1 commit into
apache:masterfrom
anmolnar:ZOOKEEPER-4846

Conversation

@anmolnar

@anmolnar anmolnar commented Feb 7, 2025

Copy link
Copy Markdown
Contributor

In the scenario when ZooKeeper restore from fuzzy snapshot, there's possibility that we have a znode in the snapshot, but the attached ACL is missing from the ACL cache. In this case ZK will fast forward to znode's createTxn and replays it. While we skip re-creating the existing znode, we have a chance to fix the potentially wrong ACL reference.

@anmolnar

anmolnar commented Feb 7, 2025

Copy link
Copy Markdown
Contributor Author

ping @ztzg @cnauroth

@cnauroth cnauroth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Nice job debugging this and the test looks good too, @anmolnar !

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@ztzg
ztzg self-requested a review February 11, 2025 05:54
if (children.contains(childName)) {
DataNode existingChild = nodes.get(path);
if (existingChild != null) {
existingChild.acl = acls;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have not had a chance to play with this, but: this means we are relying on PrepRequestProcessor to prevent OpCode.create* operations from being abused as OpCode.setACL, right? (Just thinking out loud.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure to be honest and not sure if I understand your question.

@anmolnar
anmolnar merged commit 8532163 into apache:master Feb 11, 2025
asfgit pushed a commit that referenced this pull request Feb 11, 2025
ZOOKEEPER-4846. Fix ACL reference on existing znode when trying to create
Reviewers: cnauroth, eolivelli, ztzg
Author: anmolnar
Closes #2222 from anmolnar/ZOOKEEPER-4846

(cherry picked from commit 8532163)
Signed-off-by: Andor Molnar <andor@apache.org>
asfgit pushed a commit that referenced this pull request Feb 11, 2025
ZOOKEEPER-4846. Fix ACL reference on existing znode when trying to create
Reviewers: cnauroth, eolivelli, ztzg
Author: anmolnar
Closes #2222 from anmolnar/ZOOKEEPER-4846

(cherry picked from commit 8532163)
Signed-off-by: Andor Molnar <andor@apache.org>
@anmolnar
anmolnar deleted the ZOOKEEPER-4846 branch February 11, 2025 16:50
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.

4 participants