Addresses review comments-metric name updated everywhere

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java

@@ -53,8 +53,8 @@ public interface MetricsRegionSource extends Comparable<MetricsRegionSource> {
   String COPROCESSOR_EXECUTION_STATISTICS_DESC = "Statistics for coprocessor execution times";
   String REPLICA_ID = "replicaid";
   String REPLICA_ID_DESC = "The replica ID of a region. 0 is primary, otherwise is secondary";
-  String READ_REQUEST_ON_MEMSTORE = "readRequestCountOnMemstore";
-  String READ_REQUEST_ON_MEMSTORE_DESC = "Reads happening out of memstore";
+  String READ_REQUEST_ONLY_ON_MEMSTORE = "readRequestCountOnlyOnMemstore";
+  String READ_REQUEST_ONLY_ON_MEMSTORE_DESC = "Reads happening only out of memstore";
   String MIXED_READ_REQUEST_ON_STORE = "mixedReadRequestCountOnStore";
   String MIXED_READ_REQUEST_ON_STORE_DESC = "Reads happening out of files and memstore on store";
 

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java

@@ -310,10 +310,10 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
               regionNamePrefix + MetricsRegionSource.MAX_FLUSH_QUEUE_SIZE,
               MetricsRegionSource.MAX_FLUSH_QUEUE_DESC),
           this.regionWrapper.getMaxFlushQueueSize());
-      addCounter(mrb, this.regionWrapper.getMemstoreReadRequestsCount(),
-        MetricsRegionSource.READ_REQUEST_ON_MEMSTORE,
-        MetricsRegionSource.READ_REQUEST_ON_MEMSTORE_DESC);
-      addCounter(mrb, this.regionWrapper.getMixedReadRequestCount(),
+      addCounter(mrb, this.regionWrapper.getMemstoreOnlyReadRequestsCount(),
+        MetricsRegionSource.READ_REQUEST_ONLY_ON_MEMSTORE,
+        MetricsRegionSource.READ_REQUEST_ONLY_ON_MEMSTORE_DESC);
+      addCounter(mrb, this.regionWrapper.getMixedReadRequestsCount(),
         MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE,
         MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE_DESC);
     }
