From 7c1135b3e3f6796eb7884868fc1cd46c331c2bff Mon Sep 17 00:00:00 2001 From: anoopsjohn Date: Fri, 27 Jun 2014 22:35:08 +0530 Subject: [PATCH] HBASE-11424 Avoid usage of CellUtil#getTagArray(Cell cell) within server. (Anoop) --- .../apache/hadoop/hbase/protobuf/ProtobufUtil.java | 3 ++- .../main/java/org/apache/hadoop/hbase/CellUtil.java | 7 +++++++ .../src/main/java/org/apache/hadoop/hbase/Tag.java | 7 +++++++ .../hbase/security/access/AccessControlLists.java | 4 ++-- .../hbase/security/access/AccessController.java | 11 +++++------ 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java index f875e7ed890..40ab4724c90 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java @@ -1108,7 +1108,8 @@ public final class ProtobufUtil { kv.getValueArray(), kv.getValueOffset(), kv.getValueLength())); valueBuilder.setTimestamp(kv.getTimestamp()); if(cell.getTagsLength() > 0) { - valueBuilder.setTags(ByteString.copyFrom(CellUtil.getTagArray(kv))); + valueBuilder.setTags(HBaseZeroCopyByteString.wrap(kv.getTagsArray(), kv.getTagsOffset(), + kv.getTagsLength())); } if (type == MutationType.DELETE) { KeyValue.Type keyValueType = KeyValue.Type.codeToType(kv.getType()); diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java index 5acf4bd29eb..811de106088 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java @@ -83,6 +83,13 @@ public final class CellUtil { return output; } + /** + * Returns tag value in a new byte array. If server-side, use + * {@link Tag#getBuffer()} with appropriate {@link Tag#getTagOffset()} and + * {@link Tag#getTagLength()} instead to save on allocations. + * @param cell + * @return tag value in a new byte array. + */ public static byte[] getTagArray(Cell cell){ byte[] output = new byte[cell.getTagsLength()]; copyTagTo(cell, output, 0); diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/Tag.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/Tag.java index 225d6c2b9cb..332433f3cbf 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/Tag.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/Tag.java @@ -130,6 +130,13 @@ public class Tag { return this.offset + INFRASTRUCTURE_SIZE; } + /** + * Returns tag value in a new byte array. + * Primarily for use client-side. If server-side, use + * {@link #getBuffer()} with appropriate {@link #getTagOffset()} and {@link #getTagLength()} + * instead to save on allocations. + * @return tag value in a new byte array. + */ public byte[] getValue() { int tagLength = getTagLength(); byte[] tag = new byte[tagLength]; 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 972f4176d34..7dd83e1e9ff 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 @@ -685,8 +685,8 @@ public class AccessControlLists { public static List getCellPermissionsForUser(User user, Cell cell) throws IOException { List results = Lists.newArrayList(); - byte[] tags = CellUtil.getTagArray(cell); - Iterator tagsIterator = CellUtil.tagsIterator(tags, 0, tags.length); + Iterator tagsIterator = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(), + cell.getTagsLength()); while (tagsIterator.hasNext()) { Tag tag = tagsIterator.next(); if (tag.getType() == ACL_TAG_TYPE) { 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 c1f90b77318..455e8c21e5b 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 @@ -88,7 +88,6 @@ import org.apache.hadoop.hbase.regionserver.MiniBatchOperationInProgress; import org.apache.hadoop.hbase.regionserver.RegionScanner; import org.apache.hadoop.hbase.regionserver.ScanType; import org.apache.hadoop.hbase.regionserver.Store; -import org.apache.hadoop.hbase.regionserver.StoreFile; import org.apache.hadoop.hbase.regionserver.wal.WALEdit; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; @@ -753,8 +752,8 @@ public class AccessController extends BaseRegionObserver List newCells = Lists.newArrayList(); for (Cell cell: e.getValue()) { List tags = Lists.newArrayList(new Tag(AccessControlLists.ACL_TAG_TYPE, perms)); - byte[] tagBytes = CellUtil.getTagArray(cell); - Iterator tagIterator = CellUtil.tagsIterator(tagBytes, 0, tagBytes.length); + Iterator tagIterator = CellUtil.tagsIterator(cell.getTagsArray(), + cell.getTagsOffset(), cell.getTagsLength()); while (tagIterator.hasNext()) { tags.add(tagIterator.next()); } @@ -1808,14 +1807,14 @@ public class AccessController extends BaseRegionObserver List tags = Lists.newArrayList(); ListMultimap perms = ArrayListMultimap.create(); if (oldCell != null) { - byte[] tagBytes = CellUtil.getTagArray(oldCell); - Iterator tagIterator = CellUtil.tagsIterator(tagBytes, 0, tagBytes.length); + Iterator 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.getValue().length); + " length " + tag.getTagLength()); } tags.add(tag); } else {