MAPREDUCE-4384. Race conditions in IndexCache (Kihwal Lee via tgraves)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1357937 13f79535-47bb-0310-9956-ffa450edef68

hadoop-mapreduce-project/CHANGES.txt

@@ -638,6 +638,8 @@ Release 0.23.3 - UNRELEASED
     MAPREDUCE-4392. Counters.makeCompactString() changed behavior from 0.20
     (Jason Lowe via bobby)
 
+    MAPREDUCE-4384. Race conditions in IndexCache (Kihwal Lee via tgraves)
+
 Release 0.23.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IndexCache.java

@@ -67,13 +67,13 @@ class IndexCache {
     if (info == null) {
       info = readIndexFileToCache(fileName, mapId, expectedIndexOwner);
     } else {
-      synchronized (info) {
-        while (null == info.mapSpillRecord) {
-          try {
-            info.wait();
-          } catch (InterruptedException e) {
-            throw new IOException("Interrupted waiting for construction", e);
-          }
+      while (isUnderConstruction(info)) {
+        try {
+          // In case the entry is ready after the above check but
+          // before the following wait, we do timed wait.
+          info.wait(200);
+        } catch (InterruptedException e) {
+          throw new IOException("Interrupted waiting for construction", e);
         }
       }
       LOG.debug("IndexCache HIT: MapId " + mapId + " found");
@@ -88,6 +88,12 @@ class IndexCache {
     return info.mapSpillRecord.getIndex(reduce);
   }
 
+  private boolean isUnderConstruction(IndexInformation info) {
+    synchronized(info) {
+      return (null == info.mapSpillRecord);
+    }
+  }
+
   private IndexInformation readIndexFileToCache(Path indexFileName,
                                                 String mapId,
                                                 String expectedIndexOwner)
@@ -95,13 +101,13 @@ class IndexCache {
     IndexInformation info;
     IndexInformation newInd = new IndexInformation();
     if ((info = cache.putIfAbsent(mapId, newInd)) != null) {
-      synchronized (info) {
-        while (null == info.mapSpillRecord) {
-          try {
-            info.wait();
-          } catch (InterruptedException e) {
-            throw new IOException("Interrupted waiting for construction", e);
-          }
+      while (isUnderConstruction(info)) {
+        try {
+          // In case the entry is ready after the above check but
+          // before the following wait, we do timed wait.
+          info.wait(200);
+        } catch (InterruptedException e) {
+          throw new IOException("Interrupted waiting for construction", e);
         }
       }
       LOG.debug("IndexCache HIT: MapId " + mapId + " found");
@@ -139,7 +145,7 @@ class IndexCache {
    */
   public void removeMap(String mapId) {
     IndexInformation info = cache.get(mapId);
-    if ((info != null) && (info.getSize() == 0)) {
+    if (info == null || ((info != null) && isUnderConstruction(info))) {
       return;
     }
     info = cache.remove(mapId);