[GitHub] [hbase] Apache9 commented on a diff in pull request #4891: HBASE-27483 Expose table and region storefiles accessed days and size to the metrics

[GitBox]

Apache9 commented on code in PR #4891:
URL: https://github.com/apache/hbase/pull/4891#discussion_r1055030620


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java:
##########
@@ -313,9 +341,24 @@ public void run() {
             tempVal += store.getMemstoreOnlyRowReadsCount();
           }
           readsOnlyFromMemstore.put(store.getColumnFamilyName(), tempVal);
+
+          Map<String, Pair<Long, Long>> sfAccessTimeAndSizeMap =
+            store.getStoreFilesAccessTimeAndSize();
+          Long now = EnvironmentEdgeManager.currentTime();

Review Comment:
   Use `long` instead of `Long` as later we will do calculation?



##########
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java:
##########
@@ -56,6 +56,8 @@ public interface MetricsRegionSource extends 
Comparable<MetricsRegionSource> {
   String ROW_READS_ONLY_ON_MEMSTORE_DESC = "Row reads happening completely out 
of memstore";
   String MIXED_ROW_READS = "mixedRowReadsCount";
   String MIXED_ROW_READS_ON_STORE_DESC = "Row reads happening out of files and 
memstore on store";
+  String STOREFILES_ACCESSED_DAYS_AND_SIZE = "storeFilesAccessed%sDaysSize";

Review Comment:
   Better name it XXX_TEMPLATE.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java:
##########
@@ -1780,6 +1781,29 @@ public OptionalDouble getAvgStoreFileAge() {
     return getStoreFileAgeStream().average();
   }
 
+  @Override
+  public Map<String, Pair<Long, Long>> getStoreFilesAccessTimeAndSize() {
+    Collection<HStoreFile> storeFiles = 
this.storeEngine.getStoreFileManager().getStorefiles();
+    Map<String, Pair<Long, Long>> sfAccessTimeAndSizeMap = new HashMap<>();
+    for (HStoreFile sf : storeFiles) {
+      if (sf.getReader() == null) {
+        continue;
+      }
+      FileStatus fileStatus;
+      try {
+        fileStatus = sf.getFileInfo().getFileStatus();

Review Comment:
   If this FileStatus is cached, then the accessTime will not be updated?



-- 
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: issues-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org