Uh oh!
There was an error while loading. Please reload this page.
HBASE-28466 Integration of time-based priority logic of bucket cache in prefetch functionality of HBase - #5808
Merged
Conversation
Apache-HBase
commented
Apr 9, 2024
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 9, 2024
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 9, 2024
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 10, 2024
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 10, 2024
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 10, 2024
💔 -1 overall
This message was automatically generated. |
wchevreuil
requested changes
Apr 12, 2024
Comment on lines
+42
to
+49
| try { | ||
| DataTieringManager dataTieringManager = DataTieringManager.getInstance(); | ||
| if (!dataTieringManager.isHotData(this)) { | ||
| LOG.debug("Data tiering is enabled for path '{}' and it is not hot data", path); | ||
| return; | ||
| } | ||
| } catch (IllegalStateException e) { | ||
| LOG.error("Error while getting DataTieringManager instance: {}", e.getMessage()); |
Contributor
There was a problem hiding this comment.
Please move this to BucketCache.shouldCacheFile implementation. Any caching related logic should be in the Cache related classes. Reader should only defer the decisions about caching to the Cache implementations.
ContributorAuthor
There was a problem hiding this comment.
Got it. How should I proceed? The DataTieringManager#isHotData() method requires HFilePreadReader. Should I update the existing shouldCacheFile method to pass HFilePreadReader like this?
@Override
public Optional<Boolean> shouldCacheFile(HFilePreadReader reader) {
try {
DataTieringManager dataTieringManager = DataTieringManager.getInstance();
if (!dataTieringManager.isHotData(reader)) {
LOG.debug("Data tiering is enabled for path '{}' and it is not hot data", reader.getPath());
return Optional.of(false);
}
} catch (IllegalStateException e) {
LOG.error("Error while getting DataTieringManager instance: {}", e.getMessage());
}
// if we don't have the file in fullyCachedFiles, we should cache it
String fileName = reader.getPath().getName();
return Optional.of(!fullyCachedFiles.containsKey(fileName));
}
ContributorAuthor
There was a problem hiding this comment.
Another option is something like below.
public HFilePreadReader(ReaderContext context, HFileInfo fileInfo, CacheConfig cacheConf,
Configuration conf) throws IOException {
super(context, fileInfo, cacheConf, conf);
boolean isHotData = true;
try {
DataTieringManager dataTieringManager = DataTieringManager.getInstance();
isHotData = dataTieringManager.isHotData(this);
} catch (IllegalStateException e) {
LOG.error("Error while getting DataTieringManager instance: {}", e.getMessage());
}
final MutableBoolean shouldCache = new MutableBoolean(true);
boolean finalIsHotData = isHotData;
cacheConf.getBlockCache().ifPresent(cache -> {
Optional<Boolean> result = cache.shouldCacheFile(path.getName(), finalIsHotData);
shouldCache.setValue(result.isPresent() ? result.get().booleanValue() : true);
});
Contributor
There was a problem hiding this comment.
It seems you are adding a reader dependency on DataTieringManager mainly to create an HFileInfo instance and infer the max/min timestamps. Can't you just pass the fileInfo attr to the shouldCacheFile method?
ContributorAuthor
There was a problem hiding this comment.
Yeah, this looks correct. but we also need Configuration which is not present in HFileInfo.
Apache-HBase
commented
Apr 15, 2024
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 15, 2024
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 15, 2024
💔 -1 overall
This message was automatically generated. |
Comment on lines
+44
to
+45
| HFileInfo hFileInfoForCaching = new HFileInfo(context, conf); | ||
| hFileInfoForCaching.initMetaAndIndex(this); |
Contributor
There was a problem hiding this comment.
Why are you creating a new HFileInfo here? Can you just use the super.fileInfo reference?
ContributorAuthor
There was a problem hiding this comment.
If I have to pass super.fileInfo, then we need to call initMetaAndIndex() on that. So, I thought that might disrupt the existing logic. Also, if I modify the HFileInfo state (the one we passed as an argument), it might come as a side effect, right?
Contributor
There was a problem hiding this comment.
AFAICT, we would just be calling it again in HStoreFile.open, right after we return from reader creation. I don't like duplicate object or code run unnecessarily, so maybe we could modify initMetaAndIndex to do its stuff just if the file info hasn't been initialized yet.
ContributorAuthor
There was a problem hiding this comment.
maybe we could modify initMetaAndIndex to do its stuff just if the file info hasn't been initialized yet.
Hmmm, I'm not sure how we can do that. Maybe use a boolean field in the HFileInfo to track whether initMetaAndIndex is done or not? Because initMetaAndIndex does a couple of things, and it's hard to say whether that is initialized or not. Can you think of any way we can achieve that?
Contributor
There was a problem hiding this comment.
Maybe use a boolean field in the HFileInfo to track whether initMetaAndIndex is done or not?
Yeah, I don't think there's another way.
Apache-HBase
commented
Apr 19, 2024
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 19, 2024
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 19, 2024
💔 -1 overall
This message was automatically generated. |
wchevreuil
approved these changes
Apr 22, 2024
wchevreuil pushed a commit
that referenced
this pull request
Apr 22, 2024
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Apr 25, 2024
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
May 20, 2024
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
vinayakphegde added a commit
to vinayakphegde/hbase
that referenced
this pull request
May 21, 2024
…in prefetch functionality of HBase (apache#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
May 21, 2024
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
jhungund pushed a commit
to janardhanrh/hbase
that referenced
this pull request
May 29, 2024
…in prefetch functionality of HBase (apache#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I2443acef22393731454af9458785c617acc08af6
wchevreuil pushed a commit
that referenced
this pull request
Jun 24, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Jul 10, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Jul 11, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Aug 5, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Sep 1, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
to wchevreuil/hbase
that referenced
this pull request
Sep 1, 2025
…in prefetch functionality of HBase (apache#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I374a9ab88807da3188962d7ac0b2b727eaffd010
wchevreuil pushed a commit
that referenced
this pull request
Sep 1, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
wchevreuil pushed a commit
that referenced
this pull request
Sep 3, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I374a9ab88807da3188962d7ac0b2b727eaffd010
wchevreuil pushed a commit
to wchevreuil/hbase
that referenced
this pull request
Sep 3, 2025
…in prefetch functionality of HBase (apache#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I374a9ab88807da3188962d7ac0b2b727eaffd010
wchevreuil pushed a commit
that referenced
this pull request
Sep 3, 2025
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I374a9ab88807da3188962d7ac0b2b727eaffd010
stoty pushed a commit
to stoty/hbase
that referenced
this pull request
Nov 22, 2025
…into cdh_main HBASE-28466 Integration of time-based priority logic of bucket cache in prefetch functionality of HBase (apache#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Change-Id: I374a9ab88807da3188962d7ac0b2b727eaffd010
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.