@@ -322,9 +322,7 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
   private void addCounter(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();
+      for (Entry<String, Long> entry : metricMap.entrySet()) {
         // append 'store' and its name to the metric
         mrb.addCounter(Interns.info(
           this.regionNamePrefix1 + _STORE + entry.getKey() + this.regionNamePrefix2 + metricName,

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java

@@ -174,13 +174,13 @@ public interface MetricsRegionWrapper {
   long getMaxCompactedStoreFileRefCount();
 
   /**
-   * @return the number of reads on memstore per store
+   * @return the number of reads only on memstore per store
    */
-  Map<String, Long> getMemstoreReadRequestsCount();
+  Map<String, Long> getMemstoreOnlyReadRequestsCount();
 
   /**
-   * @return the mixed reads happening per store
+   * @return the happening on memstore and file per store
    */
-  Map<String, Long> getMixedReadRequestCount();
+  Map<String, Long> getMixedReadRequestsCount();
 
 }

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableSourceImpl.java

@@ -322,10 +322,10 @@ 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()),
+        addGauge(mrb, tableWrapperAgg.getMemstoreOnlyReadRequestsCount(tableName.getNameAsString()),
+          MetricsRegionSource.READ_REQUEST_ONLY_ON_MEMSTORE,
+          MetricsRegionSource.READ_REQUEST_ONLY_ON_MEMSTORE_DESC);
+        addGauge(mrb, tableWrapperAgg.getMixedReadRequestsCount(tableName.getNameAsString()),
           MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE,
           MetricsRegionSource.MIXED_READ_REQUEST_ON_STORE_DESC);
       }
@@ -335,9 +335,7 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
   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();
+      for (Entry<String, Long> entry : metricMap.entrySet()) {
         // append 'store' and its name to the metric
         mrb.addGauge(Interns.info(this.tableNamePrefixPart1 + _STORE
             + entry.getKey().split(MetricsTableWrapperAggregate.UNDERSCORE)[1]

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregate.java

@@ -110,12 +110,12 @@ public interface MetricsTableWrapperAggregate {
   long getNumReferenceFiles(String table);
 
   /**
-   * @return number of get requests from memstore per store for this table
+   * @return number of read requests only from memstore per store for this table
    */
-  Map<String, Long> getMemstoreReadRequestsCount(String table);
+  Map<String, Long> getMemstoreOnlyReadRequestsCount(String table);
 
   /**
-   * @return number of get requests from file per store for this table
+   * @return number of read requests from file and memstore per store for this table
    */
-  Map<String, Long> getMixedRequestsCount(String table);
+  Map<String, Long> getMixedReadRequestsCount(String table);
 }

hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperStub.java

@@ -114,14 +114,14 @@ public long getCpRequestsCount(String table) {
   }
 
   @Override
-  public Map<String, Long> getMemstoreReadRequestsCount(String table) {
+  public Map<String, Long> getMemstoreOnlyReadRequestsCount(String table) {
     Map<String, Long> map = new HashMap<String, Long>();
     map.put("table_info", 3L);
     return map;
   }
 
   @Override
-  public Map<String, Long> getMixedRequestsCount(String table) {
+  public Map<String, Long> getMixedReadRequestsCount(String table) {
     Map<String, Long> map = new HashMap<String, Long>();
     map.put("table_info", 3L);
     return map;

hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java

@@ -221,14 +221,14 @@ public long getTotalRequestCount() {
     }
 
     @Override
-    public Map<String, Long> getMemstoreReadRequestsCount() {
+    public Map<String, Long> getMemstoreOnlyReadRequestsCount() {
       Map<String, Long> map = new HashMap<String, Long>();
       map.put("info", 0L);
       return map;
     }
 
     @Override
-    public Map<String, Long> getMixedReadRequestCount() {
+    public Map<String, Long> getMixedReadRequestsCount() {
       Map<String, Long> map = new HashMap<String, Long>();
       map.put("info", 0L);
       return map;

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java

@@ -164,7 +164,8 @@ public class HStore implements Store, HeapSize, StoreConfigInformation,
   volatile boolean forceMajor = false;
   private AtomicLong storeSize = new AtomicLong();
   private AtomicLong totalUncompressedBytes = new AtomicLong();
-  private LongAdder readRequestsFromMemstore = new LongAdder();
+  private LongAdder memstoreOnlyReadRequests = new LongAdder();
+  // rows that has cells from both memstore and files (or only files)
   private LongAdder mixedReadRequests = new LongAdder();
 
   private boolean cacheOnWriteLogged;
@@ -2902,8 +2903,8 @@ public int getMaxCompactedStoreFileRefCount() {
   }
 
   @Override
-  public long getReadRequestsCountFromMemstore() {
-    return readRequestsFromMemstore.sum();
+  public long getMemstoreOnlyReadsCount() {
+    return memstoreOnlyReadRequests.sum();
   }
 
   @Override
@@ -2913,7 +2914,7 @@ public long getMixedReadRequestsCount() {
 
   void updateMetricsStore(boolean memstoreRead) {
     if (memstoreRead) {
-      readRequestsFromMemstore.increment();
+      memstoreOnlyReadRequests.increment();
     } else {
       mixedReadRequests.increment();
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java

@@ -112,7 +112,7 @@ public Cell peek() {
     return this.current.peek();
   }
 
-  boolean isCellFromMemstoreScanner() {
+  boolean isLatestCellFromMemstore() {
     return !this.current.isFileScanner();
   }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java

@@ -59,7 +59,7 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable
   private long numReferenceFiles;
   private long maxFlushQueueSize;
   private long maxCompactionQueueSize;
-  private Map<String, Long> readsFromMemstore;
+  private Map<String, Long> readsOnlyFromMemstore;
   private Map<String, Long> mixedReadsOnStore;
 
   private ScheduledFuture<?> regionMetricsUpdateTask;
@@ -237,12 +237,12 @@ public int getRegionHashCode() {
   }
 
   @Override
-  public Map<String, Long> getMemstoreReadRequestsCount() {
-    return readsFromMemstore;
+  public Map<String, Long> getMemstoreOnlyReadRequestsCount() {
+    return readsOnlyFromMemstore;
   }
 
   @Override
-  public Map<String, Long> getMixedReadRequestCount() {
+  public Map<String, Long> getMixedReadRequestsCount() {
     return mixedReadsOnStore;
   }
 
@@ -302,16 +302,16 @@ public void run() {
             tempVal += store.getMixedReadRequestsCount();
           }
           mixedReadsOnStore.put(store.getColumnFamilyName(), tempVal);
-          if (readsFromMemstore == null) {
-            readsFromMemstore = new HashMap<String, Long>();
+          if (readsOnlyFromMemstore == null) {
+            readsOnlyFromMemstore = new HashMap<String, Long>();
           }
-          tempVal = readsFromMemstore.get(store.getColumnFamilyName());
+          tempVal = readsOnlyFromMemstore.get(store.getColumnFamilyName());
           if (tempVal == null) {
             tempVal = 0L;
           } else {
-            tempVal += store.getReadRequestsCountFromMemstore();
+            tempVal += store.getMemstoreOnlyReadsCount();
           }
-          readsFromMemstore.put(store.getColumnFamilyName(), tempVal);
+          readsOnlyFromMemstore.put(store.getColumnFamilyName(), tempVal);
         }
       }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java

@@ -92,18 +92,18 @@ public void run() {
             }
             mt.storeCount += 1;
             tempKey = tbl.getNameAsString() + UNDERSCORE + familyName;
-            Long tempVal = mt.perStoreMemstoreReadCount.get(tempKey);
+            Long tempVal = mt.perStoreMemstoreOnlyReadCount.get(tempKey);
             if (tempVal == null) {
               tempVal = 0L;
             }
-            memstoreReadCount = store.getReadRequestsCountFromMemstore() + tempVal;
+            memstoreReadCount = store.getMemstoreOnlyReadsCount() + tempVal;
             tempVal = mt.perStoreMixedReadCount.get(tempKey);
             if (tempVal == null) {
               tempVal = 0L;
             }
             mixedReadCount = store.getMixedReadRequestsCount() + tempVal;
             // accumulate the count
-            mt.perStoreMemstoreReadCount.put(tempKey, memstoreReadCount);
+            mt.perStoreMemstoreOnlyReadCount.put(tempKey, memstoreReadCount);
             mt.perStoreMixedReadCount.put(tempKey, mixedReadCount);
           }
 
@@ -150,17 +150,17 @@ public long getReadRequestCount(String table) {
   }
 
   @Override
-  public Map<String, Long> getMemstoreReadRequestsCount(String table) {
+  public Map<String, Long> getMemstoreOnlyReadRequestsCount(String table) {
     MetricsTableValues metricsTable = metricsTableMap.get(TableName.valueOf(table));
     if (metricsTable == null) {
       return null;
     } else {
-      return metricsTable.perStoreMemstoreReadCount;
+      return metricsTable.perStoreMemstoreOnlyReadCount;
     }
   }
 
   @Override
-  public Map<String, Long> getMixedRequestsCount(String table) {
+  public Map<String, Long> getMixedReadRequestsCount(String table) {
     MetricsTableValues metricsTable = metricsTableMap.get(TableName.valueOf(table));
     if (metricsTable == null) {
       return null;
@@ -340,7 +340,7 @@ private static class MetricsTableValues {
     long totalStoreFileAge;
     long referenceFileCount;
     long cpRequestCount;
-    Map<String, Long> perStoreMemstoreReadCount = new HashMap<>();
+    Map<String, Long> perStoreMemstoreOnlyReadCount = new HashMap<>();
     Map<String, Long> perStoreMixedReadCount = new HashMap<>();
   }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java

@@ -284,9 +284,9 @@ public interface Store {
   int getCurrentParallelPutCount();
 
   /**
-   * @return the number of read requests that was targeted at the memstore part of this store
+   * @return the number of read requests purely from the memstore.
    */
-  long getReadRequestsCountFromMemstore();
+  long getMemstoreOnlyReadsCount();
 
   /**
    * @return the number of read requests from the files under this store.

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java

@@ -94,9 +94,9 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
   private final long maxRowSize;
   private final long cellsPerHeartbeatCheck;
   @VisibleForTesting
-  long memstoreReads;
+  long memstorOnlyReads;
   @VisibleForTesting
-  long fileReads;
+  long mixedReads;
 
   // 1) Collects all the KVHeap that are eagerly getting closed during the
   //    course of a scan
@@ -641,9 +641,7 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
                * Increment the metric if all the cells are from memstore.
                * If not we will account it for mixed reads
                */
-              if (onlyFromMemstore && !heap.isCellFromMemstoreScanner()) {
-                onlyFromMemstore = false;
-              }
+              onlyFromMemstore = onlyFromMemstore && heap.isLatestCellFromMemstore();
               // Update the progress of the scanner context
               scannerContext.incrementSizeProgress(cellSize, cell.heapSize());
               scannerContext.incrementBatchProgress(1);
@@ -762,9 +760,9 @@ private void updateMetricsStore(boolean memstoreRead) {
     } else {
       // for testing.
       if (memstoreRead) {
-        memstoreReads++;
+        memstorOnlyReads++;
       } else {
-        fileReads++;
+        mixedReads++;
       }
     }
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperStub.java

@@ -187,14 +187,14 @@ public long getTotalRequestCount() {
   }
 
   @Override
-  public Map<String, Long> getMemstoreReadRequestsCount() {
+  public Map<String, Long> getMemstoreOnlyReadRequestsCount() {
     Map<String, Long> map = new HashMap<>();
     map.put("info", 0L);
     return map;
   }
 
   @Override
-  public Map<String, Long> getMixedReadRequestCount() {
+  public Map<String, Long> getMixedReadRequestsCount() {
     Map<String, Long> map = new HashMap<>();
     map.put("info", 0L);
     return map;

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java

@@ -405,7 +405,7 @@ public void testFullRowGetDoesNotOverreadWhenRowInsideOneBlock() throws IOExcept
       // We should have gone the optimize route 5 times totally... an INCLUDE for the four cells
       // in the row plus the DONE on the end.
       assertEquals(5, scanner.count.get());
-      assertEquals(1, scanner.memstoreReads);
+      assertEquals(1, scanner.memstorOnlyReads);
       // For a full row Get, there should be no opportunity for scanner optimization.
       assertEquals(0, scanner.optimization.get());
     }
@@ -481,7 +481,7 @@ public void testOptimizeAndGet() throws IOException {
       assertEquals("First qcode is SEEK_NEXT_COL and second INCLUDE_AND_SEEK_NEXT_ROW", 3,
         scanner.count.get());
       assertEquals("Memstore Read count should be", 1,
-        scanner.memstoreReads);
+        scanner.memstorOnlyReads);
     }
   }
 
@@ -581,7 +581,7 @@ public void testScanSameTimestamp() throws IOException {
       List<Cell> results = new ArrayList<>();
       assertEquals(true, scan.next(results));
       assertEquals(1, results.size());
-      assertEquals(1, scan.memstoreReads);
+      assertEquals(1, scan.memstorOnlyReads);
       assertEquals(kvs[0], results.get(0));
     }
   }