-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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.
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-27370 Avoid decompressing blocks when reading from bucket cache… #4781
Conversation
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
… prefetch threads Change-Id: Ie18665a3f21fe2797f6272085cef54d8936097a6 fixing spotless issues Change-Id: Ief22be71cd5bc067fd319ee387e3d6eb237dbb9e checkstyles Change-Id: I75fea74c19dc6170fb040db6b910434c5f1f420a
2bf5e9b
to
40abb7c
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Spotbugs issue shouldn't be related. See ongoing PR for fixing sputbugs: #4787 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one minor question on the cacheOnly parameter.
.withBlockSize(DATA_BLOCK_SIZE).build(); | ||
Path storeFile = writeStoreFile("TestPrefetchCompressed", context); | ||
readStoreFileCacheOnly(storeFile); | ||
conf.setBoolean(CACHE_DATA_BLOCKS_COMPRESSED_KEY, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] is it for resetting configuration to avoid error for other tests? should we use a cleanup? but it's not a blocker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, don't want to interfere on other tests.
@Override | ||
public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize, final boolean cacheBlock, | ||
boolean pread, final boolean isCompaction, boolean updateCacheMetrics, | ||
BlockType expectedBlockType, DataBlockEncoding expectedDataBlockEncoding, boolean cacheOnly) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] this new flag cacheOnly=true
should just skip reading blocks from the local cache, and it does not have any caller other in the unit tests, are you planning to introduce a new behavior in the future ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We call this on line #60 of HFilePreadReader. That line is executed once we set CACHE_DATA_BLOCKS_COMPRESSED_KEY to true, which is how we are testing it further down on the UT.
#4781) Co-authored-by: Josh Elser <[email protected]> Signed-off-by: Peter Somogyi <[email protected]> Signed-off-by: Tak Lon (Stephen) Wu <[email protected]>
#4781) Co-authored-by: Josh Elser <[email protected]> Signed-off-by: Peter Somogyi <[email protected]> Signed-off-by: Tak Lon (Stephen) Wu <[email protected]>
#4781) Co-authored-by: Josh Elser <[email protected]> Signed-off-by: Peter Somogyi <[email protected]> Signed-off-by: Tak Lon (Stephen) Wu <[email protected]>
apache#4781) Co-authored-by: Josh Elser <[email protected]> Signed-off-by: Peter Somogyi <[email protected]> Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> (cherry picked from commit e25b2a7) Change-Id: I3d713c54f2d715486509e7b26d3db8d4af10e1ed
… prefetch threads