svn commit: r1225316 - /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

[ns...@apache.org]

Author: nspiegelberg
Date: Wed Dec 28 19:40:14 2011
New Revision: 1225316

URL: http://svn.apache.org/viewvc?rev=1225316&view=rev
Log:
[HBASE-4997] Pull in HBASE-4897 and HBASE-4997

Summary:
This fixes 3 issues

1. It fixes double counting of batch.installed. In createTaskIfAbsent(), if the
newly constructed task is not eventually used then we have to remember to
decrement batch.installed

2. There was an unhandled race between the zk-callback thread updating a task
without a batch and the another thread installing a batch into that task.

3. Made sure that batch.installed is being accessed by only one thread. Easier
to argue correctness that way.

Test Plan: TestSplitLogManager passes

Reviewers: kannan, kranganathan, mbautin

Reviewed By: kannan

CC: hbase-eng@lists, pkhemani, kannan

Differential Revision: 375764

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java?rev=1225316&r1=1225315&r2=1225316&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java Wed Dec 28 19:40:14 2011
@@ -337,22 +337,22 @@ public class SplitLogManager implements 
         LOG.debug("unacquired orphan task is done " + path);
       }
     } else {
-      // if in stopTrackingTasks() we were to make tasks orphan instead of
-      // forgetting about them then we will have to handle the race when
-      // accessing task.batch here.
-      if (!task.isOrphan()) {
-        synchronized (task.batch) {
-          if (status == SUCCESS) {
-            task.batch.done++;
-          } else {
-            task.batch.error++;
-          }
-          if ((task.batch.done + task.batch.error) == task.batch.installed) {
+      synchronized (task) {
+        task.deleted = true;
+        // if in stopTrackingTasks() we were to make tasks orphan instead of
+        // forgetting about them then we will have to handle the race when
+        // accessing task.batch here.
+        if (!task.isOrphan()) {
+          synchronized (task.batch) {
+            if (status == SUCCESS) {
+              task.batch.done++;
+            } else {
+              task.batch.error++;
+            }
             task.batch.notify();
           }
         }
       }
-      task.deleted = true;
     }
     // delete the task node in zk. Keep trying indefinitely - its an async
     // call and no one is blocked waiting for this node to be deleted. All
@@ -595,12 +595,29 @@ public class SplitLogManager implements 
    */
   private Task createTaskIfAbsent(String path, TaskBatch batch) {
     Task oldtask;
+    // batch.installed is only changed via this function and
+    // a single thread touches batch.installed.
     oldtask = tasks.putIfAbsent(path, new Task(batch));
-    if (oldtask != null && oldtask.isOrphan()) {
-        LOG.info("Previously orphan task " + path +
-            " is now being waited upon");
-        oldtask.setBatch(batch);
-        return (null);
+    if (oldtask != null) {
+      // new task was not used.
+      batch.installed--;
+      synchronized (oldtask) {
+        if (oldtask.isOrphan()) {
+          if (oldtask.deleted) {
+            // The task is already done. Do not install the batch for this
+            // task because it might be too late for setDone() to update
+            // batch.done. There is no need for the batch creator to wait for
+            // this task to complete.
+            return (null);
+          }
+          // have to synchronize with setDone() when setting the batch on
+          // the old task
+          oldtask.setBatch(batch);
+        }
+      }
+      LOG.info("Previously orphan task " + path +
+          " is now being waited upon");
+      return (null);
     }
     return oldtask;
   }