wchevreuil commented on code in PR #5492:
URL: https://github.com/apache/hbase/pull/5492#discussion_r1394160396
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java:
##########
@@ -2651,4 +2651,10 @@ List<LogEntry> getLogEntries(Set<ServerName>
serverNames, String logType, Server
* Get the list of cached files
*/
List<String> getCachedFilesList(ServerName serverName) throws IOException;
+
+ /**
+ * Clean Cache by evicting the blocks of files belonging to regions that are
no longer served by
+ * the RegionServer.
+ */
Review Comment:
Please explain in javadocs what's being returned.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java:
##########
@@ -435,6 +437,16 @@ public Optional<Map<String, Boolean>>
getFullyCachedFiles() {
return this.l2Cache.getFullyCachedFiles();
}
+ @Override
+ public Optional<List<HStoreFile>> getFilesWithStaleBlocks() {
+ return l2Cache.getFilesWithStaleBlocks();
+ }
+
+ @Override
+ public Optional<Map<String, Integer>> uncacheStaleBlocks() {
+ return l2Cache.uncacheStaleBlocks();
Review Comment:
We should do this to L1 as well.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -1985,4 +1991,29 @@ public static Optional<BucketCache>
getBucketCacheFromCacheConfig(CacheConfig ca
return Optional.empty();
}
+ @Override
+ public Optional<List<HStoreFile>> getFilesWithStaleBlocks() {
+ return Optional.of(filesWithStaleBlocks);
+ }
+
+ public static void clearFilesWithStaleBlocks() {
+ filesWithStaleBlocks.clear();
+ }
+
+ public void setFilesWithStaleBlocks(HRegion hRegion) {
+ for (HStore hs : hRegion.getStores()) {
+ filesWithStaleBlocks.addAll(hs.getStorefiles());
Review Comment:
Shouldn't we check first which files from the region's store have blocks in
the cache? If the total cache size is smaller than the store file size, we
might not be able to cache all files.
Also, I wonder what's the heap usage impact of keeping such extra list.
Could you check that with a heap dump?
--
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]
