Skip to content

HBASE-26777 BufferedDataBlockEncoder$OffheapDecodedExtendedCell.deepC… #4139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 4, 2022
Merged

HBASE-26777 BufferedDataBlockEncoder$OffheapDecodedExtendedCell.deepC… #4139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81918a4
HBASE-26777 BufferedDataBlockEncoder$OffheapDecodedExtendedCell.deepC…
stoty Feb 28, 2022
8b7d2a1
copy the Cell to a KeyValue explicitly
stoty Mar 1, 2022
47e8ad2
use KeyValueUtil.copyToNewKeyValue, which should be faster.
stoty Mar 1, 2022
904daa2
add test
stoty Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ public static byte[] copyToNewByteArray(final Cell cell) {
//Cell#getSerializedSize returns the serialized size of the Source cell, which may
//not serialize all fields. We are constructing a KeyValue backing array here,
//which does include all fields, and must allocate accordingly.
//TODO we could probably use Cell#getSerializedSize safely, the errors were
//caused by cells corrupted by use-after-free bugs
int v1Length = length(cell.getRowLength(), cell.getFamilyLength(),
cell.getQualifierLength(), cell.getValueLength(), cell.getTagsLength(), true);
byte[] backingBytes = new byte[v1Length];
Expand Down
3 changes: 2 additions & 1 deletion hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
import org.apache.hadoop.hbase.HConstants.OperationStatusCode;
import org.apache.hadoop.hbase.HDFSBlocksDistribution;
import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.KeyValueUtil;
import org.apache.hadoop.hbase.MetaCellComparator;
import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.hadoop.hbase.NotServingRegionException;
Expand Down Expand Up @@ -7867,7 +7868,7 @@ private List<Cell> getInternal(Get get, boolean withCoprocessor, long nonceGroup
// See more details in HBASE-26036.
for (Cell cell : tmp) {
results.add(cell instanceof ByteBufferExtendedCell ?
((ByteBufferExtendedCell) cell).deepClone(): cell);
KeyValueUtil.copyToNewKeyValue(cell) : cell);
}
}

Expand Down
24 changes: 20 additions & 4 deletions ...e-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutateWithByteBuff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.ByteBuffAllocator;
import org.apache.hadoop.hbase.io.DeallocateRewriteByteBuffAllocator;
import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
import org.apache.hadoop.hbase.io.hfile.BlockCacheFactory;
import org.apache.hadoop.hbase.regionserver.HRegion;
import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
Expand Down Expand Up @@ -87,8 +88,20 @@ public static void tearDownAfterClass() throws Exception {
}

@Test
public void testCheckAndMutateWithByteBuff() throws Exception {
Table testTable = createTable(TableName.valueOf(name.getMethodName()));
public void testCheckAndMutateWithByteBuffNoEncode() throws Exception {
testCheckAndMutateWithByteBuff(TableName.valueOf(name.getMethodName()), DataBlockEncoding.NONE);
}

@Test
public void testCheckAndMutateWithByteBuffEncode() throws Exception {
// Tests for HBASE-26777.
// As most HBase.getRegion() calls have been factored out from HBase, you'd need to revert
// both HBASE-26777, and the HBase.get() replacements from HBASE-26036 for this test to fail
testCheckAndMutateWithByteBuff(TableName.valueOf(name.getMethodName()), DataBlockEncoding.FAST_DIFF);
}

private void testCheckAndMutateWithByteBuff(TableName tableName, DataBlockEncoding dbe) throws Exception {
Table testTable = createTable(tableName, dbe);
byte[] checkRow = Bytes.toBytes("checkRow");
byte[] checkQualifier = Bytes.toBytes("cq");
byte[] checkValue = Bytes.toBytes("checkValue");
Expand All @@ -104,10 +117,13 @@ public void testCheckAndMutateWithByteBuff() throws Exception {
Bytes.toBytes("testValue"))));
}

private Table createTable(TableName tableName)
private Table createTable(TableName tableName, DataBlockEncoding dbe)
throws IOException {
TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(CF).setBlocksize(100).build())
.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(CF)
.setBlocksize(100)
.setDataBlockEncoding(dbe)
.build())
.build();
return TEST_UTIL.createTable(td, null);
}
Expand Down