diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index a8e0cb1406e..29205283d26 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/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 diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IndexCache.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IndexCache.java index 63c148d5f25..2dbdf119602 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IndexCache.java +++ b/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);