You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by li...@apache.org on 2012/12/05 20:20:17 UTC

svn commit: r1417593 - in /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase: metrics/MetricsRate.java metrics/RequestMetrics.java regionserver/HRegion.java regionserver/HRegionServer.java regionserver/metrics/RegionServerMetrics.java

Author: liyin
Date: Wed Dec  5 19:20:16 2012
New Revision: 1417593

URL: http://svn.apache.org/viewvc?rev=1417593&view=rev
Log:
[HBASE-7276] Optimize the read/write requests metrics in the RegionServer level

Author: andrewl

Summary:
This diff removes redundant read/write request metrics
and merges Region's metrics to RegionServer

Test Plan: Unit tests

Reviewers: liyintang, christianl, kannan

Reviewed By: liyintang

Differential Revision: https://phabricator.fb.com/D645831

Task ID: 1879160

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/MetricsRate.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/RequestMetrics.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerMetrics.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/MetricsRate.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/MetricsRate.java?rev=1417593&r1=1417592&r2=1417593&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/MetricsRate.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/MetricsRate.java Wed Dec  5 19:20:16 2012
@@ -56,6 +56,10 @@ public class MetricsRate extends Metrics
     value++;
   }
 
+  public synchronized void set(final int value) {
+    this.value = value;
+  }
+
   private synchronized void intervalHeartBeat() {
     long now = System.currentTimeMillis();
     long diff = (now-ts)/1000;

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/RequestMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/RequestMetrics.java?rev=1417593&r1=1417592&r2=1417593&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/RequestMetrics.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/metrics/RequestMetrics.java Wed Dec  5 19:20:16 2012
@@ -39,11 +39,11 @@ public class RequestMetrics {
     return totalRequestCount;
   }
 
-  public synchronized void incrTotalRequstCount(long incr) {
+  public synchronized void incrTotalRequestCount(long incr) {
     this.totalRequestCount += incr;
   }
 
-  public synchronized void incrTotalRequstCount() {
+  public synchronized void incrTotalRequestCount() {
     this.totalRequestCount ++;
   }
 
@@ -70,4 +70,4 @@ public class RequestMetrics {
     return requestPerSecond;
   }
 
-}
\ No newline at end of file
+}

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1417593&r1=1417592&r2=1417593&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Wed Dec  5 19:20:16 2012
@@ -184,8 +184,8 @@ public class HRegion implements HeapSize
   // private byte [] name = null;
 
   protected final AtomicLong memstoreSize = new AtomicLong(0);
-  private RequestMetrics readRequests = null;
-  private RequestMetrics writeRequests = null;
+  protected final RequestMetrics readRequests = new RequestMetrics();
+  protected final RequestMetrics writeRequests = new RequestMetrics();
 
   private HRegionServer regionServer = null;
   /**
@@ -456,7 +456,7 @@ public class HRegion implements HeapSize
   }
 
   /**
-   * HRegion constructor.  his constructor should only be used for testing and
+   * HRegion constructor. This constructor should only be used for testing and
    * extensions.  Instances of HRegion should be instantiated with the
    * {@link HRegion#newHRegion(Path, HLog, FileSystem, Configuration, org.apache.hadoop.hbase.HRegionInfo, FlushRequester)} method.
    *
@@ -479,7 +479,6 @@ public class HRegion implements HeapSize
    * failed.  Can be null.
    *
    * @see HRegion#newHRegion(Path, HLog, FileSystem, Configuration, org.apache.hadoop.hbase.HRegionInfo, FlushRequester)
-
    */
   public HRegion(Path tableDir, HLog log, FileSystem fs,
       Configuration confParam, final HRegionInfo regionInfo,
@@ -529,9 +528,7 @@ public class HRegion implements HeapSize
     this.waitOnMemstoreBlock =
         conf.getBoolean(HConstants.HREGION_MEMSTORE_WAIT_ON_BLOCK, true);
     this.scannerReadPoints = new ConcurrentHashMap<RegionScanner, Long>();
-    
-    this.readRequests =new RequestMetrics();
-    this.writeRequests =new RequestMetrics();
+
   }
 
   /**
@@ -740,6 +737,7 @@ public class HRegion implements HeapSize
 
   /** @return a HRegionInfo object for this region */
   public HRegionInfo getRegionInfo() {
+    readRequests.incrTotalRequestCount();
     return this.regionInfo;
   }
 
@@ -1662,6 +1660,7 @@ public class HRegion implements HeapSize
   }
 
   protected InternalScanner getScanner(Scan scan, List<KeyValueScanner> additionalScanners) throws IOException {
+    readRequests.incrTotalRequestCount();
     newScannerLock.readLock().lock();
     try {
       if (this.closed.get()) {
@@ -1739,6 +1738,7 @@ public class HRegion implements HeapSize
    */
   public void delete(Delete delete, Integer lockid, boolean writeToWAL)
   throws IOException {
+    writeRequests.incrTotalRequestCount();
     checkReadOnly();
     checkResources();
     Integer lid = null;
@@ -1753,7 +1753,7 @@ public class HRegion implements HeapSize
       delete(delete.getFamilyMap(), writeToWAL);
 
     } finally {
-      if(lockid == null) releaseRowLock(lid);
+      if(lockid == null) internalReleaseRowLock(lid);
       splitsAndClosesLock.readLock().unlock();
     }
   }
@@ -1821,7 +1821,6 @@ public class HRegion implements HeapSize
   public void delete(Map<byte[], List<KeyValue>> familyMap, boolean writeToWAL)
   throws IOException {
     long now = EnvironmentEdgeManager.currentTimeMillis();
-    this.writeRequests.incrTotalRequstCount();
 
     byte [] byteNow = Bytes.toBytes(now);
     boolean flush = false;
@@ -1910,7 +1909,7 @@ public class HRegion implements HeapSize
     // read lock, resources may run out.  For now, the thought is that this
     // will be extremely rare; we'll deal with it when it happens.
     checkResources();
-    this.writeRequests.incrTotalRequstCount();
+    writeRequests.incrTotalRequestCount();
     splitsAndClosesLock.readLock().lock();
 
     try {
@@ -1927,7 +1926,7 @@ public class HRegion implements HeapSize
         // All edits for the given row (across all column families) must happen atomically.
         put(put.getFamilyMap(), writeToWAL);
       } finally {
-        if(lockid == null) releaseRowLock(lid);
+        if(lockid == null) internalReleaseRowLock(lid);
       }
     } finally {
       splitsAndClosesLock.readLock().unlock();
@@ -1978,7 +1977,7 @@ public class HRegion implements HeapSize
    */
   public OperationStatusCode[] batchMutateWithLocks(Pair<Mutation, Integer>[] putsAndLocks,
       String methodName) throws IOException {
-    this.writeRequests.incrTotalRequstCount();
+    writeRequests.incrTotalRequestCount();
     BatchOperationInProgress<Pair<Mutation, Integer>> batchOp =
       new BatchOperationInProgress<Pair<Mutation,Integer>>(putsAndLocks);
 
@@ -2179,6 +2178,7 @@ public class HRegion implements HeapSize
   public boolean checkAndMutate(byte [] row, byte [] family, byte [] qualifier,
       byte [] expectedValue, Writable w, Integer lockId, boolean writeToWAL)
   throws IOException{
+    writeRequests.incrTotalRequestCount();
     checkReadOnly();
     //TODO, add check for value length or maybe even better move this to the
     //client if this becomes a global setting
@@ -2228,7 +2228,7 @@ public class HRegion implements HeapSize
         }
         return false;
       } finally {
-        if(lockId == null) releaseRowLock(lid);
+        if(lockId == null) internalReleaseRowLock(lid);
       }
     } finally {
       splitsAndClosesLock.readLock().unlock();
@@ -2814,6 +2814,7 @@ public class HRegion implements HeapSize
    * @return The id of the held lock.
    */
   public Integer obtainRowLock(final byte [] row) throws IOException {
+    readRequests.incrTotalRequestCount();
     return internalObtainRowLock(row, true);
   }
 
@@ -2893,7 +2894,12 @@ public class HRegion implements HeapSize
    * Release the row lock!
    * @param lockid  The lock ID to release.
    */
-  void releaseRowLock(final Integer lockid) {
+  public void releaseRowLock(final Integer lockid) {
+    readRequests.incrTotalRequestCount();
+    internalReleaseRowLock(lockid);
+  }
+
+  private void internalReleaseRowLock(final Integer lockid) {
     synchronized (lockedRows) {
       byte[] row = lockIds.remove(lockid);
       lockedRows.remove(row);
@@ -3180,7 +3186,6 @@ public class HRegion implements HeapSize
           } while (moreRows
               && (getOriginalScan().getCurrentPartialResponseSize() < 
                   maxResponseSize && currentNbRows < nbRows));
-          readRequests.incrTotalRequstCount(currentNbRows);
           scanResult = new ScanResult(moreRows, 
               outResults.toArray(new Result[0]));
         } catch (IOException e) {
@@ -3208,6 +3213,7 @@ public class HRegion implements HeapSize
      */
     public synchronized Result[] nextRows(int nbRows, String metric) 
     throws IOException {
+      readRequests.incrTotalRequestCount();
       preCondition();
       boolean prefetchingEnabled = getOriginalScan().getServerPrefetching();
       int limit = this.getOriginalScan().getBatch();
@@ -3251,7 +3257,6 @@ public class HRegion implements HeapSize
     @Override
     public synchronized boolean next(List<KeyValue> outResults, int limit,
         String metric) throws IOException {
-      readRequests.incrTotalRequstCount();
       preCondition();
       boolean returnResult;
       if (outResults.isEmpty()) {
@@ -3390,6 +3395,7 @@ public class HRegion implements HeapSize
 
     @Override
     public synchronized void close() {
+      readRequests.incrTotalRequestCount();
       if (storeHeap != null) {
         storeHeap.close();
         storeHeap = null;
@@ -3537,7 +3543,7 @@ public class HRegion implements HeapSize
           Writables.getBytes(r.getRegionInfo())));
       meta.put(HConstants.CATALOG_FAMILY, edits);
     } finally {
-      meta.releaseRowLock(lid);
+      meta.internalReleaseRowLock(lid);
     }
   }
 
@@ -3904,6 +3910,7 @@ public class HRegion implements HeapSize
    * @throws IOException read exceptions
    */
   public Result get(final Get get, final Integer lockid) throws IOException {
+    readRequests.incrTotalRequestCount();
     // Verify families are all valid
     if (get.hasFamilies()) {
       for (byte [] family: get.familySet()) {
@@ -3952,6 +3959,8 @@ public class HRegion implements HeapSize
 
     Integer lid = null;
 
+    writeRequests.incrTotalRequestCount();
+
     splitsAndClosesLock.readLock().lock();
     try {
       // 1. run all pre-hooks before the atomic operation
@@ -4023,7 +4032,7 @@ public class HRegion implements HeapSize
     } finally {
       if (lid != null) {
         // 11. release the row lock
-        releaseRowLock(lid);
+        internalReleaseRowLock(lid);
       }
       if (flush) {
         // 12. Flush cache if needed. Do it outside update lock.
@@ -4047,7 +4056,7 @@ public class HRegion implements HeapSize
   throws IOException {
     // to be used for metrics
     long before = EnvironmentEdgeManager.currentTimeMillis();
-    this.writeRequests.incrTotalRequstCount();
+    writeRequests.incrTotalRequestCount();
 
     checkRow(row);
     boolean flush = false;
@@ -4094,7 +4103,7 @@ public class HRegion implements HeapSize
       flush = isFlushSize(size);
     } finally {
       this.updatesLock.readLock().unlock();
-      releaseRowLock(lid);
+      internalReleaseRowLock(lid);
       HRegion.writeOps.incrementAndGet();
     }
 

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1417593&r1=1417592&r2=1417593&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Wed Dec  5 19:20:16 2012
@@ -652,10 +652,10 @@ public class HRegionServer implements HR
           try {
             MemoryUsage memory =
               ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
+            doMetrics();
             HServerLoad hsl = new HServerLoad((numReads.get() + numWrites.get()),
               (int)(memory.getUsed()/1024/1024),
               (int)(memory.getMax()/1024/1024));
-            doMetrics();
             for (HRegion r: onlineRegions.values()) {
               hsl.addRegionInfo(createRegionLoad(r));
             }
@@ -1338,16 +1338,10 @@ public class HRegionServer implements HR
   }
 
   protected void metrics() {
-    int numReads = this.numReads.get();
-    this.numReads.addAndGet(-numReads);
-
-    int numWrites = this.numWrites.get();
-    this.numWrites.addAndGet(-numWrites);
+    int numReads = 0;
+    int numWrites = 0;
 
     this.metrics.regions.set(this.onlineRegions.size());
-    this.metrics.incrementRequests(numReads + numWrites);
-    this.metrics.numReads.inc(numReads);
-    this.metrics.numWrites.inc(numWrites);
     // Is this too expensive every three seconds getting a lock on onlineRegions
     // and then per store carried?  Can I make metrics be sloppier and avoid
     // the synchronizations?
@@ -1370,6 +1364,8 @@ public class HRegionServer implements HR
       for (Map.Entry<Integer, HRegion> e: this.onlineRegions.entrySet()) {
         HRegion r = e.getValue();
         memstoreSize += r.memstoreSize.get();
+        numReads += r.readRequests.getTotalRequestCount();
+        numWrites += r.writeRequests.getTotalRequestCount();
         synchronized (r.stores) {
           stores += r.stores.size();
           for(Map.Entry<byte [], Store> ee: r.stores.entrySet()) {
@@ -1427,6 +1423,12 @@ public class HRegionServer implements HR
       HRegion.setNumericMetric(e.getKey(), e.getValue().longValue());
     }
 
+    this.numReads.set(numReads);
+    this.numWrites.set(numWrites);
+
+    this.metrics.requests.set(numReads + numWrites);
+    this.metrics.numReads.set(numReads);
+    this.metrics.numWrites.set(numWrites);
     this.metrics.stores.set(stores);
     this.metrics.storefiles.set(storefiles);
     this.metrics.memstoreSizeMB.set((int)(memstoreSize/(1024*1024)));
@@ -2320,7 +2322,6 @@ public class HRegionServer implements HR
   @Override
   public HRegionInfo getRegionInfo(final byte [] regionName)
   throws NotServingRegionException {
-    numReads.incrementAndGet();
     return getRegion(regionName).getRegionInfo();
   }
 
@@ -2330,7 +2331,6 @@ public class HRegionServer implements HR
     final byte [] row, final byte [] family)
   throws IOException {
     checkOpen();
-    numReads.incrementAndGet();
     try {
       // locate the region we're operating on
       HRegion region = getRegion(regionName);
@@ -2346,7 +2346,6 @@ public class HRegionServer implements HR
   @Override
   public Result get(byte [] regionName, Get get) throws IOException {
     checkOpen();
-    numReads.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       return region.get(get, getLockFromId(get.getLockId()));
@@ -2359,7 +2358,6 @@ public class HRegionServer implements HR
   public Result[] get(byte[] regionName, List<Get> gets)
       throws IOException {
     checkOpen();
-    numReads.addAndGet(gets.size());
     Result[] rets = new Result[gets.size()];
     try {
       HRegion region = getRegion(regionName);
@@ -2382,14 +2380,12 @@ public class HRegionServer implements HR
       throw new IOException("Invalid arguments to atomicMutation " +
       "regionName is null");
     }
-    numWrites.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       if (!region.getRegionInfo().isMetaTable()) {
         this.cacheFlusher.reclaimMemStoreMemory();
       }
       for (RowMutations arm: armList) {
-        numWrites.incrementAndGet();
         region.mutateRow(arm);
       }
     } catch (Throwable t) {
@@ -2406,7 +2402,6 @@ public class HRegionServer implements HR
   @Override
   public boolean exists(byte [] regionName, Get get) throws IOException {
     checkOpen();
-    numReads.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       Result r = region.get(get, getLockFromId(get.getLockId()));
@@ -2423,7 +2418,6 @@ public class HRegionServer implements HR
       throw new IllegalArgumentException("update has null row");
 
     checkOpen();
-    numWrites.incrementAndGet();
     HRegion region = getRegion(regionName);
     try {
       if (!region.getRegionInfo().isMetaTable()) {
@@ -2463,7 +2457,6 @@ public class HRegionServer implements HR
         opWithLocks[i++] = new Pair<Mutation, Integer>(p, lock);
       }
 
-      numWrites.addAndGet(mutations.size());
       OperationStatusCode[] codes = region.batchMutateWithLocks(opWithLocks,
           methodName);
       for (i = 0; i < codes.length; i++) {
@@ -2480,7 +2473,6 @@ public class HRegionServer implements HR
       final byte [] family, final byte [] qualifier, final byte [] value,
       final Writable w, Integer lock) throws IOException {
     checkOpen();
-    numWrites.incrementAndGet();
     HRegion region = getRegion(regionName);
     try {
       if (!region.getRegionInfo().isMetaTable()) {
@@ -2549,7 +2541,6 @@ public class HRegionServer implements HR
     if (npe != null) {
       throw new IOException("Invalid arguments to openScanner", npe);
     }
-    numReads.incrementAndGet();
     try {
       HRegion r = getRegion(regionName);
       return addScanner(r.getScanner(scan));
@@ -2631,11 +2622,7 @@ public class HRegionServer implements HR
         throw e;
       }
       this.leases.renewLease(scannerName);
-      Result[] results = s.nextRows(nbRows, HRegion.METRIC_NEXTSIZE);
-      if (results != null) {
-        numReads.addAndGet(results.length);
-      }
-      return results;
+      return s.nextRows(nbRows, HRegion.METRIC_NEXTSIZE);
     } catch (Throwable t) {
       if (t instanceof NotServingRegionException) {
         String scannerName = String.valueOf(scannerId);
@@ -2649,7 +2636,6 @@ public class HRegionServer implements HR
   public void close(final long scannerId) throws IOException {
     try {
       checkOpen();
-      numReads.incrementAndGet();
       String scannerName = String.valueOf(scannerId);
       InternalScanner s = scanners.remove(scannerName);
       if (s != null) {
@@ -2695,7 +2681,6 @@ public class HRegionServer implements HR
     checkOpen();
     try {
       boolean writeToWAL = delete.getWriteToWAL();
-      numWrites.incrementAndGet();
       HRegion region = getRegion(regionName);
       if (!region.getRegionInfo().isMetaTable()) {
         this.cacheFlusher.reclaimMemStoreMemory();
@@ -2728,7 +2713,6 @@ public class HRegionServer implements HR
       io.initCause(npe);
       throw io;
     }
-    numReads.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       Integer r = region.obtainRowLock(row);
@@ -2787,7 +2771,6 @@ public class HRegionServer implements HR
       io.initCause(npe);
       throw io;
     }
-    numReads.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       String lockName = String.valueOf(lockId);
@@ -3232,7 +3215,6 @@ public class HRegionServer implements HR
       throw new IOException("Invalid arguments to incrementColumnValue " +
       "regionName is null");
     }
-    numWrites.incrementAndGet();
     try {
       HRegion region = getRegion(regionName);
       long retval = region.incrementColumnValue(row, family, qualifier, amount,

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerMetrics.java?rev=1417593&r1=1417592&r2=1417593&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerMetrics.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerMetrics.java Wed Dec  5 19:20:16 2012
@@ -113,7 +113,7 @@ public class RegionServerMetrics impleme
   /*
    * Count of requests to the regionservers since last call to metrics update
    */
-  private final MetricsRate requests = new MetricsRate("requests", registry);
+  public final MetricsRate requests = new MetricsRate("requests", registry);
 
   /**
    * Count of stores open on the regionserver.
@@ -419,13 +419,6 @@ public class RegionServerMetrics impleme
     }
   }
 
-  /**
-   * @param inc How much to add to requests.
-   */
-  public void incrementRequests(final int inc) {
-    this.requests.inc(inc);
-  }
-
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();