svn commit: r727154 - in /hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver: HRegion.java HRegionServer.java

[st...@apache.org]

Author: stack
Date: Tue Dec 16 13:11:54 2008
New Revision: 727154

URL: http://svn.apache.org/viewvc?rev=727154&view=rev
Log:
HBASE-900 Regionserver memory leak causing OOME during relatively modest bulk importing; part 3

Modified:
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=727154&r1=727153&r2=727154&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java Tue Dec 16 13:11:54 2008
@@ -839,13 +839,14 @@
     long sequenceId = -1L;
     long completeSequenceId = -1L;
     this.updatesLock.writeLock().lock();
+    // Get current size of memcaches.
+    final long currentMemcacheSize = this.memcacheSize.get();
     try {
       for (HStore s: stores.values()) {
         s.snapshot();
       }
       sequenceId = log.startCacheFlush();
       completeSequenceId = this.getCompleteCacheFlushSequenceId(sequenceId);
-      this.memcacheSize.set(0);
     } finally {
       this.updatesLock.writeLock().unlock();
     }
@@ -865,6 +866,8 @@
           compactionRequested = true;
         }
       }
+      // Set down the memcache size by amount of flush.
+      this.memcacheSize.addAndGet(-currentMemcacheSize);
     } catch (Throwable t) {
       // An exception here means that the snapshot was not persisted.
       // The hlog needs to be replayed so its content is restored to memcache.
@@ -896,12 +899,12 @@
     }
     
     if (LOG.isDebugEnabled()) {
-      String timeTaken = StringUtils.formatTimeDiff(System.currentTimeMillis(), 
-          startTime);
-      LOG.debug("Finished memcache flush for region " + this +
-        " in " +
-          (System.currentTimeMillis() - startTime) + "ms, sequence id=" +
-          sequenceId + ", compaction requested=" + compactionRequested);
+      long now = System.currentTimeMillis();
+      String timeTaken = StringUtils.formatTimeDiff(now, startTime);
+      LOG.debug("Finished memcache flush of ~" +
+        StringUtils.humanReadableInt(currentMemcacheSize) + " for region " +
+        this + " in " + (now - startTime) + "ms, sequence id=" + sequenceId +
+        ", compaction requested=" + compactionRequested);
       if (!regionInfo.isMetaRegion()) {
         this.historian.addRegionFlush(regionInfo, timeTaken);
       }
@@ -1274,8 +1277,6 @@
   /*
    * Check if resources to support an update.
    * 
-   * For now, just checks memcache saturation.
-   * 
    * Here we synchronize on HRegion, a broad scoped lock.  Its appropriate
    * given we're figuring in here whether this region is able to take on
    * writes.  This is only method with a synchronize (at time of writing),

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=727154&r1=727153&r2=727154&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Tue Dec 16 13:11:54 2008
@@ -847,7 +847,7 @@
       for (Map.Entry<Integer, HRegion> e: this.onlineRegions.entrySet()) {
         HRegion r = e.getValue();
         memcacheSize += r.memcacheSize.get();
-        synchronized(r.stores) {
+        synchronized (r.stores) {
           stores += r.stores.size();
           for(Map.Entry<Integer, HStore> ee: r.stores.entrySet()) {
             HStore store = ee.getValue();