HBASE-11774 Avoid allocating unnecessary tag iterators

This commit is contained in:
Andrew Purtell 2014-08-19 09:38:33 -07:00
parent 4901e649b6
commit f3f3947397
6 changed files with 93 additions and 62 deletions

View File

@ -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());

View File

@ -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);
}
}
}

View File

@ -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) {

View File

@ -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

View File

@ -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;
}
}
}

View File

@ -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) {