You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2014/08/19 21:59:29 UTC
[1/3] git commit: HBASE-11774 Avoid allocating unnecessary tag
iterators
Repository: hbase
Updated Branches:
refs/heads/0.98 dbda5c38f -> e26411fc7
refs/heads/branch-1 4901e649b -> f3f394739
refs/heads/master 393a2a381 -> 3b864842c
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/3b864842
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/3b864842
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/3b864842
Branch: refs/heads/master
Commit: 3b864842c7058d0bb4da92fa73dade937ceb832d
Parents: 393a2a3
Author: Andrew Purtell <ap...@apache.org>
Authored: Tue Aug 19 09:38:33 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Aug 19 09:38: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 | 63 ++++++++++----------
.../security/visibility/VisibilityUtils.java | 11 +++-
6 files changed, 93 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/3b864842/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 c00be7b..19e5136 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
@@ -686,6 +686,10 @@ public class AccessControlLists {
public static List<Permission> getCellPermissionsForUser(User user, Cell cell)
throws IOException {
+ // Save an object allocation where we can
+ if (cell.getTagsLength() == 0) {
+ return null;
+ }
List<Permission> results = Lists.newArrayList();
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
http://git-wip-us.apache.org/repos/asf/hbase/blob/3b864842/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 a0dd438..53d3f35 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.getTagsLength() > 0) {
+ Iterator<Tag> tagIterator = CellUtil.tagsIterator(cell.getTagsArray(),
cell.getTagsOffset(), cell.getTagsLength());
- 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.
@@ -1694,21 +1697,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.getTagsLength() > 0) {
+ Iterator<Tag> tagIterator = CellUtil.tagsIterator(oldCell.getTagsArray(),
oldCell.getTagsOffset(), oldCell.getTagsLength());
- 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/3b864842/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/3b864842/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 de37690..fb62c2b 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.getTagsLength());
- 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.getTagsLength() > 0) {
+ // Carry forward all other tags
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(newCell.getTagsArray(), newCell.getTagsOffset(),
+ newCell.getTagsLength());
+ 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/3b864842/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 14d69ed..ea579ff 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.getTagsLength());
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.getTagsLength() > 0) {
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+ cell.getTagsLength());
+ 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;
}
}
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/3b864842/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 35843ff..f77eb96 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.getTagsLength() == 0) {
+ return sortedOrder;
+ }
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
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.getTagsLength() == 0) {
+ return false;
+ }
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
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.getTagsLength() == 0) {
+ return fullTagsList;
+ }
Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
- List<List<Integer>> fullTagsList = new ArrayList<List<Integer>>();
while (tagsItr.hasNext()) {
Tag tag = tagsItr.next();
if (tag.getType() == VisibilityUtils.VISIBILITY_TAG_TYPE) {
[2/3] git commit: HBASE-11774 Avoid allocating unnecessary tag
iterators
Posted by ap...@apache.org.
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/f3f39473
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/f3f39473
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/f3f39473
Branch: refs/heads/branch-1
Commit: f3f39473977682c5a968f57937d28d1eec041759
Parents: 4901e64
Author: Andrew Purtell <ap...@apache.org>
Authored: Tue Aug 19 09:38:33 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Aug 19 09:52:57 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 | 63 ++++++++++----------
.../security/visibility/VisibilityUtils.java | 11 +++-
6 files changed, 93 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/f3f39473/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 c00be7b..19e5136 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
@@ -686,6 +686,10 @@ public class AccessControlLists {
public static List<Permission> getCellPermissionsForUser(User user, Cell cell)
throws IOException {
+ // Save an object allocation where we can
+ if (cell.getTagsLength() == 0) {
+ return null;
+ }
List<Permission> results = Lists.newArrayList();
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
http://git-wip-us.apache.org/repos/asf/hbase/blob/f3f39473/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 a0dd438..53d3f35 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.getTagsLength() > 0) {
+ Iterator<Tag> tagIterator = CellUtil.tagsIterator(cell.getTagsArray(),
cell.getTagsOffset(), cell.getTagsLength());
- 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.
@@ -1694,21 +1697,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.getTagsLength() > 0) {
+ Iterator<Tag> tagIterator = CellUtil.tagsIterator(oldCell.getTagsArray(),
oldCell.getTagsOffset(), oldCell.getTagsLength());
- 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/f3f39473/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/f3f39473/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 70edf98..28208c2 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.getTagsLength());
- 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.getTagsLength() > 0) {
+ // Carry forward all other tags
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(newCell.getTagsArray(), newCell.getTagsOffset(),
+ newCell.getTagsLength());
+ 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/f3f39473/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 14d69ed..ea579ff 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.getTagsLength());
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.getTagsLength() > 0) {
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+ cell.getTagsLength());
+ 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;
}
}
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/f3f39473/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 35843ff..f77eb96 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.getTagsLength() == 0) {
+ return sortedOrder;
+ }
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
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.getTagsLength() == 0) {
+ return false;
+ }
Iterator<Tag> tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
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.getTagsLength() == 0) {
+ return fullTagsList;
+ }
Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
cell.getTagsLength());
- List<List<Integer>> fullTagsList = new ArrayList<List<Integer>>();
while (tagsItr.hasNext()) {
Tag tag = tagsItr.next();
if (tag.getType() == VisibilityUtils.VISIBILITY_TAG_TYPE) {
[3/3] git commit: HBASE-11774 Avoid allocating unnecessary tag
iterators
Posted by ap...@apache.org.
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 <ap...@apache.org>
Authored: Tue Aug 19 09:38:33 2014 -0700
Committer: Andrew Purtell <ap...@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) {