ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1055

check for duplicate ACLs in addACL() and create()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.4.0
    • Fix Version/s: 3.4.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      refresh against trunk.

      Description

      actual result:

      [zk: (CONNECTED) 0] create /test2 'test2' digest:test:test:cdrwa,digest:test:test:cdrwa
      Created /test2
      [zk: (CONNECTED) 1] getAcl /test2
      'digest,'test:test
      : cdrwa
      'digest,'test:test
      : cdrwa
      [zk: (CONNECTED) 2]

      but getAcl should only have a single entry.

      1. ZOOKEEPER-1055.patch
        5 kB
        Eugene Koontz
      2. ZOOKEEPER-1055.patch
        6 kB
        Eugene Koontz
      3. ZOOKEEPER-1055.patch
        6 kB
        Eugene Koontz
      4. ZOOKEEPER-1055.patch
        10 kB
        Eugene Koontz
      5. ZOOKEEPER-1055.patch
        10 kB
        Mahadev konar
      6. ZOOKEEPER-1055.patch
        10 kB
        Eugene Koontz

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1266 (See https://builds.apache.org/job/ZooKeeper-trunk/1266/)
          ZOOKEEPER-1055. check for duplicate ACLs in addACL() and create(). (Eugene Koontz via mahadev)

          mahadev : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1157685
          Files :

          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1266 (See https://builds.apache.org/job/ZooKeeper-trunk/1266/ ) ZOOKEEPER-1055 . check for duplicate ACLs in addACL() and create(). (Eugene Koontz via mahadev) mahadev : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1157685 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
          Hide
          Mahadev konar added a comment -

          I just committed this. Thanks Eugene!

          Show
          Mahadev konar added a comment - I just committed this. Thanks Eugene!
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12489131/ZOOKEEPER-1055.patch
          against trunk revision 1152141.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12489131/ZOOKEEPER-1055.patch against trunk revision 1152141. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/444//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          looks like the c tests failed. Eugene, can you please take a look and see if this is just a one time off test failure or due to the patch?

          Show
          Mahadev konar added a comment - looks like the c tests failed. Eugene, can you please take a look and see if this is just a one time off test failure or due to the patch?
          Hide
          Eugene Koontz added a comment -

          Hi Mahadev,
          The patch looks good to me. Thanks for updating it.
          -Eugene

          Show
          Eugene Koontz added a comment - Hi Mahadev, The patch looks good to me. Thanks for updating it. -Eugene
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12485535/ZOOKEEPER-1055.patch
          against trunk revision 1142377.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12485535/ZOOKEEPER-1055.patch against trunk revision 1142377. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/371//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          Eugene, I just copied your code over. Multi Update had changed most of the codebase. Can you please review and see if I missed something or not?

          Show
          Mahadev konar added a comment - Eugene, I just copied your code over. Multi Update had changed most of the codebase. Can you please review and see if I missed something or not?
          Hide
          Mahadev konar added a comment -

          the patch has gone stale. Sorry Eugene.

          Show
          Mahadev konar added a comment - the patch has gone stale. Sorry Eugene.
          Hide
          Eugene Koontz added a comment -

          Hi Mahadev,
          Looks good to me! Thanks a lot for your review!
          -Eugene

          Show
          Eugene Koontz added a comment - Hi Mahadev, Looks good to me! Thanks a lot for your review! -Eugene
          Hide
          Mahadev konar added a comment -

          eugene,
          The patch looks good.

          One minor nit:

           if (authInfo.contains(id) == false) {
                     authInfo.add(id);
           }
          

          can be replaced by:

           if (!authInfo.contains(id)) {
                     authInfo.add(id);
                  }
          

          I can make the minor change and check it in. Let me know.

          Show
          Mahadev konar added a comment - eugene, The patch looks good. One minor nit: if (authInfo.contains(id) == false ) { authInfo.add(id); } can be replaced by: if (!authInfo.contains(id)) { authInfo.add(id); } I can make the minor change and check it in. Let me know.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12479195/ZOOKEEPER-1055.patch
          against trunk revision 1103811.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12479195/ZOOKEEPER-1055.patch against trunk revision 1103811. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/264//console This message is automatically generated.
          Hide
          Eugene Koontz added a comment -

          add removeDuplicates() function to remove duplicate ACLs from client create()s and setACL()s before the ACLs are added to the node by the server.

          Show
          Eugene Koontz added a comment - add removeDuplicates() function to remove duplicate ACLs from client create()s and setACL()s before the ACLs are added to the node by the server.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12479064/ZOOKEEPER-1055.patch
          against trunk revision 1099329.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12479064/ZOOKEEPER-1055.patch against trunk revision 1099329. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/260//console This message is automatically generated.
          Hide
          Eugene Koontz added a comment -

          --no-prefix

          Show
          Eugene Koontz added a comment - --no-prefix
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12479042/ZOOKEEPER-1055.patch
          against trunk revision 1099329.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/258//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12479042/ZOOKEEPER-1055.patch against trunk revision 1099329. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/258//console This message is automatically generated.
          Hide
          Eugene Koontz added a comment -

          I noticed that we use SelectionKey.attach() to attach authInfo information to ClientCnxns. A bit of googling led me to this blog post: "http://jfarcand.wordpress.com/2006/07/06/tricks-and-tips-with-nio-part-ii-why-selectionkey-attach-is-evil/"

          I'm not sure if this blog post is relevant, but thought I'd mention it.

          Show
          Eugene Koontz added a comment - I noticed that we use SelectionKey.attach() to attach authInfo information to ClientCnxns. A bit of googling led me to this blog post: "http://jfarcand.wordpress.com/2006/07/06/tricks-and-tips-with-nio-part-ii-why-selectionkey-attach-is-evil/" I'm not sure if this blog post is relevant, but thought I'd mention it.
          Hide
          Eugene Koontz added a comment -

          With patch, unit test passes consistently.

          Show
          Eugene Koontz added a comment - With patch, unit test passes consistently.
          Hide
          Eugene Koontz added a comment -

          Correction: sometimes fails on trunk, sometimes succeeds.

          Show
          Eugene Koontz added a comment - Correction: sometimes fails on trunk, sometimes succeeds.
          Hide
          Eugene Koontz added a comment -

          This is not a fix yet; but is simply a unit test that shows the bug (this test fails on trunk).

          (removed irrelevant logging change from patch)

          Show
          Eugene Koontz added a comment - This is not a fix yet; but is simply a unit test that shows the bug (this test fails on trunk). (removed irrelevant logging change from patch)
          Hide
          Eugene Koontz added a comment -

          This is not a fix yet; but is simply a unit test that shows the bug (this test fails on trunk).

          Show
          Eugene Koontz added a comment - This is not a fix yet; but is simply a unit test that shows the bug (this test fails on trunk).

            People

            • Assignee:
              Eugene Koontz
              Reporter:
              Eugene Koontz
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development