svn commit: r1345449 - /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

[mb...@apache.org]

Author: mbautin
Date: Sat Jun  2 06:26:56 2012
New Revision: 1345449

URL: http://svn.apache.org/viewvc?rev=1345449&view=rev
Log:
[jira] [HBASE-6121] [89-fb] Fix TaskMonitor/MultiPut multithreading bug

Author: amirshim

Summary:
We shouldn't clear an ArrayList that might be iterated on by another thread.

Specifically, multiput() calls clear() on ArrayList (to free up some memory) while MultiPut.toMap is iterating over that ArrayList in a different thread (called from MonitorTasks UI)

Test Plan: Run a load tester that does lots of multiputs, and access the web gui. Make sure it doesn't crash. (It's hard to test multithreaded bugs)

Reviewers: kannan

Reviewed By: kannan

CC: mbautin, rthiessen, nspiegelberg, hbase-eng@

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

Task ID: 1066743

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

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=1345449&r1=1345448&r2=1345449&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 Sat Jun  2 06:26:56 2012
@@ -146,6 +146,7 @@ public class HRegionServer implements HR
   private static final HMsg [] EMPTY_HMSG_ARRAY = new HMsg [] {};
   private static final String UNABLE_TO_READ_MASTER_ADDRESS_ERR_MSG =
       "Unable to read master address from ZooKeeper";
+  private static final ArrayList<Put> emptyPutArray = new ArrayList<Put>();
 
   // Set when a report to the master comes back with a message asking us to
   // shutdown.  Also set by call to stop when debugging or running unit tests
@@ -2923,14 +2924,20 @@ public class HRegionServer implements HR
 
       index++;
       if (index < size) {
-        // clear each regions list of Puts to save RAM except for the
-        // last one. We will lose the reference to the last one pretty
+        // remove the reference to the region list of Puts to save RAM except
+        // for the last one. We will lose the reference to the last one pretty
         // soon anyway; keep it for a little more, until we get back
         // to HBaseServer level, where we might need to pretty print
         // the MultiPut request for debugging slow/large puts.
         // Note: A single row "put" from client also end up in server
         // as a multiPut().
-        e.getValue().clear(); // clear some RAM
+        // We set the value to an empty array so taskmonitor can either get
+        //  the old value or an empty array. If we call clear() on the array,
+        //  then we might have a race condition where we're iterating over the
+        //  array at the same time we clear it (which throws an exception)
+        // This relies on the fact that Map.Entry.setValue() boils down to a
+        //   simple reference assignment, which is atomic
+        e.setValue(emptyPutArray); // clear some RAM
       }
     }