diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index c4f7673d94..4ee9e40431 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/hadoop-mapreduce-project/CHANGES.txt @@ -625,7 +625,10 @@ Release 0.23.1 - Unreleased (ramya via acmurthy) MAPREDUCE-3764. Fixed resource usage metrics for queues and users. - (acmurthy) + (acmurthy) + + MAPREDUCE-3749. ConcurrentModificationException in counter groups. + (tomwhite) Release 0.23.0 - 2011-11-01 diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounterGroup.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounterGroup.java index de6c4f20c4..68fded801d 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounterGroup.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounterGroup.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.Map; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import com.google.common.collect.Maps; @@ -56,7 +57,7 @@ public AbstractCounterGroup(String name, String displayName, } @Override - public synchronized String getName() { + public String getName() { return name; } @@ -95,7 +96,7 @@ private T addCounterImpl(String name, String displayName, long value) { } @Override - public T findCounter(String counterName, String displayName) { + public synchronized T findCounter(String counterName, String displayName) { String saveName = limits.filterCounterName(counterName); T counter = findCounterImpl(saveName, false); if (counter == null) { @@ -109,7 +110,7 @@ public synchronized T findCounter(String counterName, boolean create) { return findCounterImpl(limits.filterCounterName(counterName), create); } - private T findCounterImpl(String counterName, boolean create) { + private synchronized T findCounterImpl(String counterName, boolean create) { T counter = counters.get(counterName); if (counter == null && create) { String localized = @@ -142,7 +143,7 @@ protected abstract T newCounter(String counterName, String displayName, @Override public synchronized Iterator iterator() { - return counters.values().iterator(); + return ImmutableSet.copyOf(counters.values()).iterator(); } /** diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounters.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounters.java index b09aaef793..0a6e3b75a5 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounters.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/counters/AbstractCounters.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.Map; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.common.collect.Maps; @@ -179,13 +180,14 @@ public synchronized C findCounter(String scheme, FileSystemCounter key) { * @return Set of counter names. */ public synchronized Iterable getGroupNames() { - return Iterables.concat(fgroups.keySet(), groups.keySet()); + return Iterables.concat(ImmutableSet.copyOf(fgroups.keySet()), + ImmutableSet.copyOf(groups.keySet())); } @Override - public Iterator iterator() { - return Iterators.concat(fgroups.values().iterator(), - groups.values().iterator()); + public synchronized Iterator iterator() { + return Iterators.concat(ImmutableSet.copyOf(fgroups.values()).iterator(), + ImmutableSet.copyOf(groups.values()).iterator()); } /** diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestCounters.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestCounters.java index 5df6ede9cb..926daf7d01 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestCounters.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestCounters.java @@ -21,9 +21,11 @@ import java.io.IOException; import java.text.ParseException; +import java.util.Iterator; import java.util.Random; import org.apache.hadoop.mapred.Counters.Counter; +import org.apache.hadoop.mapred.Counters.Group; import org.apache.hadoop.mapreduce.FileSystemCounter; import org.apache.hadoop.mapreduce.JobCounter; import org.apache.hadoop.mapreduce.TaskCounter; @@ -71,8 +73,6 @@ private void testCounter(Counters counter) throws ParseException { // Check for recovery from string assertEquals("Recovered counter does not match on content", counter, recoveredCounter); - assertEquals("recovered counter has wrong hash code", - counter.hashCode(), recoveredCounter.hashCode()); } @Test @@ -159,6 +159,28 @@ public void testLegacyNames() { "FILE_BYTES_READ").getValue()); } + @SuppressWarnings("deprecation") + @Test + public void testCounterIteratorConcurrency() { + Counters counters = new Counters(); + counters.incrCounter("group1", "counter1", 1); + Iterator iterator = counters.iterator(); + counters.incrCounter("group2", "counter2", 1); + iterator.next(); + } + + + @SuppressWarnings("deprecation") + @Test + public void testGroupIteratorConcurrency() { + Counters counters = new Counters(); + counters.incrCounter("group1", "counter1", 1); + Group group = counters.getGroup("group1"); + Iterator iterator = group.iterator(); + counters.incrCounter("group1", "counter2", 1); + iterator.next(); + } + public static void main(String[] args) throws IOException { new TestCounters().testCounters(); }