You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by "anmolnar (via GitHub)" <gi...@apache.org> on 2025/02/07 21:23:15 UTC

[PR] ZOOKEEPER-4846 Failure to reload database due to missing ACL [zookeeper]

anmolnar opened a new pull request, #2222:
URL: https://github.com/apache/zookeeper/pull/2222

   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 the createTxn and replays is and while we skip re-creating the existing znode, we have a chance to fix the potentially wrong ACL reference.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-4846 Failure to reload database due to missing ACL [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #2222:
URL: https://github.com/apache/zookeeper/pull/2222#discussion_r1951123453


##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java:
##########
@@ -462,8 +462,9 @@ public void createNode(final String path, byte[] data, List<ACL> acl, long ephem
             // we did for the global sessions.
             Long acls = aclCache.convertAcls(acl);
 
-            Set<String> children = parent.getChildren();
-            if (children.contains(childName)) {
+            DataNode existingChild = nodes.get(path);
+            if (existingChild != null) {
+                existingChild.acl = acls;

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



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-4846 Failure to reload database due to missing ACL [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar merged PR #2222:
URL: https://github.com/apache/zookeeper/pull/2222


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-4846 Failure to reload database due to missing ACL [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2222:
URL: https://github.com/apache/zookeeper/pull/2222#issuecomment-2644156194

   ping @ztzg @cnauroth 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-4846 Failure to reload database due to missing ACL [zookeeper]

Posted by "ztzg (via GitHub)" <gi...@apache.org>.
ztzg commented on code in PR #2222:
URL: https://github.com/apache/zookeeper/pull/2222#discussion_r1950277286


##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java:
##########
@@ -462,8 +462,9 @@ public void createNode(final String path, byte[] data, List<ACL> acl, long ephem
             // we did for the global sessions.
             Long acls = aclCache.convertAcls(acl);
 
-            Set<String> children = parent.getChildren();
-            if (children.contains(childName)) {
+            DataNode existingChild = nodes.get(path);
+            if (existingChild != null) {
+                existingChild.acl = acls;

Review Comment:
   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.)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org