ZOOKEEPER-33. Better ACL management (Mahadev Konar)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@704592 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -136,3 +136,4 @@ Backward compatibile changes:
 
  ZOOKEEPER-172. FLE Test (Flavio Junqueira via breed)
 
+ ZOOKEEPER-33. Better ACL management (Mahadev Konar)

src/java/main/org/apache/zookeeper/server/DataNode.java

@@ -43,7 +43,7 @@ public class DataNode implements Record {
         // default rather than public constructor
     }
 
-    DataNode(DataNode parent, byte data[], List<ACL> acl, StatPersisted stat) {
+    DataNode(DataNode parent, byte data[], Long acl, StatPersisted stat) {
         this.parent = parent;
         this.data = data;
         this.acl = acl;
@@ -55,7 +55,7 @@ public class DataNode implements Record {
 
     byte data[];
 
-    List<ACL> acl;
+    Long acl;
 
     public StatPersisted stat;
 
@@ -78,17 +78,7 @@ public class DataNode implements Record {
             throws IOException {
         archive.startRecord("node");
         data = archive.readBuffer("data");
-        Index i = archive.startVector("acl");
-        if (i != null) {
-            acl = new ArrayList<ACL>();
-            while (!i.done()) {
-                ACL a = new ACL();
-                a.deserialize(archive, "aclEntry");
-                acl.add(a);
-                i.incr();
-            }
-        }
-        archive.endVector("acl");
+        acl = archive.readLong("acl");
         stat = new StatPersisted();
         stat.deserialize(archive, "statpersisted");
         archive.endRecord("node");
@@ -98,14 +88,12 @@ public class DataNode implements Record {
             throws IOException {
         archive.startRecord(this, "node");
         archive.writeBuffer(data, "data");
-        archive.startVector(acl, "acl");
-        if (acl != null) {
-            for (ACL a : acl) {
-                a.serialize(archive, "aclEntry");
-            }
-        }
-        archive.endVector(acl, "acl");
+        archive.writeLong(acl, "acl");
         stat.serialize(archive, "statpersisted");
         archive.endRecord(this, "node");
     }
+    
+    public int compareTo(Object o) {
+        return -1;
+    }
 }

src/java/main/org/apache/zookeeper/server/DataTree.java

@@ -21,11 +21,15 @@ package org.apache.zookeeper.server;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.apache.jute.Index;
 import org.apache.jute.InputArchive;
 import org.apache.jute.OutputArchive;
 import org.apache.jute.Record;
@@ -73,6 +77,25 @@ public class DataTree {
      */
     private ConcurrentHashMap<Long, HashSet<String>> ephemerals = new ConcurrentHashMap<Long, HashSet<String>>();
 
+    /**
+     * this is map from longs to acl's. It saves acl's being stored 
+     * for each datanode.
+     */
+    public Map<Long, List<ACL>> longKeyMap =
+                             new HashMap<Long, List<ACL>>();
+    
+    /**
+     * this a map from acls to long. 
+     */
+    public Map<List<ACL>, Long> aclKeyMap = 
+                            new HashMap<List<ACL>, Long>();
+    
+     /**
+     * these are the number of acls 
+     * that we have in the datatree
+     */
+    protected long aclIndex = 0;
+    
     /** A debug string * */
     private String debug = "debug";
 
@@ -88,7 +111,69 @@ public class DataTree {
         }
         return cloned;
     }
-
+    
+    private long incrementIndex() {
+       return ++aclIndex;
+    }
+    
+    /**
+     * compare two list of acls. if there elements are in the same
+     * order and the same size then return true else return false
+     * @param lista the list to be compared
+     * @param listb the list to be compared
+     * @return true if and only if the lists are of the same size
+     * and the elements are in the same order in lista and listb
+     */
+    private boolean listACLEquals(List<ACL> lista, List<ACL> listb) {
+        if (lista.size() != listb.size()) {
+            return false;
+        }
+        for (int i = 0; i < lista.size(); i++) {
+            ACL a = lista.get(i);
+            ACL b = listb.get(i);
+            if (!a.equals(b)) {
+                return false;
+            }
+        }
+        return true;
+    }
+    
+    /**
+     * converts the list of acls to a list of 
+     * longs. 
+     * @param acls
+     * @return a list of longs that map to the acls
+     */
+    public synchronized Long convertAcls(List<ACL> acls) {
+        if (acls == null)
+            return -1L;
+        // get the value from the map 
+        Long ret = aclKeyMap.get(acls);
+        // could not find the map
+        if (ret != null)
+            return ret;
+        long val = incrementIndex();
+        longKeyMap.put(val,acls);
+        aclKeyMap.put(acls, val);
+        return val;
+    }
+    
+    /**
+     * converts a list of longs to a list of acls. 
+     * @param longs the list of longs 
+     * @return a list of ACLs that map to longs
+     */
+    public synchronized List<ACL> convertLong(Long longVal) {
+        if (longVal == null || longVal == -1L) 
+            return null;
+        List<ACL> acls = longKeyMap.get(longVal);
+        if (acls == null) {
+            LOG.error("ERROR: ACL not available for long " + longVal);
+            throw new RuntimeException("Failed to fetch acls for " + longVal);
+        }
+        return acls;
+    }
+    
     public Collection<Long> getSessions() {
         return ephemerals.keySet();
     }
@@ -110,7 +195,7 @@ public class DataTree {
      * but we usually use the nodes hashmap to find nodes in the tree.
      */
     private DataNode root =
-        new DataNode(null, new byte[0], null, new StatPersisted());
+        new DataNode(null, new byte[0], -1L, new StatPersisted());
 
     public DataTree() {
         /* Rather than fight it, let root have an alias */
@@ -171,7 +256,8 @@ public class DataTree {
      * @throws KeeperException
      */
     public String createNode(String path, byte data[], List<ACL> acl,
-            long ephemeralOwner, long zxid, long time) throws KeeperException.NoNodeException, KeeperException.NodeExistsException {
+            long ephemeralOwner, long zxid, long time) 
+        throws KeeperException.NoNodeException, KeeperException.NodeExistsException {
         int lastSlash = path.lastIndexOf('/');
         String parentName = path.substring(0, lastSlash);
         String childName = path.substring(lastSlash + 1);
@@ -194,7 +280,8 @@ public class DataTree {
             int cver = parent.stat.getCversion();
             cver++;
             parent.stat.setCversion(cver);
-            DataNode child = new DataNode(parent, data, acl, stat);
+            Long longval = convertAcls(acl);
+            DataNode child = new DataNode(parent, data, longval, stat);
             parent.children.add(childName);
             nodes.put(path, child);
             if (ephemeralOwner != 0) {
@@ -213,6 +300,11 @@ public class DataTree {
         return path;
     }
 
+    /**
+     * remove the path from the datatree
+     * @param path the path to be deleted
+     * @throws KeeperException.NoNodeException
+     */
     public void deleteNode(String path) throws KeeperException.NoNodeException {
         int lastSlash = path.lastIndexOf('/');
         String parentName = path.substring(0, lastSlash);
@@ -323,7 +415,7 @@ public class DataTree {
         }
         synchronized (n) {
             n.stat.setAversion(version);
-            n.acl = acl;
+            n.acl = convertAcls(acl);
             n.copyStat(stat);
             return stat;
         }
@@ -337,7 +429,7 @@ public class DataTree {
         }
         synchronized (n) {
             n.copyStat(stat);
-            return new ArrayList<ACL>(n.acl);
+            return new ArrayList<ACL>(convertLong(n.acl));
         }
     }
 
@@ -507,9 +599,48 @@ public class DataTree {
     int scount;
 
     public boolean initialized = false;
-
+    
+    private void deserializeList(Map<Long, List<ACL>> longKeyMap, InputArchive ia) 
+            throws IOException {
+        int i = ia.readInt("map");
+        while (i > 0) {
+            Long val = ia.readLong("long");
+            if (aclIndex < val) {
+                aclIndex = val;
+            }
+            List<ACL> aclList = new ArrayList<ACL>();
+            Index j = ia.startVector("acls");
+            while (!j.done()) {
+                ACL acl = new ACL();
+                acl.deserialize(ia, "acl");
+                aclList.add(acl);
+                j.incr();
+            }
+            longKeyMap.put(val, aclList);
+            aclKeyMap.put(aclList, val);
+            i--;
+        }
+    }
+    
+    private synchronized void serializeList(Map<Long, List<ACL>> longKeyMap, 
+            OutputArchive oa) 
+            throws IOException {
+        oa.writeInt(longKeyMap.size(), "map");
+        Set<Map.Entry<Long, List<ACL>>> set = longKeyMap.entrySet();
+        for (Map.Entry<Long, List<ACL>> val : set) {
+            oa.writeLong(val.getKey(), "long");
+            List<ACL> aclList = val.getValue();
+            oa.startVector(aclList, "acls");
+            for (ACL acl: aclList) {
+                acl.serialize(oa, "acl");
+            }
+            oa.endVector(aclList, "acls");
+        }
+    }
+    
     public void serialize(OutputArchive oa, String tag) throws IOException {
         scount = 0;
+        serializeList(longKeyMap, oa);
         serializeNode(oa, new StringBuilder(""));
         // / marks end of stream
         // we need to check if clear had been called in between the snapshot.
@@ -519,6 +650,7 @@ public class DataTree {
     }
 
     public void deserialize(InputArchive ia, String tag) throws IOException {
+        deserializeList(longKeyMap, ia);
         nodes.clear();
         String path = ia.readString("path");
         while (!path.equals("/")) {

src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java

@@ -187,7 +187,8 @@ public class FinalRequestProcessor implements RequestProcessor {
                 if (n == null) {
                     throw new KeeperException.NoNodeException();
                 }
-                PrepRequestProcessor.checkACL(zks, n.acl, ZooDefs.Perms.READ,
+                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(n.acl),
+                        ZooDefs.Perms.READ,
                         request.authInfo);
                 stat = new Stat();
                 byte b[] = zks.dataTree.getData(getDataRequest.getPath(), stat,
@@ -212,7 +213,8 @@ public class FinalRequestProcessor implements RequestProcessor {
                 if (n == null) {
                     throw new KeeperException.NoNodeException();
                 }
-                PrepRequestProcessor.checkACL(zks, n.acl, ZooDefs.Perms.READ,
+                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(n.acl), 
+                        ZooDefs.Perms.READ,
                         request.authInfo);
                 List<String> children = zks.dataTree.getChildren(
                         getChildrenRequest.getPath(), stat, getChildrenRequest

src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java

@@ -109,6 +109,10 @@ public class NIOServerCnxn implements Watcher, ServerCnxn {
             }
         }
 
+        public ZooKeeperServer getZooKeeperServer() {
+            return this.zks;
+        }
+ 
         public InetSocketAddress getLocalAddress(){
             return (InetSocketAddress)ss.socket().getLocalSocketAddress();
         }

src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java

@@ -119,7 +119,7 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
                 DataNode n = zks.dataTree.getNode(path);
                 if (n != null) {
                     lastChange = new ChangeRecord(-1, path, n.stat, n.children
-                            .size(), n.acl);
+                            .size(), zks.dataTree.convertLong(n.acl));
                 }
             }
         }

src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java

@@ -63,7 +63,7 @@ public class FileSnap implements SnapShot {
         }
         InputStream snapIS = new BufferedInputStream(new FileInputStream(snap));
         InputArchive ia=BinaryInputArchive.getArchive(snapIS);
-        deserialize(dt,sessions,ia);
+        deserialize(dt,sessions, ia);
         snapIS.close();
         dt.lastProcessedZxid = Util.getZxidFromName(snap.getName(), "snapshot");
         return dt.lastProcessedZxid;

src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java

@@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.apache.jute.Record;
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.zookeeper.data.ACL;
 import org.apache.zookeeper.server.DataTree;
 import org.apache.zookeeper.server.Request;
 import org.apache.zookeeper.server.ZooTrace;
@@ -165,7 +166,8 @@ public class FileTxnSnapLog {
      * @throws IOException
      */
     public void save(DataTree dataTree,
-            ConcurrentHashMap<Long, Integer> sessionsWithTimeouts) throws IOException {
+            ConcurrentHashMap<Long, Integer> sessionsWithTimeouts)
+        throws IOException {
         long lastZxid = dataTree.lastProcessedZxid;
         LOG.info("Snapshotting: " + Long.toHexString(lastZxid));
         File snapshot=new File(

src/java/main/org/apache/zookeeper/server/persistence/SnapShot.java

@@ -47,7 +47,8 @@ public interface SnapShot {
      * @param sessions 
      * @throws IOException
      */
-    void serialize(DataTree dt, Map<Long, Integer> sessions, File name) 
+    void serialize(DataTree dt, Map<Long, Integer> sessions, 
+            File name) 
         throws IOException;
     
     /**

src/java/main/org/apache/zookeeper/server/persistence/Util.java

@@ -32,7 +32,6 @@ import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Properties;
-import java.util.Set;
 
 import org.apache.jute.BinaryOutputArchive;
 import org.apache.jute.InputArchive;

src/java/test/org/apache/zookeeper/test/ACLTest.java

@@ -0,0 +1,155 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import junit.framework.TestCase;
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.server.DataTree;
+import org.apache.zookeeper.server.NIOServerCnxn;
+import org.apache.zookeeper.server.ServerStats;
+import org.apache.zookeeper.server.SyncRequestProcessor;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+public class ACLTest extends TestCase implements Watcher {
+    private static final Logger LOG = Logger.getLogger(ACLTest.class);
+    private static String HOSTPORT = "127.0.0.1:2355";
+    ZooKeeperServer zks;
+    private CountDownLatch startSignal;
+    
+    @Override
+    protected void setUp() throws Exception {
+        LOG.info("STARTING " + getName());
+        ServerStats.registerAsConcrete();
+    }
+    @Override
+    protected void tearDown() throws Exception {
+        ServerStats.unregister();
+        LOG.info("FINISHED " + getName());
+    }
+
+    /**
+     * Verify that acl optimization of storing just 
+     * a few acls and there references in the data 
+     * node is actually working.
+     */
+    public void testAcls() throws Exception {
+        File tmpDir = ClientBase.createTmpDir();
+        ClientBase.setupTestEnv();
+        zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
+        SyncRequestProcessor.snapCount = 1000;
+        final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+        NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
+        f.startup(zks);
+        LOG.info("starting up the zookeeper server .. waiting");
+        assertTrue("waiting for server being up", 
+                ClientBase.waitForServerUp(HOSTPORT,CONNECTION_TIMEOUT));
+        ZooKeeper zk = new ZooKeeper(HOSTPORT, 20000, this);
+        String path;
+        LOG.info("starting creating acls");
+        for (int i = 0; i < 100; i++) {
+            path = "/" + i;
+            zk.create(path, path.getBytes(), Ids.OPEN_ACL_UNSAFE,
+                    CreateMode.PERSISTENT);
+        }
+        assertTrue("size of the acl map ", (1 == zks.dataTree.longKeyMap.size()));
+        for (int j =100; j < 200; j++) {
+            path = "/" + j;
+            ACL acl = new ACL();
+            acl.setPerms(0);
+            Id id = new Id();
+            id.setId(j + "");
+            id.setScheme("host");
+            acl.setId(id);
+            ArrayList<ACL> list = new ArrayList<ACL>();
+            list.add(acl);
+            zk.create(path, path.getBytes(), list, CreateMode.PERSISTENT);
+        }
+        assertTrue("size of the acl map ", (101 == zks.dataTree.longKeyMap.size()));
+        // now shutdown the server and restart it
+        f.shutdown();
+        assertTrue("waiting for server down",
+                ClientBase.waitForServerDown(HOSTPORT,
+                        CONNECTION_TIMEOUT));
+        startSignal = new CountDownLatch(1);
+
+        zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
+        f = new NIOServerCnxn.Factory(PORT);
+        
+        f.startup(zks);
+
+        assertTrue("waiting for server up",
+                   ClientBase.waitForServerUp(HOSTPORT,
+                                       CONNECTION_TIMEOUT));
+        
+        startSignal.await(CONNECTION_TIMEOUT,
+                TimeUnit.MILLISECONDS);
+        assertTrue("count == 0", startSignal.getCount() == 0);
+        
+        assertTrue("acl map ", (101 == zks.dataTree.longKeyMap.size()));
+        for (int j =200; j < 205; j++) {
+            path = "/" + j;
+            ACL acl = new ACL();
+            acl.setPerms(0);
+            Id id = new Id();
+            id.setId(j + "");
+            id.setScheme("host");
+            acl.setId(id);
+            ArrayList<ACL> list = new ArrayList<ACL>();
+            list.add(acl);
+            zk.create(path, path.getBytes(), list, CreateMode.PERSISTENT);
+        }
+        assertTrue("acl map ", (106 == zks.dataTree.longKeyMap.size()));
+
+        f.shutdown();
+
+        assertTrue("waiting for server down",
+                   ClientBase.waitForServerDown(HOSTPORT,
+                           ClientBase.CONNECTION_TIMEOUT));
+        
+    }
+    
+    /*                  
+     * (non-Javadoc)    
+     *                          
+     * @see org.apache.zookeeper.Watcher#process(org.apache.zookeeper.WatcherEvent)
+     */         
+    public void process(WatchedEvent event) {
+        LOG.info("Event:" + event.getState() + " " + event.getType() + " " + event.getPath());
+        if (event.getState() == KeeperState.SyncConnected
+                && startSignal != null && startSignal.getCount() > 0)
+        {              
+            startSignal.countDown();      
+        }
+    }
+}

src/java/test/org/apache/zookeeper/test/ClientBase.java

@@ -51,7 +51,6 @@ public abstract class ClientBase extends TestCase {
     protected String hostPort = "127.0.0.1:33221";
     protected NIOServerCnxn.Factory serverFactory = null;
     protected File tmpDir = null;
-
     public ClientBase() {
         super();
     }

src/zookeeper.jute

@@ -21,7 +21,7 @@ module org.apache.zookeeper.data {
         ustring scheme;
         ustring id;
     }
-    class ACL {
+   class ACL {
         int perms;
         Id id;
     }