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

Jump to bottom

HBASE-27232 Fix checking for encoded block size when deciding if bloc… #4640

Merged
merged 6 commits into from
Jul 25, 2022
Merged

HBASE-27232 Fix checking for encoded block size when deciding if bloc… #4640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
05ba4b4
HBASE-27232 Fix checking for encoded block size when deciding if bloc…
wchevreuil Jul 21, 2022
79fd61e
fixing comment
wchevreuil Jul 21, 2022
4bf5d49
addressing review comments & spotless checks
wchevreuil Jul 21, 2022
1b46055
fixing UT error and checkstyle
wchevreuil Jul 22, 2022
8eb91cc
spotless
wchevreuil Jul 22, 2022
3b57fe6
review changes
wchevreuil Jul 22, 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
19 changes: 12 additions & 7 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,9 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this changes the default behavior i believe. do you think this change is applicable to most users (i.e. net positive for them)? not against it, just asking... alternatively we can add handling of 0 value below and leave default. i have no opinion since i dont fully grasp the feature yet.

Copy link
Contributor Author

@wchevreuil wchevreuil Jul 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't change the default behaviour in the sense that if "hbase.writer.unified.encoded.blocksize.ratio" isn't set, we consider only the unencoded size for calculating block limit, which is the same with previous if condition on checkBlockBoundary method.

The difference is when "hbase.writer.unified.encoded.blocksize.ratio" is set, as we now can have 64KB of encoded data (whilst it would never be possible before).

this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);

finishInit(conf);
if (LOG.isTraceEnabled()) {
LOG.trace("Writer" + (path != null ? " for " + path : "") + " initialized with cacheConf: "
Expand Down Expand Up @@ -309,12 +310,16 @@ protected void finishInit(final Configuration conf) {
* At a block boundary, write all the inline blocks and opens new block.
*/
protected void checkBlockBoundary() throws IOException {
// For encoder like prefixTree, encoded size is not available, so we have to compare both
// encoded size and unencoded size to blocksize limit.
if (
blockWriter.encodedBlockSizeWritten() >= encodedBlockSizeLimit
|| blockWriter.blockSizeWritten() >= hFileContext.getBlocksize()
) {
boolean shouldFinishBlock = false;
// This means hbase.writer.unified.encoded.blocksize.ratio was set to something different from 0
// and we should use the encoding ratio
if (encodedBlockSizeLimit > 0) {
shouldFinishBlock = blockWriter.encodedBlockSizeWritten() >= encodedBlockSizeLimit;
} else {
shouldFinishBlock = blockWriter.encodedBlockSizeWritten() >= hFileContext.getBlocksize()
|| blockWriter.blockSizeWritten() >= hFileContext.getBlocksize();
}
if (shouldFinishBlock) {
finishBlock();
writeInlineBlocks(false);
newBlock();
Expand Down
52 changes: 52 additions & 0 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
import org.apache.hadoop.hbase.io.hfile.BlockCacheFactory;
import org.apache.hadoop.hbase.io.hfile.CacheConfig;
import org.apache.hadoop.hbase.io.hfile.CacheStats;
import org.apache.hadoop.hbase.io.hfile.FixedFileTrailer;
import org.apache.hadoop.hbase.io.hfile.HFile;
import org.apache.hadoop.hbase.io.hfile.HFileBlock;
import org.apache.hadoop.hbase.io.hfile.HFileContext;
import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
import org.apache.hadoop.hbase.io.hfile.HFileDataBlockEncoder;
Expand Down Expand Up @@ -1141,4 +1144,53 @@ public void testDataBlockEncodingMetaData() throws IOException {
byte[] value = fileInfo.get(HFileDataBlockEncoder.DATA_BLOCK_ENCODING);
assertArrayEquals(dataBlockEncoderAlgo.getNameInBytes(), value);
}

@Test
public void testDataBlockSizeEncoded() throws Exception {
// Make up a directory hierarchy that has a regiondir ("7e0102") and familyname.
Path dir = new Path(new Path(this.testDir, "7e0102"), "familyname");
Path path = new Path(dir, "1234567890");

DataBlockEncoding dataBlockEncoderAlgo = DataBlockEncoding.FAST_DIFF;

conf.setDouble("hbase.writer.unified.encoded.blocksize.ratio", 1);

cacheConf = new CacheConfig(conf);
HFileContext meta =
new HFileContextBuilder().withBlockSize(BLOCKSIZE_SMALL).withChecksumType(CKTYPE)
.withBytesPerCheckSum(CKBYTES).withDataBlockEncoding(dataBlockEncoderAlgo).build();
// Make a store file and write data to it.
StoreFileWriter writer = new StoreFileWriter.Builder(conf, cacheConf, this.fs)
.withFilePath(path).withMaxKeyCount(2000).withFileContext(meta).build();
writeStoreFile(writer);

HStoreFile storeFile =
new HStoreFile(fs, writer.getPath(), conf, cacheConf, BloomType.NONE, true);
storeFile.initReader();
StoreFileReader reader = storeFile.getReader();

Map<byte[], byte[]> fileInfo = reader.loadFileInfo();
byte[] value = fileInfo.get(HFileDataBlockEncoder.DATA_BLOCK_ENCODING);
assertEquals(dataBlockEncoderAlgo.name(), Bytes.toString(value));

HFile.Reader fReader =
HFile.createReader(fs, writer.getPath(), storeFile.getCacheConf(), true, conf);

FSDataInputStreamWrapper fsdis = new FSDataInputStreamWrapper(fs, writer.getPath());
long fileSize = fs.getFileStatus(writer.getPath()).getLen();
FixedFileTrailer trailer = FixedFileTrailer.readFromStream(fsdis.getStream(false), fileSize);
long offset = trailer.getFirstDataBlockOffset(), max = trailer.getLastDataBlockOffset();
HFileBlock block;
while (offset <= max) {
block = fReader.readBlock(offset, -1, /* cacheBlock */
false, /* pread */ false, /* isCompaction */ false, /* updateCacheMetrics */
false, null, null);
offset += block.getOnDiskSizeWithHeader();
double diff = block.getOnDiskSizeWithHeader() - BLOCKSIZE_SMALL;
if (offset <= max) {
assertTrue(diff >= 0 && diff < (BLOCKSIZE_SMALL * 0.05));
}
}
}

}