From 4d385d1509d2b20fb0e455c20f28cf0d3b059001 Mon Sep 17 00:00:00 2001 From: anoopsjohn Date: Sat, 25 Oct 2014 21:13:49 +0530 Subject: [PATCH] HBASE-11870 Optimization : Avoid copy of key and value for tags addition in AC and VC. --- .../hadoop/hbase/SettableSequenceId.java | 4 +- .../hadoop/hbase/SettableTimestamp.java | 6 +- .../java/org/apache/hadoop/hbase/Tag.java | 21 +- .../apache/hadoop/hbase/TagRewriteCell.java | 202 ++++++++++++++++++ .../hbase/regionserver/wal/HLogSplitter.java | 7 +- .../security/access/AccessController.java | 22 +- .../visibility/VisibilityController.java | 24 +-- 7 files changed, 242 insertions(+), 44 deletions(-) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/TagRewriteCell.java diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableSequenceId.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableSequenceId.java index ffb347031ba..352028a62d8 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableSequenceId.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableSequenceId.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hbase; +import java.io.IOException; + import org.apache.hadoop.hbase.classification.InterfaceAudience; /** @@ -30,5 +32,5 @@ public interface SettableSequenceId { * Sets with the given seqId. * @param seqId */ - void setSequenceId(long seqId); + void setSequenceId(long seqId) throws IOException; } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableTimestamp.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableTimestamp.java index c9675c2e88c..6dac5ae0bca 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableTimestamp.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/SettableTimestamp.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hbase; +import java.io.IOException; + import org.apache.hadoop.hbase.classification.InterfaceAudience; /** @@ -30,12 +32,12 @@ public interface SettableTimestamp { * Sets with the given timestamp. * @param ts */ - void setTimestamp(long ts); + void setTimestamp(long ts) throws IOException; /** * Sets with the given timestamp. * @param ts buffer containing the timestamp value * @param tsOffset offset to the new timestamp */ - void setTimestamp(byte[] ts, int tsOffset); + void setTimestamp(byte[] ts, int tsOffset) throws IOException; } 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 8d3c0b93af6..644173c8ef5 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 @@ -173,7 +173,26 @@ public class Tag { } return tags; } - + + /** + * Write a list of tags into a byte array + * @param tags + * @return the serialized tag data as bytes + */ + public static byte[] fromList(List tags) { + int length = 0; + for (Tag tag: tags) { + length += tag.length; + } + byte[] b = new byte[length]; + int pos = 0; + for (Tag tag: tags) { + System.arraycopy(tag.bytes, tag.offset, b, pos, tag.length); + pos += tag.length; + } + return b; + } + /** * Retrieve the first tag from the tags byte array matching the passed in tag type * @param b diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/TagRewriteCell.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/TagRewriteCell.java new file mode 100644 index 00000000000..313ecb8b586 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/TagRewriteCell.java @@ -0,0 +1,202 @@ +/** + * 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.hbase; + +import java.io.IOException; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.io.HeapSize; +import org.apache.hadoop.hbase.util.ClassSize; + +/** + * This can be used when a Cell has to change with addition/removal of one or more tags. This is an + * efficient way to do so in which only the tags bytes part need to recreated and copied. All other + * parts, refer to the original Cell. + */ +@InterfaceAudience.Private +public class TagRewriteCell implements Cell, SettableSequenceId, SettableTimestamp, HeapSize { + + private Cell cell; + private byte[] tags; + + /** + * @param cell The original Cell which it rewrites + * @param tags the tags bytes. The array suppose to contain the tags bytes alone. + */ + public TagRewriteCell(Cell cell, byte[] tags) { + assert cell instanceof SettableSequenceId; + assert cell instanceof SettableTimestamp; + this.cell = cell; + this.tags = tags; + // tag offset will be treated as 0 and length this.tags.length + if (this.cell instanceof TagRewriteCell) { + // Cleaning the ref so that the byte[] can be GCed + ((TagRewriteCell) this.cell).tags = null; + } + } + + @Override + public byte[] getRowArray() { + return cell.getRowArray(); + } + + @Override + public int getRowOffset() { + return cell.getRowOffset(); + } + + @Override + public short getRowLength() { + return cell.getRowLength(); + } + + @Override + public byte[] getFamilyArray() { + return cell.getFamilyArray(); + } + + @Override + public int getFamilyOffset() { + return cell.getFamilyOffset(); + } + + @Override + public byte getFamilyLength() { + return cell.getFamilyLength(); + } + + @Override + public byte[] getQualifierArray() { + return cell.getQualifierArray(); + } + + @Override + public int getQualifierOffset() { + return cell.getQualifierOffset(); + } + + @Override + public int getQualifierLength() { + return cell.getQualifierLength(); + } + + @Override + public long getTimestamp() { + return cell.getTimestamp(); + } + + @Override + public byte getTypeByte() { + return cell.getTypeByte(); + } + + @Override + @Deprecated + public long getMvccVersion() { + return getSequenceId(); + } + + @Override + public long getSequenceId() { + return cell.getSequenceId(); + } + + @Override + public byte[] getValueArray() { + return cell.getValueArray(); + } + + @Override + public int getValueOffset() { + return cell.getValueOffset(); + } + + @Override + public int getValueLength() { + return cell.getValueLength(); + } + + @Override + public byte[] getTagsArray() { + return this.tags; + } + + @Override + public int getTagsOffset() { + return 0; + } + + @Override + public int getTagsLength() { + return this.tags.length; + } + + @Override + @Deprecated + public byte[] getValue() { + return cell.getValue(); + } + + @Override + @Deprecated + public byte[] getFamily() { + return cell.getFamily(); + } + + @Override + @Deprecated + public byte[] getQualifier() { + return cell.getQualifier(); + } + + @Override + @Deprecated + public byte[] getRow() { + return cell.getRow(); + } + + @Override + public long heapSize() { + long sum = CellUtil.estimatedHeapSizeOf(cell) - cell.getTagsLength(); + sum += ClassSize.OBJECT;// this object itself + sum += (2 * ClassSize.REFERENCE);// pointers to cell and tags array + if (this.tags != null) { + sum += ClassSize.align(ClassSize.ARRAY);// "tags" + sum += this.tags.length; + } + return sum; + } + + @Override + public void setTimestamp(long ts) throws IOException { + // The incoming cell is supposed to be SettableTimestamp type. + CellUtil.setTimestamp(cell, ts); + } + + @Override + public void setTimestamp(byte[] ts, int tsOffset) throws IOException { + // The incoming cell is supposed to be SettableTimestamp type. + CellUtil.setTimestamp(cell, ts, tsOffset); + } + + @Override + public void setSequenceId(long seqId) throws IOException { + // The incoming cell is supposed to be SettableSequenceId type. + CellUtil.setSequenceId(cell, seqId); + } +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java index 3fc72e72181..b057f5f60d9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java @@ -62,13 +62,13 @@ import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HRegionLocation; -import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.RemoteExceptionHandler; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.TableStateManager; import org.apache.hadoop.hbase.Tag; +import org.apache.hadoop.hbase.TagRewriteCell; import org.apache.hadoop.hbase.TagType; import org.apache.hadoop.hbase.client.ConnectionUtils; import org.apache.hadoop.hbase.client.Delete; @@ -1926,7 +1926,10 @@ public class HLogSplitter { List newTags = new ArrayList(); Tag replayTag = new Tag(TagType.LOG_REPLAY_TAG_TYPE, Bytes.toBytes(seqId)); newTags.add(replayTag); - return KeyValue.cloneAndAddTags(cell, newTags); + if (cell.getTagsLength() > 0) { + newTags.addAll(Tag.asList(cell.getTagsArray(), cell.getTagsOffset(), cell.getTagsLength())); + } + return new TagRewriteCell(cell, Tag.fromList(newTags)); } return cell; } 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 cf25702c176..bd91501ab1e 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 @@ -52,6 +52,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.Tag; +import org.apache.hadoop.hbase.TagRewriteCell; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Durability; @@ -771,13 +772,7 @@ public class AccessController extends BaseMasterAndRegionObserver tags.add(tagIterator.next()); } } - newCells.add( - new KeyValue(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength(), - cell.getFamilyArray(), cell.getFamilyOffset(), cell.getFamilyLength(), - cell.getQualifierArray(), cell.getQualifierOffset(), cell.getQualifierLength(), - cell.getTimestamp(), KeyValue.Type.codeToType(cell.getTypeByte()), - cell.getValueArray(), cell.getValueOffset(), cell.getValueLength(), - tags)); + newCells.add(new TagRewriteCell(cell, Tag.fromList(tags))); } // This is supposed to be safe, won't CME e.setValue(newCells); @@ -1754,17 +1749,8 @@ public class AccessController extends BaseMasterAndRegionObserver return newCell; } - // We need to create another KV, unfortunately, because the current new KV - // has no space for tags - KeyValue rewriteKv = new KeyValue(newCell.getRowArray(), newCell.getRowOffset(), - newCell.getRowLength(), newCell.getFamilyArray(), newCell.getFamilyOffset(), - newCell.getFamilyLength(), newCell.getQualifierArray(), newCell.getQualifierOffset(), - newCell.getQualifierLength(), newCell.getTimestamp(), KeyValue.Type.codeToType(newCell - .getTypeByte()), newCell.getValueArray(), newCell.getValueOffset(), - newCell.getValueLength(), tags); - // Preserve mvcc data - rewriteKv.setSequenceId(newCell.getSequenceId()); - return rewriteKv; + Cell rewriteCell = new TagRewriteCell(newCell, Tag.fromList(tags)); + return rewriteCell; } @Override 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 753a75f40f9..d880205546f 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 @@ -43,8 +43,7 @@ import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.KeyValue; -import org.apache.hadoop.hbase.KeyValue.Type; +import org.apache.hadoop.hbase.TagRewriteCell; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Tag; import org.apache.hadoop.hbase.MetaTableAccessor; @@ -315,12 +314,7 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements List tags = Tag.asList(cell.getTagsArray(), cell.getTagsOffset(), cell.getTagsLength()); tags.addAll(visibilityTags); - Cell updatedCell = new KeyValue(cell.getRowArray(), cell.getRowOffset(), - cell.getRowLength(), cell.getFamilyArray(), cell.getFamilyOffset(), - cell.getFamilyLength(), cell.getQualifierArray(), cell.getQualifierOffset(), - cell.getQualifierLength(), cell.getTimestamp(), Type.codeToType(cell - .getTypeByte()), cell.getValueArray(), cell.getValueOffset(), - cell.getValueLength(), tags); + Cell updatedCell = new TagRewriteCell(cell, Tag.fromList(tags)); updatedCells.add(updatedCell); } m.getFamilyCellMap().clear(); @@ -330,7 +324,6 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements Put p = (Put) m; p.add(cell); } else if (m instanceof Delete) { - // TODO : Cells without visibility tags would be handled in follow up issue Delete d = (Delete) m; d.addDeleteMarker(cell); } @@ -614,17 +607,8 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements } } - // We need to create another KV, unfortunately, because the current new KV - // has no space for tags - KeyValue rewriteKv = new KeyValue(newCell.getRowArray(), newCell.getRowOffset(), - newCell.getRowLength(), newCell.getFamilyArray(), newCell.getFamilyOffset(), - newCell.getFamilyLength(), newCell.getQualifierArray(), newCell.getQualifierOffset(), - newCell.getQualifierLength(), newCell.getTimestamp(), KeyValue.Type.codeToType(newCell - .getTypeByte()), newCell.getValueArray(), newCell.getValueOffset(), - newCell.getValueLength(), tags); - // Preserve mvcc data - rewriteKv.setSequenceId(newCell.getSequenceId()); - return rewriteKv; + Cell rewriteCell = new TagRewriteCell(newCell, Tag.fromList(tags)); + return rewriteCell; } @Override