Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: server
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    1. ZOOKEEPER-33.patch
      22 kB
      Mahadev konar
    2. ZOOKEEPER-33.patch
      22 kB
      Mahadev konar
    3. ZOOKEEPER-33.patch
      22 kB
      Mahadev konar
    4. ZOOKEEPER-33.patch
      16 kB
      Mahadev konar
    5. ZOOKEEPER-33.patch
      16 kB
      Mahadev konar
    6. ZOOKEEPER-33.patch
      23 kB
      Mahadev konar
    7. ZOOKEEPER-33.patch
      23 kB
      Mahadev konar
    8. ZOOKEEPER-33.patch
      15 kB
      Mahadev konar

      Issue Links

        Activity

        Hide
        Mahadev konar added a comment -

        here is a patch for this issue. its a preliminray patch. I will be adding tests.

        The patch does the following:

        has a mapping from long to acls in the datatree
        each datanode has a reference to acls.

        this only impacts the serialization and deserialization of acls on the datatree.

        Show
        Mahadev konar added a comment - here is a patch for this issue. its a preliminray patch. I will be adding tests. The patch does the following: has a mapping from long to acls in the datatree each datanode has a reference to acls. this only impacts the serialization and deserialization of acls on the datatree.
        Hide
        Mahadev konar added a comment -

        this is a patch that includes the tests for this jira. The jira includes

        1) create a map from long to acl and acl to long in the datatree
        2) serialize the maps to the snapshots
        3) change the datanodes to now have a reference to the acl maps ( storing longs instead of the acls)
        4) the tests check to see if the acl map works correctly – storing only a single copy of equal acls and also tests to see if the acl map works after a server is shutdown adn then brought up back again...

        Show
        Mahadev konar added a comment - this is a patch that includes the tests for this jira. The jira includes 1) create a map from long to acl and acl to long in the datatree 2) serialize the maps to the snapshots 3) change the datanodes to now have a reference to the acl maps ( storing longs instead of the acls) 4) the tests check to see if the acl map works correctly – storing only a single copy of equal acls and also tests to see if the acl map works after a server is shutdown adn then brought up back again...
        Hide
        Mahadev konar added a comment -

        attaching a new patch. The last patch went stale with checking in of ZOOKEEPER-8.

        Show
        Mahadev konar added a comment - attaching a new patch. The last patch went stale with checking in of ZOOKEEPER-8 .
        Hide
        Benjamin Reed added a comment -

        -1 I like the patch, but there are just a couple of things that need fixing:

        1) The mapping should be between # and ArrayList<ACL> rather than # and ACL. I think it further simplifies things and will make serialization/deserialization go even faster. convertACLs should return a long for example.

        2) When you serialize the # to ACL map and the ACL to # map, you only need to serialize once. I would serialize the # to ACL map. When you deserialize you can rebuild the ACL to # map at the same time since they have the same information.

        Show
        Benjamin Reed added a comment - -1 I like the patch, but there are just a couple of things that need fixing: 1) The mapping should be between # and ArrayList<ACL> rather than # and ACL. I think it further simplifies things and will make serialization/deserialization go even faster. convertACLs should return a long for example. 2) When you serialize the # to ACL map and the ACL to # map, you only need to serialize once. I would serialize the # to ACL map. When you deserialize you can rebuild the ACL to # map at the same time since they have the same information.
        Hide
        Mahadev konar added a comment -

        this pathc addesses the issues pouinted out by ben..
        ben can you take a look again?

        Show
        Mahadev konar added a comment - this pathc addesses the issues pouinted out by ben.. ben can you take a look again?
        Hide
        Mahadev konar added a comment -

        fixed a bug in the old patch ... the old patch overrides acls after deserializing the acls from the datatrree. I just reassigned the index number of acls maps to the deserialized maximum long from the map.

        Show
        Mahadev konar added a comment - fixed a bug in the old patch ... the old patch overrides acls after deserializing the acls from the datatrree. I just reassigned the index number of acls maps to the deserialized maximum long from the map.
        Hide
        Mahadev konar added a comment -

        just to make sure the patch uploaded at 5:53 pm is the latest one... the jira does not show it as the last patch...

        Show
        Mahadev konar added a comment - just to make sure the patch uploaded at 5:53 pm is the latest one... the jira does not show it as the last patch...
        Hide
        Mahadev konar added a comment -

        forgot to add the test...

        Show
        Mahadev konar added a comment - forgot to add the test...
        Hide
        Mahadev konar added a comment -

        the patch uploaded at 06:59 PM is the latest now ...

        Show
        Mahadev konar added a comment - the patch uploaded at 06:59 PM is the latest now ...
        Hide
        Benjamin Reed added a comment -

        -1 Sorry Mahadev, two small issues (one is super trivial

        1) convertLongs should be called covertLong since there is only a single Long being converted
        2) convertAcls should use a Map<List<ACL>, Long> to convert an ACL list to a long, otherwise that could become an expensive method when more than a few different ACLs are used. Note, that Map does not need to be serialized and can be deserialized with the Map<Long,List<ACL>>

        Show
        Benjamin Reed added a comment - -1 Sorry Mahadev, two small issues (one is super trivial 1) convertLongs should be called covertLong since there is only a single Long being converted 2) convertAcls should use a Map<List<ACL>, Long> to convert an ACL list to a long, otherwise that could become an expensive method when more than a few different ACLs are used. Note, that Map does not need to be serialized and can be deserialized with the Map<Long,List<ACL>>
        Hide
        Mahadev konar added a comment -

        attaching the patch with ben's comments ....
        1) fixed
        2) fixed

        Show
        Mahadev konar added a comment - attaching the patch with ben's comments .... 1) fixed 2) fixed
        Hide
        Benjamin Reed added a comment -

        +1 Great job! Thanx Mahadev. Please fix one tiny thing before you commit (you don't need to do another patch): the comment of aclIndex should be "the number of acls that we have in the datatree" rather than "the max number of acls". Right? The latter sounds like an upper bound to the number of acls you can have.

        (I would have committed the patch, but you didn't submit it

        Show
        Benjamin Reed added a comment - +1 Great job! Thanx Mahadev. Please fix one tiny thing before you commit (you don't need to do another patch): the comment of aclIndex should be "the number of acls that we have in the datatree" rather than "the max number of acls". Right? The latter sounds like an upper bound to the number of acls you can have. (I would have committed the patch, but you didn't submit it
        Hide
        Mahadev konar added a comment -

        changed the comment... thanks for the review ben...

        Show
        Mahadev konar added a comment - changed the comment... thanks for the review ben...
        Hide
        Mahadev konar added a comment -

        I just committed this.

        Show
        Mahadev konar added a comment - I just committed this.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #114 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/114/)
        . Better ACL management (Mahadev Konar)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #114 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/114/ ) . Better ACL management (Mahadev Konar)
        Hide
        Patrick Hunt added a comment -

        3.0.0 has been released, closing issues.

        Show
        Patrick Hunt added a comment - 3.0.0 has been released, closing issues.

          People

          • Assignee:
            Mahadev konar
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development