Skip to content
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

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

Merged
merged 2 commits into from
Aug 15, 2023
Merged

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7df733
HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker
jbewing Aug 9, 2023
458e99f
Enforce that right Cell is always on-heap
jbewing Aug 14, 2023
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
157 changes: 134 additions & 23 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,100 @@ public ByteBuffer decodeKeyValues(DataInputStream source,
// Having this as static is fine but if META is having DBE then we should
// change this.
public static int compareCommonRowPrefix(Cell left, Cell right, int rowCommonPrefix) {
return Bytes.compareTo(left.getRowArray(), left.getRowOffset() + rowCommonPrefix,
left.getRowLength() - rowCommonPrefix, right.getRowArray(),
right.getRowOffset() + rowCommonPrefix, right.getRowLength() - rowCommonPrefix);
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(bbLeft.getRowByteBuffer(),
bbLeft.getRowPosition() + rowCommonPrefix, left.getRowLength() - rowCommonPrefix,
bbRight.getRowByteBuffer(), bbRight.getRowPosition() + rowCommonPrefix,
right.getRowLength() - rowCommonPrefix);
} else {
return ByteBufferUtils.compareTo(bbLeft.getRowByteBuffer(),
bbLeft.getRowPosition() + rowCommonPrefix, left.getRowLength() - rowCommonPrefix,
right.getRowArray(), right.getRowOffset() + rowCommonPrefix,
right.getRowLength() - rowCommonPrefix);
}
} else {
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(left.getRowArray(), left.getRowOffset() + rowCommonPrefix,
left.getRowLength() - rowCommonPrefix, bbRight.getRowByteBuffer(),
bbRight.getRowPosition() + rowCommonPrefix, right.getRowLength() - rowCommonPrefix);
} else {
return Bytes.compareTo(left.getRowArray(), left.getRowOffset() + rowCommonPrefix,
left.getRowLength() - rowCommonPrefix, right.getRowArray(),
right.getRowOffset() + rowCommonPrefix, right.getRowLength() - rowCommonPrefix);
}
}
}

public static int compareCommonFamilyPrefix(Cell left, Cell right, int familyCommonPrefix) {
return Bytes.compareTo(left.getFamilyArray(), left.getFamilyOffset() + familyCommonPrefix,
left.getFamilyLength() - familyCommonPrefix, right.getFamilyArray(),
right.getFamilyOffset() + familyCommonPrefix, right.getFamilyLength() - familyCommonPrefix);
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(bbLeft.getFamilyByteBuffer(),
bbLeft.getFamilyPosition() + familyCommonPrefix,
left.getFamilyLength() - familyCommonPrefix, bbRight.getFamilyByteBuffer(),
bbRight.getFamilyPosition() + familyCommonPrefix,
right.getFamilyLength() - familyCommonPrefix);
} else {
return ByteBufferUtils.compareTo(bbLeft.getFamilyByteBuffer(),
bbLeft.getFamilyPosition() + familyCommonPrefix,
left.getFamilyLength() - familyCommonPrefix, right.getFamilyArray(),
right.getFamilyOffset() + familyCommonPrefix,
right.getFamilyLength() - familyCommonPrefix);
}
} else {
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(left.getFamilyArray(),
left.getFamilyOffset() + familyCommonPrefix, left.getFamilyLength() - familyCommonPrefix,
bbRight.getFamilyByteBuffer(), bbRight.getFamilyPosition() + familyCommonPrefix,
right.getFamilyLength() - familyCommonPrefix);
} else {
return Bytes.compareTo(left.getFamilyArray(), left.getFamilyOffset() + familyCommonPrefix,
left.getFamilyLength() - familyCommonPrefix, right.getFamilyArray(),
right.getFamilyOffset() + familyCommonPrefix,
right.getFamilyLength() - familyCommonPrefix);
}
}
}

public static int compareCommonQualifierPrefix(Cell left, Cell right, int qualCommonPrefix) {
return Bytes.compareTo(left.getQualifierArray(), left.getQualifierOffset() + qualCommonPrefix,
left.getQualifierLength() - qualCommonPrefix, right.getQualifierArray(),
right.getQualifierOffset() + qualCommonPrefix, right.getQualifierLength() - qualCommonPrefix);
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(bbLeft.getQualifierByteBuffer(),
bbLeft.getQualifierPosition() + qualCommonPrefix,
left.getQualifierLength() - qualCommonPrefix, bbRight.getQualifierByteBuffer(),
bbRight.getQualifierPosition() + qualCommonPrefix,
right.getQualifierLength() - qualCommonPrefix);
} else {
return ByteBufferUtils.compareTo(bbLeft.getQualifierByteBuffer(),
bbLeft.getQualifierPosition() + qualCommonPrefix,
left.getQualifierLength() - qualCommonPrefix, right.getQualifierArray(),
right.getQualifierOffset() + qualCommonPrefix,
right.getQualifierLength() - qualCommonPrefix);
}
} else {
if (right instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
return ByteBufferUtils.compareTo(left.getQualifierArray(),
left.getQualifierOffset() + qualCommonPrefix,
left.getQualifierLength() - qualCommonPrefix, bbRight.getQualifierByteBuffer(),
bbRight.getQualifierPosition() + qualCommonPrefix,
right.getQualifierLength() - qualCommonPrefix);
} else {
return Bytes.compareTo(left.getQualifierArray(),
left.getQualifierOffset() + qualCommonPrefix,
left.getQualifierLength() - qualCommonPrefix, right.getQualifierArray(),
right.getQualifierOffset() + qualCommonPrefix,
right.getQualifierLength() - qualCommonPrefix);
}
}
}

