HDFS-7454. Reduce memory footprint for AclEntries in NameNode. Contributed by Vinayakumar B.

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -175,6 +175,9 @@ Release 2.7.0 - UNRELEASED
 
   OPTIMIZATIONS
 
+    HDFS-7454. Reduce memory footprint for AclEntries in NameNode.
+    (Vinayakumar B via wheat9)
+
   BUG FIXES
 
     HDFS-6741. Improve permission denied message when

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclEntryStatusFormat.java

@@ -0,0 +1,136 @@
+/**
+ * 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.hadoop.hdfs.server.namenode;
+
+import java.util.List;
+
+import org.apache.hadoop.fs.permission.AclEntry;
+import org.apache.hadoop.fs.permission.AclEntryScope;
+import org.apache.hadoop.fs.permission.AclEntryType;
+import org.apache.hadoop.fs.permission.FsAction;
+import org.apache.hadoop.hdfs.util.LongBitFormat;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Class to pack an AclEntry into an integer. <br>
+ * An ACL entry is represented by a 32-bit integer in Big Endian format. <br>
+ * The bits can be divided in four segments: <br>
+ * [0:1) || [1:3) || [3:6) || [6:7) || [7:32) <br>
+ * <br>
+ * [0:1) -- the scope of the entry (AclEntryScope) <br>
+ * [1:3) -- the type of the entry (AclEntryType) <br>
+ * [3:6) -- the permission of the entry (FsAction) <br>
+ * [6:7) -- A flag to indicate whether Named entry or not <br>
+ * [7:32) -- the name of the entry, which is an ID that points to a <br>
+ * string in the StringTableSection. <br>
+ */
+public enum AclEntryStatusFormat {
+
+  SCOPE(null, 1),
+  TYPE(SCOPE.BITS, 2),
+  PERMISSION(TYPE.BITS, 3),
+  NAMED_ENTRY_CHECK(PERMISSION.BITS, 1),
+  NAME(NAMED_ENTRY_CHECK.BITS, 25);
+
+  private final LongBitFormat BITS;
+
+  private AclEntryStatusFormat(LongBitFormat previous, int length) {
+    BITS = new LongBitFormat(name(), previous, length, 0);
+  }
+
+  static AclEntryScope getScope(int aclEntry) {
+    int ordinal = (int) SCOPE.BITS.retrieve(aclEntry);
+    return AclEntryScope.values()[ordinal];
+  }
+
+  static AclEntryType getType(int aclEntry) {
+    int ordinal = (int) TYPE.BITS.retrieve(aclEntry);
+    return AclEntryType.values()[ordinal];
+  }
+
+  static FsAction getPermission(int aclEntry) {
+    int ordinal = (int) PERMISSION.BITS.retrieve(aclEntry);
+    return FsAction.values()[ordinal];
+  }
+
+  static String getName(int aclEntry) {
+    int nameExists = (int) NAMED_ENTRY_CHECK.BITS.retrieve(aclEntry);
+    if (nameExists == 0) {
+      return null;
+    }
+    int id = (int) NAME.BITS.retrieve(aclEntry);
+    AclEntryType type = getType(aclEntry);
+    if (type == AclEntryType.USER) {
+      return SerialNumberManager.INSTANCE.getUser(id);
+    } else if (type == AclEntryType.GROUP) {
+      return SerialNumberManager.INSTANCE.getGroup(id);
+    }
+    return null;
+  }
+
+  static int toInt(AclEntry aclEntry) {
+    long aclEntryInt = 0;
+    aclEntryInt = SCOPE.BITS
+        .combine(aclEntry.getScope().ordinal(), aclEntryInt);
+    aclEntryInt = TYPE.BITS.combine(aclEntry.getType().ordinal(), aclEntryInt);
+    aclEntryInt = PERMISSION.BITS.combine(aclEntry.getPermission().ordinal(),
+        aclEntryInt);
+    if (aclEntry.getName() != null) {
+      aclEntryInt = NAMED_ENTRY_CHECK.BITS.combine(1, aclEntryInt);
+      if (aclEntry.getType() == AclEntryType.USER) {
+        int userId = SerialNumberManager.INSTANCE.getUserSerialNumber(aclEntry
+            .getName());
+        aclEntryInt = NAME.BITS.combine(userId, aclEntryInt);
+      } else if (aclEntry.getType() == AclEntryType.GROUP) {
+        int groupId = SerialNumberManager.INSTANCE
+            .getGroupSerialNumber(aclEntry.getName());
+        aclEntryInt = NAME.BITS.combine(groupId, aclEntryInt);
+      }
+    }
+    return (int) aclEntryInt;
+  }
+
+  static AclEntry toAclEntry(int aclEntry) {
+    AclEntry.Builder builder = new AclEntry.Builder();
+    builder.setScope(getScope(aclEntry)).setType(getType(aclEntry))
+        .setPermission(getPermission(aclEntry));
+    if (getName(aclEntry) != null) {
+      builder.setName(getName(aclEntry));
+    }
+    return builder.build();
+  }
+
+  public static int[] toInt(List<AclEntry> aclEntries) {
+    int[] entries = new int[aclEntries.size()];
+    for (int i = 0; i < entries.length; i++) {
+      entries[i] = toInt(aclEntries.get(i));
+    }
+    return entries;
+  }
+
+  public static ImmutableList<AclEntry> toAclEntries(int[] entries) {
+    ImmutableList.Builder<AclEntry> b = new ImmutableList.Builder<AclEntry>();
+    for (int entry : entries) {
+      AclEntry aclEntry = toAclEntry(entry);
+      b.add(aclEntry);
+    }
+    return b.build();
+  }
+}

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclFeature.java

