[GitHub] [hbase] anoopsjohn commented on a change in pull request #1552: HBASE-24205 Create metric to know the number of reads that happens fr…

[GitBox <gi...@apache.org>]

anoopsjohn commented on a change in pull request #1552:
URL: https://github.com/apache/hbase/pull/1552#discussion_r438696429



##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableSourceImpl.java
##########
@@ -311,6 +322,27 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
         mrb.addGauge(Interns.info(tableNamePrefix + MetricsRegionServerSource.NUM_REFERENCE_FILES,
             MetricsRegionServerSource.NUM_REFERENCE_FILES_DESC),
             tableWrapperAgg.getNumReferenceFiles(tableName.getNameAsString()));
+        addGauge(mrb, tableWrapperAgg.getMemstoreReadRequestsCount(tableName.getNameAsString()),
+          MetricsRegionSource.READ_REQUEST_ON_MEMSTORE,
+          MetricsRegionSource.READ_REQUEST_ON_MEMSTORE_DESC);
+        addGauge(mrb, tableWrapperAgg.getMixedRequestsCount(tableName.getNameAsString()),
+          MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE,
+          MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE_DESC);
+      }
+    }
+  }
+
+  private void addGauge(MetricsRecordBuilder mrb, Map<String, Long> metricMap, String metricName,
+      String metricDesc) {
+    if (metricMap != null) {
+      Iterator<Entry<String, Long>> iterator = metricMap.entrySet().iterator();
+      while (iterator.hasNext()) {
+        Entry<String, Long> entry = iterator.next();
+        // append 'store' and its name to the metric
+        mrb.addGauge(Interns.info(this.tableNamePrefixPart1 + _STORE
+            + entry.getKey().split(MetricsTableWrapperAggregate.UNDERSCORE)[1]

Review comment:
       Here the key will be <regionName>_<storeName>?

##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregate.java
##########
@@ -107,6 +109,13 @@
    */
   long getNumReferenceFiles(String table);
 
+  /**
+   * @return number of get requests from memstore per store for this table
+   */
+  Map<String, Long> getMemstoreReadRequestsCount(String table);
 
-
+  /**
+   * @return number of get requests from file per store for this table
+   */
+  Map<String, Long> getMixedRequestsCount(String table);

Review comment:
       We dont say whether it is read reqs in this method name.  This is the total  #rows reads from this Store right?  Can we name in that way and avoid the term mixed?




----------------------------------------------------------------
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.

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