protected static class SeekerState {
Expand Down Expand Up @@ -954,25 +1033,57 @@ private int compareTypeBytes(Cell key, Cell right) {
return 0;
}

private static int findCommonPrefixInRowPart(Cell left, Cell right, int rowCommonPrefix) {
return Bytes.findCommonPrefix(left.getRowArray(), right.getRowArray(),
left.getRowLength() - rowCommonPrefix, right.getRowLength() - rowCommonPrefix,
left.getRowOffset() + rowCommonPrefix, right.getRowOffset() + rowCommonPrefix);
// These findCommonPrefix* methods rely on the fact that keyOnlyKv is the "right" cell argument
Apache9 marked this conversation as resolved.
Show resolved Hide resolved
// and always on-heap

private static int findCommonPrefixInRowPart(Cell left, KeyValue.KeyOnlyKeyValue right,
int rowCommonPrefix) {
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
return ByteBufferUtils.findCommonPrefix(bbLeft.getRowByteBuffer(),
bbLeft.getRowPosition() + rowCommonPrefix, left.getRowLength() - rowCommonPrefix,
right.getRowArray(), right.getRowOffset() + rowCommonPrefix,
right.getRowLength() - rowCommonPrefix);
} else {
return Bytes.findCommonPrefix(left.getRowArray(), right.getRowArray(),
left.getRowLength() - rowCommonPrefix, right.getRowLength() - rowCommonPrefix,
left.getRowOffset() + rowCommonPrefix, right.getRowOffset() + rowCommonPrefix);
}
}

private static int findCommonPrefixInFamilyPart(Cell left, Cell right, int familyCommonPrefix) {
return Bytes.findCommonPrefix(left.getFamilyArray(), right.getFamilyArray(),
left.getFamilyLength() - familyCommonPrefix, right.getFamilyLength() - familyCommonPrefix,
left.getFamilyOffset() + familyCommonPrefix, right.getFamilyOffset() + familyCommonPrefix);
private static int findCommonPrefixInFamilyPart(Cell left, KeyValue.KeyOnlyKeyValue right,
int familyCommonPrefix) {
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
return ByteBufferUtils.findCommonPrefix(bbLeft.getFamilyByteBuffer(),
bbLeft.getFamilyPosition() + familyCommonPrefix,
left.getFamilyLength() - familyCommonPrefix, right.getFamilyArray(),
right.getFamilyOffset() + familyCommonPrefix,
right.getFamilyLength() - familyCommonPrefix);
} else {
return Bytes.findCommonPrefix(left.getFamilyArray(), right.getFamilyArray(),
left.getFamilyLength() - familyCommonPrefix, right.getFamilyLength() - familyCommonPrefix,
left.getFamilyOffset() + familyCommonPrefix,
right.getFamilyOffset() + familyCommonPrefix);
}
}

private static int findCommonPrefixInQualifierPart(Cell left, Cell right,
private static int findCommonPrefixInQualifierPart(Cell left, KeyValue.KeyOnlyKeyValue right,
int qualifierCommonPrefix) {
return Bytes.findCommonPrefix(left.getQualifierArray(), right.getQualifierArray(),
left.getQualifierLength() - qualifierCommonPrefix,
right.getQualifierLength() - qualifierCommonPrefix,
left.getQualifierOffset() + qualifierCommonPrefix,
right.getQualifierOffset() + qualifierCommonPrefix);
if (left instanceof ByteBufferExtendedCell) {
ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
return ByteBufferUtils.findCommonPrefix(bbLeft.getQualifierByteBuffer(),
bbLeft.getQualifierPosition() + qualifierCommonPrefix,
left.getQualifierLength() - qualifierCommonPrefix, right.getQualifierArray(),
right.getQualifierOffset() + qualifierCommonPrefix,
right.getQualifierLength() - qualifierCommonPrefix);
} else {
return Bytes.findCommonPrefix(left.getQualifierArray(), right.getQualifierArray(),
left.getQualifierLength() - qualifierCommonPrefix,
right.getQualifierLength() - qualifierCommonPrefix,
left.getQualifierOffset() + qualifierCommonPrefix,
right.getQualifierOffset() + qualifierCommonPrefix);
}
}

private void moveToPrevious() {
Expand Down
24 changes: 24 additions & 0 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,30 @@ public static int findCommonPrefix(ByteBuffer left, int leftOffset, int leftLeng
return result;
}

/**
* Find length of common prefix in two arrays.
* @param left ByteBuffer to be compared.
* @param leftOffset Offset in left ByteBuffer.
* @param leftLength Length of left ByteBuffer.
* @param right Array to be compared
* @param rightOffset Offset in right Array.
* @param rightLength Length of right Array.
*/
public static int findCommonPrefix(ByteBuffer left, int leftOffset, int leftLength, byte[] right,
int rightOffset, int rightLength) {
int length = Math.min(leftLength, rightLength);
int result = 0;

while (
jbewing marked this conversation as resolved.
Show resolved Hide resolved
result < length
&& ByteBufferUtils.toByte(left, leftOffset + result) == right[rightOffset + result]
) {
result++;
}

return result;
}

/**
* Check whether two parts in the same buffer are equal.
* @param buffer In which buffer there are parts
Expand Down
65 changes: 65 additions & 0 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestDataBlockEncoders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.concurrent.ThreadLocalRandom;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ArrayBackedTag;
import org.apache.hadoop.hbase.ByteBufferKeyValue;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellComparatorImpl;
import org.apache.hadoop.hbase.CellUtil;
Expand Down Expand Up @@ -230,6 +231,59 @@ public void testSeekingOnSample() throws IOException {
LOG.info("Done");
}

@Test
public void testSeekingToOffHeapKeyValueInSample() throws IOException {
List<KeyValue> sampleKv = generator.generateTestKeyValues(NUMBER_OF_KV, includesTags);

// create all seekers
List<DataBlockEncoder.EncodedSeeker> encodedSeekers = new ArrayList<>();
for (DataBlockEncoding encoding : DataBlockEncoding.values()) {
LOG.info("Encoding: " + encoding);
DataBlockEncoder encoder = encoding.getEncoder();
if (encoder == null) {
continue;
}
LOG.info("Encoder: " + encoder);
ByteBuffer encodedBuffer = encodeKeyValues(encoding, sampleKv,
getEncodingContext(conf, Compression.Algorithm.NONE, encoding), this.useOffheapData);
HFileContext meta =
new HFileContextBuilder().withHBaseCheckSum(false).withIncludesMvcc(includesMemstoreTS)
.withIncludesTags(includesTags).withCompression(Compression.Algorithm.NONE).build();
DataBlockEncoder.EncodedSeeker seeker =
encoder.createSeeker(encoder.newDataBlockDecodingContext(conf, meta));
seeker.setCurrentBuffer(new SingleByteBuff(encodedBuffer));
encodedSeekers.add(seeker);
}
LOG.info("Testing it!");
// test it!
// try a few random seeks
Random rand = ThreadLocalRandom.current();
for (boolean seekBefore : new boolean[] { false, true }) {
for (int i = 0; i < NUM_RANDOM_SEEKS; ++i) {
int keyValueId;
if (!seekBefore) {
keyValueId = rand.nextInt(sampleKv.size());
} else {
keyValueId = rand.nextInt(sampleKv.size() - 1) + 1;
}

KeyValue keyValue = sampleKv.get(keyValueId);
checkSeekingConsistency(encodedSeekers, seekBefore, buildOffHeapKeyValue(keyValue));
}
}

// check edge cases
LOG.info("Checking edge cases");
checkSeekingConsistency(encodedSeekers, false, sampleKv.get(0));
for (boolean seekBefore : new boolean[] { false, true }) {
checkSeekingConsistency(encodedSeekers, seekBefore, sampleKv.get(sampleKv.size() - 1));
KeyValue midKv = sampleKv.get(sampleKv.size() / 2);
Cell lastMidKv = PrivateCellUtil.createLastOnRowCol(midKv);
checkSeekingConsistency(encodedSeekers, seekBefore, lastMidKv);
}
LOG.info("Done");
}

static ByteBuffer encodeKeyValues(DataBlockEncoding encoding, List<KeyValue> kvs,
HFileBlockEncodingContext encodingContext, boolean useOffheapData) throws IOException {
DataBlockEncoder encoder = encoding.getEncoder();
Expand Down Expand Up @@ -438,4 +492,15 @@ private void testAlgorithm(byte[] encodedData, ByteBuffer unencodedDataBuf,
assertEquals("Encoding -> decoding gives different results for " + encoder,
Bytes.toStringBinary(unencodedDataBuf), Bytes.toStringBinary(actualDataset));
}

private static ByteBufferKeyValue buildOffHeapKeyValue(KeyValue keyValue) throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
keyValue.write(out, false);
byte[] bytes = out.toByteArray();
ByteBuffer bb = ByteBuffer.allocateDirect(bytes.length);
bb.put(bytes);
bb.flip();

return new ByteBufferKeyValue(bb, 0, bytes.length);
}
}