@@ -21,6 +21,7 @@ package org.apache.hadoop.hdfs.server.namenode;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.fs.permission.AclEntry;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 /**
@@ -31,13 +32,28 @@ public class AclFeature implements INode.Feature {
   public static final ImmutableList<AclEntry> EMPTY_ENTRY_LIST =
     ImmutableList.of();
 
-  private final ImmutableList<AclEntry> entries;
+  private final int [] entries;
 
-  public AclFeature(ImmutableList<AclEntry> entries) {
+  public AclFeature(int[] entries) {
     this.entries = entries;
   }
 
-  public ImmutableList<AclEntry> getEntries() {
-    return entries;
+  /**
+   * Get the number of entries present
+   */
+  int getEntriesSize() {
+    return entries.length;
+  }
+
+  /**
+   * Get the entry at the specified position
+   * @param pos Position of the entry to be obtained
+   * @return integer representation of AclEntry
+   * @throws IndexOutOfBoundsException if pos out of bound
+   */
+  int getEntryAt(int pos) {
+    Preconditions.checkPositionIndex(pos, entries.length,
+        "Invalid position for AclEntry");
+    return entries[pos];
   }
 }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclStorage.java

@@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.server.namenode;
 import java.util.Collections;
 import java.util.List;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
@@ -76,7 +77,8 @@ final class AclStorage {
     }
 
     // Split parent's entries into access vs. default.
-    List<AclEntry> featureEntries = parent.getAclFeature().getEntries();
+    List<AclEntry> featureEntries = getEntriesFromAclFeature(parent
+        .getAclFeature());
     ScopedAclEntries scopedEntries = new ScopedAclEntries(featureEntries);
     List<AclEntry> parentDefaultEntries = scopedEntries.getDefaultEntries();
 
