wchevreuil commented on code in PR #5826:
URL: https://github.com/apache/hbase/pull/5826#discussion_r1565965514
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -935,6 +936,16 @@ void freeSpace(final String why) {
}
try {
freeInProgress = true;
+
+ // Check the list of files to determine the cold files which can be
readily evicted.
+ Set<String> coldFiles =
+ DataTieringManager.getInstance().getColdDataFiles(backingMap.keySet());
+ if (coldFiles != null) {
+ for(String fileName : coldFiles) {
+ evictBlocksByHfileName(fileName);
+ }
+ }
+
Review Comment:
We can do like this here, but then can we think if we can modify
DataTieringManager somehow to track when we have no CF with TIME_RANGE type?
That way we could avoid this extra loops through the whole set of block keys
unnecessarily.
Alternatively, we could follow the BucketEntryGroup logic starting from
#987. We could define a COLD priority group and add that as the first group in
the priority queue. That way we don't need this extra loop over the whole block
sets to find out which blocks are cold. We would leverage the already existing
loop from line #996 for that.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketProtoUtils.java:
##########
@@ -130,10 +136,30 @@ static Pair<ConcurrentHashMap<BlockCacheKey,
BucketEntry>, NavigableSet<BlockCac
ConcurrentHashMap<BlockCacheKey, BucketEntry> result = new
ConcurrentHashMap<>();
NavigableSet<BlockCacheKey> resultSet = new
ConcurrentSkipListSet<>(Comparator
.comparing(BlockCacheKey::getHfileName).thenComparingLong(BlockCacheKey::getOffset));
+
+ Map<String, Path> allFilePaths = null;
+ DataTieringManager dataTieringManager;
+ try {
+ dataTieringManager = DataTieringManager.getInstance();
+ allFilePaths = dataTieringManager.getAllFilesList();
+ } catch (IllegalStateException e) {
+ // Data-Tiering manager has not been set up.
+ // Ignore the error and proceed with the normal flow.
+ LOG.error("Error while getting DataTieringManager instance: {}",
e.getMessage());
Review Comment:
Is this to be expected often? If so, let's lower the log level to DEBUG. If
not, it seems we can still continue RS normal functioning, so it should rather
be a WARNING than an ERROR.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java:
##########
@@ -79,7 +79,7 @@ public void run() {
// so we check first if the block exists on its in-memory index,
if so, we just
// update the offset and move on to the next block without
actually going read all
// the way to the cache.
- BlockCacheKey cacheKey = new BlockCacheKey(name, offset);
+ BlockCacheKey cacheKey = new BlockCacheKey(path, offset, true,
BlockType.DATA);
Review Comment:
How do you know this is a Data block? At this point, I don't think this can
be assumed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
