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

Sangjin Lee commented on MAPREDUCE-5861:
----------------------------------------

Hmm, I don't think AtomicInteger adds any benefit here. The current version of 
the code is *correct in terms of thread safety* (and I would argue even 
volatile is not needed). The atomic increment concern happens only if the said 
variable is being read and updated by multiple threads. However, in this case 
only one thread (the task runner) reads and writes to this variable. As such, 
it gets trivial thread safety (via thread confinement).


> LocalContainerLauncher does not atomically update volatile variable 
> finishedSubMaps
> -----------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-5861
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-5861
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: Tsuyoshi OZAWA
>            Priority: Minor
>         Attachments: MAPREDUCE-5861.1.patch
>
>
> Around line 374:
> {code}
>           if (++finishedSubMaps == numMapTasks) {
>             doneWithMaps = true;
>           }
> {code}
> The increment of finishedSubMaps is not atomic.
> See the answer to 
> http://stackoverflow.com/questions/9749746/what-is-the-difference-of-atomic-volatile-synchronize
>  .
> AtomicInteger can be used to achieve atomicity.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to