[ 
https://issues.apache.org/jira/browse/HADOOP-11274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14203326#comment-14203326
 ] 

Junping Du commented on HADOOP-11274:
-------------------------------------

Thanks for taking this, [~vinodkv]! The way of using ConcurrentHashMap looks 
fine especially we don't want to spend time to evaluate performance impact on 
locking whole conf object. 
The patch looks good to me. However, one thing need to be mentioned here is the 
Set of "finalParameters" meet the same situation as "updatingResource" that we 
should fix. This is get fixed in v2 patch where we were adding synchronized 
block in loadProperty(). If we want to get rid of any explicit lock in loading 
configuration, another way is to use Collections.synchronizedSet() to turn 
finalParameters into thread-safe.
Also, some minor indentation issues in Configuration(Configuration other).
{code}
+    synchronized(other) {
+
+     this.resources = (ArrayList<Resource>) other.resources.clone();
...
-     this.updatingResource = new HashMap<String, 
String[]>(other.updatingResource);
+      this.updatingResource =
+          new ConcurrentHashMap<String, String[]>(other.updatingResource);
      this.finalParameters = new HashSet<String>(other.finalParameters);
+     this.classLoader = other.getClassLoader();
...
{code}

> ConcurrentModificationException in Configuration Copy Constructor
> -----------------------------------------------------------------
>
>                 Key: HADOOP-11274
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11274
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: conf
>            Reporter: Junping Du
>            Assignee: Vinod Kumar Vavilapalli
>            Priority: Blocker
>         Attachments: HADOOP-11274-v2.patch, HADOOP-11274-v3.patch, 
> HADOOP-11274-v4.patch, HADOOP-11274.003.patch, HADOOP-11274.patch
>
>
> Exception as below happens in doing some configuration update in parallel:
> {noformat}
> java.util.ConcurrentModificationException
>       at java.util.HashMap$HashIterator.nextEntry(HashMap.java:922)
>       at java.util.HashMap$EntryIterator.next(HashMap.java:962)
>       at java.util.HashMap$EntryIterator.next(HashMap.java:960)
>       at java.util.HashMap.putAllForCreate(HashMap.java:554)
>       at java.util.HashMap.<init>(HashMap.java:298)
>       at org.apache.hadoop.conf.Configuration.<init>(Configuration.java:703)
> {noformat}
> In a constructor of Configuration - public Configuration(Configuration 
> other), the copy of updatingResource data structure in copy constructor is 
> not synchronized properly. 
> Configuration.get() eventually calls loadProperty() where updatingResource 
> gets updated. So, whats happening here is one thread is trying to do copy of 
> Configuration as demonstrated in stack trace and other thread is doing 
> Configuration.get(key) and than ConcurrentModificationException occurs 
> because copying of updatingResource is not synchronized in constructor. 
> We should make the update to updatingResource get synchronized, and also fix 
> other tiny synchronized issues there.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to