hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject [3/3] git commit: HBASE-11774 Avoid allocating unnecessary tag iterators
Date Tue, 19 Aug 2014 19:59:31 GMT
HBASE-11774 Avoid allocating unnecessary tag iterators


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e26411fc
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e26411fc
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e26411fc

Branch: refs/heads/0.98
Commit: e26411fc75d59c14f25cf9d1455618b34cba44c1
Parents: dbda5c3
Author: Andrew Purtell <apurtell@apache.org>
Authored: Tue Aug 19 09:38:33 2014 -0700
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Tue Aug 19 09:57:33 2014 -0700

----------------------------------------------------------------------
 .../security/access/AccessControlLists.java     |  4 ++
 .../hbase/security/access/AccessController.java | 40 +++++++-----
 .../hbase/security/access/TableAuthManager.java | 11 ++--
 .../visibility/VisibilityController.java        | 26 ++++----
 .../visibility/VisibilityLabelFilter.java       | 65 ++++++++++----------
 .../security/visibility/VisibilityUtils.java    | 11 +++-
 6 files changed, 94 insertions(+), 63 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
index 7206ce5..3644de1 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
@@ -683,6 +683,10 @@ public class AccessControlLists {
 
    public static List<Permission> getCellPermissionsForUser(User user, Cell cell)
        throws IOException {
+     // Save an object allocation where we can
+     if (cell.getTagsLengthUnsigned() == 0) {
+       return null;
+     }
      List<Permission> results = Lists.newArrayList();
      Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
         cell.getTagsLengthUnsigned());

http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 90cb88c..f2d588b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -758,11 +758,14 @@ public class AccessController extends BaseMasterAndRegionObserver
     for (Map.Entry<byte[], List<Cell>> e: familyMap.entrySet()) {
       List<Cell> newCells = Lists.newArrayList();
       for (Cell cell: e.getValue()) {
+        // Prepend the supplied perms in a new ACL tag to an update list of tags for the
cell
         List<Tag> tags = Lists.newArrayList(new Tag(AccessControlLists.ACL_TAG_TYPE,
perms));
-        Iterator<Tag> tagIterator = CellUtil.tagsIterator(cell.getTagsArray(),
+        if (cell.getTagsLengthUnsigned() > 0) {
+          Iterator<Tag> tagIterator = CellUtil.tagsIterator(cell.getTagsArray(),
             cell.getTagsOffset(), cell.getTagsLengthUnsigned());
-        while (tagIterator.hasNext()) {
-          tags.add(tagIterator.next());
+          while (tagIterator.hasNext()) {
+            tags.add(tagIterator.next());
+          }
         }
         // Ensure KeyValue so we can do a scatter gather copy. This is only a win if the
         // incoming cell type is actually KeyValue.
@@ -1683,21 +1686,26 @@ public class AccessController extends BaseMasterAndRegionObserver
     List<Tag> tags = Lists.newArrayList();
     ListMultimap<String,Permission> perms = ArrayListMultimap.create();
     if (oldCell != null) {
-      Iterator<Tag> tagIterator = CellUtil.tagsIterator(oldCell.getTagsArray(),
+      // Save an object allocation where we can
+      if (oldCell.getTagsLengthUnsigned() > 0) {
+        Iterator<Tag> tagIterator = CellUtil.tagsIterator(oldCell.getTagsArray(),
           oldCell.getTagsOffset(), oldCell.getTagsLengthUnsigned());
-      while (tagIterator.hasNext()) {
-        Tag tag = tagIterator.next();
-        if (tag.getType() != AccessControlLists.ACL_TAG_TYPE) {
-          if (LOG.isTraceEnabled()) {
-            LOG.trace("Carrying forward tag from " + oldCell + ": type " + tag.getType()
+
-              " length " + tag.getTagLength());
+        while (tagIterator.hasNext()) {
+          Tag tag = tagIterator.next();
+          if (tag.getType() != AccessControlLists.ACL_TAG_TYPE) {
+            // Not an ACL tag, just carry it through
+            if (LOG.isTraceEnabled()) {
+              LOG.trace("Carrying forward tag from " + oldCell + ": type " + tag.getType()
+
+                " length " + tag.getTagLength());
+            }
+            tags.add(tag);
+          } else {
+            // Merge the perms from the older ACL into the current permission set
+            ListMultimap<String,Permission> kvPerms = ProtobufUtil.toUsersAndPermissions(
+              AccessControlProtos.UsersAndPermissions.newBuilder().mergeFrom(
+                tag.getBuffer(), tag.getTagOffset(), tag.getTagLength()).build());
+            perms.putAll(kvPerms);
           }
-          tags.add(tag);
-        } else {
-          ListMultimap<String,Permission> kvPerms = ProtobufUtil.toUsersAndPermissions(
-            AccessControlProtos.UsersAndPermissions.newBuilder().mergeFrom(
-              tag.getBuffer(), tag.getTagOffset(), tag.getTagLength()).build());
-          perms.putAll(kvPerms);
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
index 8b0bb69..cbea05c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
@@ -356,11 +356,14 @@ public class TableAuthManager {
     try {
       List<Permission> perms = AccessControlLists.getCellPermissionsForUser(user, cell);
       if (LOG.isTraceEnabled()) {
-        LOG.trace("Perms for user " + user.getShortName() + " in cell " + cell + ": " + perms);
+        LOG.trace("Perms for user " + user.getShortName() + " in cell " + cell + ": " +
+          (perms != null ? perms : ""));
       }
-      for (Permission p: perms) {
-        if (p.implies(action)) {
-          return true;
+      if (perms != null) {
+        for (Permission p: perms) {
+          if (p.implies(action)) {
+            return true;
+          }
         }
       }
     } catch (IOException e) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
index 8e12708..3f9c7d5 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
@@ -925,22 +925,26 @@ public class VisibilityController extends BaseMasterAndRegionObserver
implements
     if (checkAuths && user != null && user.getShortName() != null) {
       auths = this.visibilityManager.getAuthsAsOrdinals(user.getShortName());
     }
-    // Adding all other tags
-    Iterator<Tag> tagsItr = CellUtil.tagsIterator(newCell.getTagsArray(), newCell.getTagsOffset(),
-        newCell.getTagsLengthUnsigned());
-    while (tagsItr.hasNext()) {
-      Tag tag = tagsItr.next();
-      if (tag.getType() != VisibilityUtils.VISIBILITY_TAG_TYPE
-          && tag.getType() != VisibilityUtils.VISIBILITY_EXP_SERIALIZATION_TAG_TYPE)
{
-        tags.add(tag);
-      }
-    }
+    // Prepend new visibility tags to a new list of tags for the cell
     try {
       tags.addAll(createVisibilityTags(cellVisibility.getExpression(), true, auths,
-          user.getShortName()));
+        user.getShortName()));
     } catch (ParseException e) {
       throw new IOException(e);
     }
+    // Save an object allocation where we can
+    if (newCell.getTagsLengthUnsigned() > 0) {
+      // Carry forward all other tags
+      Iterator<Tag> tagsItr = CellUtil.tagsIterator(newCell.getTagsArray(), newCell.getTagsOffset(),
+        newCell.getTagsLengthUnsigned());
+      while (tagsItr.hasNext()) {
+        Tag tag = tagsItr.next();
+        if (tag.getType() != VisibilityUtils.VISIBILITY_TAG_TYPE
+            && tag.getType() != VisibilityUtils.VISIBILITY_EXP_SERIALIZATION_TAG_TYPE)
{
+          tags.add(tag);
+        }
+      }
+    }
 
     // We need to create another KV, unfortunately, because the current new KV
     // has no space for tags

http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
index aa89a35..342d3ea 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
@@ -80,39 +80,42 @@ class VisibilityLabelFilter extends FilterBase {
       return ReturnCode.SKIP;
     }
 
-    Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
-        cell.getTagsLengthUnsigned());
     boolean visibilityTagPresent = false;
-    while (tagsItr.hasNext()) {
-      boolean includeKV = true;
-      Tag tag = tagsItr.next();
-      if (tag.getType() == VisibilityUtils.VISIBILITY_TAG_TYPE) {
-        visibilityTagPresent = true;
-        int offset = tag.getTagOffset();
-        int endOffset = offset + tag.getTagLength();
-        while (offset < endOffset) {
-          Pair<Integer, Integer> result = StreamUtils.readRawVarint32(tag.getBuffer(),
offset);
-          int currLabelOrdinal = result.getFirst();
-          if (currLabelOrdinal < 0) {
-            // check for the absence of this label in the Scan Auth labels
-            // ie. to check BitSet corresponding bit is 0
-            int temp = -currLabelOrdinal;
-            if (this.authLabels.get(temp)) {
-              includeKV = false;
-              break;
-            }
-          } else {
-            if (!this.authLabels.get(currLabelOrdinal)) {
-              includeKV = false;
-              break;
+    // Save an object allocation where we can
+    if (cell.getTagsLengthUnsigned() > 0) {
+      Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+        cell.getTagsLengthUnsigned());
+      while (tagsItr.hasNext()) {
+        boolean includeKV = true;
+        Tag tag = tagsItr.next();
+        if (tag.getType() == VisibilityUtils.VISIBILITY_TAG_TYPE) {
+          visibilityTagPresent = true;
+          int offset = tag.getTagOffset();
+          int endOffset = offset + tag.getTagLength();
+          while (offset < endOffset) {
+            Pair<Integer, Integer> result = StreamUtils.readRawVarint32(tag.getBuffer(),
offset);
+            int currLabelOrdinal = result.getFirst();
+            if (currLabelOrdinal < 0) {
+              // check for the absence of this label in the Scan Auth labels
+              // ie. to check BitSet corresponding bit is 0
+              int temp = -currLabelOrdinal;
+              if (this.authLabels.get(temp)) {
+                includeKV = false;
+                break;
+              }
+            } else {
+              if (!this.authLabels.get(currLabelOrdinal)) {
+                includeKV = false;
+                break;
+              }
             }
+            offset += result.getSecond();
+          }
+          if (includeKV) {
+            // We got one visibility expression getting evaluated to true. Good to include
this KV in
+            // the result then.
+            return ReturnCode.INCLUDE;
           }
-          offset += result.getSecond();
-        }
-        if (includeKV) {
-          // We got one visibility expression getting evaluated to true. Good to include
this KV in
-          // the result then.
-          return ReturnCode.INCLUDE;
         }
       }
     }
@@ -126,4 +129,4 @@ class VisibilityLabelFilter extends FilterBase {
     this.curFamilyMaxVersions = 0;
     this.curQualMetVersions = 0;
   }
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/e26411fc/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
index 725efb3..f3c84e1 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
@@ -176,6 +176,9 @@ public class VisibilityUtils {
    */
   public static boolean getVisibilityTags(Cell cell, List<Tag> tags) {
     boolean sortedOrder = false;
+    if (cell.getTagsLengthUnsigned() == 0) {
+      return sortedOrder;
+    }
     Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
         cell.getTagsLengthUnsigned());
     while (tagsIterator.hasNext()) {
@@ -200,6 +203,9 @@ public class VisibilityUtils {
    * @return true if found, false if not found
    */
   public static boolean isVisibilityTagsPresent(Cell cell) {
+    if (cell.getTagsLengthUnsigned() == 0) {
+      return false;
+    }
     Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
         cell.getTagsLengthUnsigned());
     while (tagsIterator.hasNext()) {
@@ -270,9 +276,12 @@ public class VisibilityUtils {
   }
 
   private static List<List<Integer>> sortTagsBasedOnOrdinal(Cell cell) throws
IOException {
+    List<List<Integer>> fullTagsList = new ArrayList<List<Integer>>();
+    if (cell.getTagsLengthUnsigned() == 0) {
+      return fullTagsList;
+    }
     Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
         cell.getTagsLengthUnsigned());
-    List<List<Integer>> fullTagsList = new ArrayList<List<Integer>>();
     while (tagsItr.hasNext()) {
       Tag tag = tagsItr.next();
       if (tag.getType() == VisibilityUtils.VISIBILITY_TAG_TYPE) {


Mime
View raw message