@@ -153,7 +155,25 @@ final class AclStorage {
    */
   public static List<AclEntry> readINodeAcl(INode inode, int snapshotId) {
     AclFeature f = inode.getAclFeature(snapshotId);
-    return f == null ? ImmutableList.<AclEntry> of() : f.getEntries();
+    return getEntriesFromAclFeature(f);
+  }
+
+  /**
+   * Build list of AclEntries from the AclFeature
+   * @param aclFeature AclFeature
+   * @return List of entries
+   */
+  @VisibleForTesting
+  static ImmutableList<AclEntry> getEntriesFromAclFeature(AclFeature aclFeature) {
+    if (aclFeature == null) {
+      return ImmutableList.<AclEntry> of();
+    }
+    ImmutableList.Builder<AclEntry> b = new ImmutableList.Builder<AclEntry>();
+    for (int pos = 0, entry; pos < aclFeature.getEntriesSize(); pos++) {
+      entry = aclFeature.getEntryAt(pos);
+      b.add(AclEntryStatusFormat.toAclEntry(entry));
+    }
+    return b.build();
   }
 
   /**
@@ -179,7 +199,7 @@ final class AclStorage {
 
     final List<AclEntry> existingAcl;
     // Split ACL entries stored in the feature into access vs. default.
-    List<AclEntry> featureEntries = f.getEntries();
+    List<AclEntry> featureEntries = getEntriesFromAclFeature(f);
     ScopedAclEntries scoped = new ScopedAclEntries(featureEntries);
     List<AclEntry> accessEntries = scoped.getAccessEntries();
     List<AclEntry> defaultEntries = scoped.getDefaultEntries();
@@ -235,7 +255,7 @@ final class AclStorage {
     }
 
     FsPermission perm = inode.getFsPermission();
-    List<AclEntry> featureEntries = f.getEntries();
+    List<AclEntry> featureEntries = getEntriesFromAclFeature(f);
     if (featureEntries.get(0).getScope() == AclEntryScope.ACCESS) {
       // Restore group permissions from the feature's entry to permission
       // bits, overwriting the mask, which is not part of a minimal ACL.
@@ -330,7 +350,7 @@ final class AclStorage {
 
     // Add all default entries to the feature.
     featureEntries.addAll(defaultEntries);
-    return new AclFeature(ImmutableList.copyOf(featureEntries));
+    return new AclFeature(AclEntryStatusFormat.toInt(featureEntries));
   }
 
   /**

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java

@@ -156,8 +156,9 @@ public final class FSImageFormatPBINode {
       }
 
       if (d.hasAcl()) {
-        dir.addAclFeature(new AclFeature(loadAclEntries(d.getAcl(),
-            state.getStringTable())));
+        int[] entries = AclEntryStatusFormat.toInt(loadAclEntries(
+            d.getAcl(), state.getStringTable()));
+        dir.addAclFeature(new AclFeature(entries));
       }
       if (d.hasXAttrs()) {
         dir.addXAttrFeature(new XAttrFeature(
@@ -293,8 +294,9 @@ public final class FSImageFormatPBINode {
           (byte)f.getStoragePolicyID());
 
       if (f.hasAcl()) {
-        file.addAclFeature(new AclFeature(loadAclEntries(f.getAcl(),
-            state.getStringTable())));
+        int[] entries = AclEntryStatusFormat.toInt(loadAclEntries(
+            f.getAcl(), state.getStringTable()));
+        file.addAclFeature(new AclFeature(entries));
       }
       
       if (f.hasXAttrs()) {
@@ -359,11 +361,13 @@ public final class FSImageFormatPBINode {
     private static AclFeatureProto.Builder buildAclEntries(AclFeature f,
         final SaverContext.DeduplicationMap<String> map) {
       AclFeatureProto.Builder b = AclFeatureProto.newBuilder();
-      for (AclEntry e : f.getEntries()) {
-        int v = ((map.getId(e.getName()) & ACL_ENTRY_NAME_MASK) << ACL_ENTRY_NAME_OFFSET)
-            | (e.getType().ordinal() << ACL_ENTRY_TYPE_OFFSET)
-            | (e.getScope().ordinal() << ACL_ENTRY_SCOPE_OFFSET)
-            | (e.getPermission().ordinal());
+      for (int pos = 0, e; pos < f.getEntriesSize(); pos++) {
+        e = f.getEntryAt(pos);
+        int nameId = map.getId(AclEntryStatusFormat.getName(e));
+        int v = ((nameId & ACL_ENTRY_NAME_MASK) << ACL_ENTRY_NAME_OFFSET)
+            | (AclEntryStatusFormat.getType(e).ordinal() << ACL_ENTRY_TYPE_OFFSET)
+            | (AclEntryStatusFormat.getScope(e).ordinal() << ACL_ENTRY_SCOPE_OFFSET)
+            | (AclEntryStatusFormat.getPermission(e).ordinal());
         b.addEntries(v);
       }
       return b;

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java

@@ -20,14 +20,12 @@ package org.apache.hadoop.hdfs.server.namenode;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.Stack;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.fs.UnresolvedLinkException;
-import org.apache.hadoop.fs.permission.AclEntry;
 import org.apache.hadoop.fs.permission.AclEntryScope;
 import org.apache.hadoop.fs.permission.AclEntryType;
 import org.apache.hadoop.fs.permission.FsAction;
@@ -35,7 +33,6 @@ import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.util.ReadOnlyList;
 import org.apache.hadoop.security.AccessControlException;
 import org.apache.hadoop.security.UserGroupInformation;
-import org.apache.hadoop.util.StringUtils;
 
 /** 
  * Class that helps in checking file system permission.
@@ -50,12 +47,6 @@ class FSPermissionChecker {
   /** @return a string for throwing {@link AccessControlException} */
   private String toAccessControlString(INode inode, int snapshotId,
       FsAction access, FsPermission mode) {
-    return toAccessControlString(inode, snapshotId, access, mode, null);
-  }
-
-  /** @return a string for throwing {@link AccessControlException} */
-  private String toAccessControlString(INode inode, int snapshotId,
-      FsAction access, FsPermission mode, List<AclEntry> featureEntries) {
     StringBuilder sb = new StringBuilder("Permission denied: ")
       .append("user=").append(user).append(", ")
       .append("access=").append(access).append(", ")
@@ -64,9 +55,6 @@ class FSPermissionChecker {
       .append(inode.getGroupName(snapshotId)).append(':')
       .append(inode.isDirectory() ? 'd' : '-')
       .append(mode);
-    if (featureEntries != null) {
-      sb.append(':').append(StringUtils.join(",", featureEntries));
-    }
     return sb.toString();
   }
 
@@ -249,10 +237,10 @@ class FSPermissionChecker {
     FsPermission mode = inode.getFsPermission(snapshotId);
     AclFeature aclFeature = inode.getAclFeature(snapshotId);
     if (aclFeature != null) {
-      List<AclEntry> featureEntries = aclFeature.getEntries();
       // It's possible that the inode has a default ACL but no access ACL.
-      if (featureEntries.get(0).getScope() == AclEntryScope.ACCESS) {
-        checkAccessAcl(inode, snapshotId, access, mode, featureEntries);
+      int firstEntry = aclFeature.getEntryAt(0);
+      if (AclEntryStatusFormat.getScope(firstEntry) == AclEntryScope.ACCESS) {
+        checkAccessAcl(inode, snapshotId, access, mode, aclFeature);
         return;
       }
     }
@@ -294,11 +282,11 @@ class FSPermissionChecker {
    * @param snapshotId int snapshot ID
    * @param access FsAction requested permission
    * @param mode FsPermission mode from inode
-   * @param featureEntries List<AclEntry> ACL entries from AclFeature of inode
+   * @param aclFeature AclFeature of inode
    * @throws AccessControlException if the ACL denies permission
    */
   private void checkAccessAcl(INode inode, int snapshotId, FsAction access,
-      FsPermission mode, List<AclEntry> featureEntries)
+      FsPermission mode, AclFeature aclFeature)
       throws AccessControlException {
     boolean foundMatch = false;
 
@@ -312,17 +300,19 @@ class FSPermissionChecker {
 
     // Check named user and group entries if user was not denied by owner entry.
     if (!foundMatch) {
-      for (AclEntry entry: featureEntries) {
-        if (entry.getScope() == AclEntryScope.DEFAULT) {
+      for (int pos = 0, entry; pos < aclFeature.getEntriesSize(); pos++) {
+        entry = aclFeature.getEntryAt(pos);
+        if (AclEntryStatusFormat.getScope(entry) == AclEntryScope.DEFAULT) {
           break;
         }
-        AclEntryType type = entry.getType();
-        String name = entry.getName();
+        AclEntryType type = AclEntryStatusFormat.getType(entry);
+        String name = AclEntryStatusFormat.getName(entry);
         if (type == AclEntryType.USER) {
           // Use named user entry with mask from permission bits applied if user
           // matches name.
           if (user.equals(name)) {
-            FsAction masked = entry.getPermission().and(mode.getGroupAction());
+            FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
+                mode.getGroupAction());
             if (masked.implies(access)) {
               return;
             }
@@ -336,7 +326,8 @@ class FSPermissionChecker {
           // it doesn't matter which is chosen, so exit early after first match.
           String group = name == null ? inode.getGroupName(snapshotId) : name;
           if (groups.contains(group)) {
-            FsAction masked = entry.getPermission().and(mode.getGroupAction());
+            FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
+                mode.getGroupAction());
             if (masked.implies(access)) {
               return;
             }
@@ -352,7 +343,7 @@ class FSPermissionChecker {
     }
 
     throw new AccessControlException(
-      toAccessControlString(inode, snapshotId, access, mode, featureEntries));
+      toAccessControlString(inode, snapshotId, access, mode));
   }
 
   /** Guarded by {@link FSNamesystem#readLock()} */

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java

@@ -36,6 +36,7 @@ import java.util.Map;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.fs.permission.PermissionStatus;
+import org.apache.hadoop.hdfs.server.namenode.AclEntryStatusFormat;
 import org.apache.hadoop.hdfs.server.namenode.AclFeature;
 import org.apache.hadoop.hdfs.server.namenode.FSDirectory;
 import org.apache.hadoop.hdfs.server.namenode.FSImageFormatPBINode;
@@ -208,8 +209,10 @@ public class FSImageFormatPBSnapshot {
 
           AclFeature acl = null;
           if (fileInPb.hasAcl()) {
-            acl = new AclFeature(FSImageFormatPBINode.Loader.loadAclEntries(
-                fileInPb.getAcl(), state.getStringTable()));
+            int[] entries = AclEntryStatusFormat
+                .toInt(FSImageFormatPBINode.Loader.loadAclEntries(
+                    fileInPb.getAcl(), state.getStringTable()));
+            acl = new AclFeature(entries);
           }
           XAttrFeature xAttrs = null;
           if (fileInPb.hasXAttrs()) {
@@ -310,8 +313,10 @@ public class FSImageFormatPBSnapshot {
               dirCopyInPb.getPermission(), state.getStringTable());
           AclFeature acl = null;
           if (dirCopyInPb.hasAcl()) {
-            acl = new AclFeature(FSImageFormatPBINode.Loader.loadAclEntries(
-                dirCopyInPb.getAcl(), state.getStringTable()));
+            int[] entries = AclEntryStatusFormat
+                .toInt(FSImageFormatPBINode.Loader.loadAclEntries(
+                    dirCopyInPb.getAcl(), state.getStringTable()));
+            acl = new AclFeature(entries);
           }
           XAttrFeature xAttrs = null;
           if (dirCopyInPb.hasXAttrs()) {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java

@@ -1395,8 +1395,8 @@ public abstract class FSAclBaseTest {
       // Intentionally capturing a reference to the entries, not using nested
       // calls.  This way, we get compile-time enforcement that the entries are
       // stored in an ImmutableList.
-      ImmutableList<AclEntry> entries = aclFeature.getEntries();
-      assertNotNull(entries);
+      ImmutableList<AclEntry> entries = AclStorage
+          .getEntriesFromAclFeature(aclFeature);
       assertFalse(entries.isEmpty());
     } else {
       assertNull(aclFeature);