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

Viraj Jasani commented on HDFS-16139:
-------------------------------------

{quote}Do you means that it could be executed unexpected when run unit test?
{quote}
That is correct.
{quote}When deploy on production environment this variable is accessed/updated 
by Scheduler only?
{quote}
That is also correct as of today. However, if we add any more writer thread in 
future, it would be problematic in prod env as well. I believe in general, we 
should avoid using volatile for non-atomic operations (increment, decrement 
etc) so that we make it future proof. But yes, as of today, this problem is 
applicable with unit tests threads and Scheduler threads trying to write at the 
same time. Thanks for your interest [~hexiaoqiao].

> Update BPServiceActor Scheduler's nextBlockReportTime atomically
> ----------------------------------------------------------------
>
>                 Key: HDFS-16139
>                 URL: https://issues.apache.org/jira/browse/HDFS-16139
>             Project: Hadoop HDFS
>          Issue Type: Task
>            Reporter: Viraj Jasani
>            Assignee: Viraj Jasani
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> BPServiceActor#Scheduler's nextBlockReportTime is declared volatile and it 
> can be assigned/read by testing threads (through BPServiceActor#triggerXXX) 
> as well as by actor threads. Hence it is declared volatile but it is still 
> assigned non-atomically
> e.g
> {code:java}
> if (factor != 0) {
>   nextBlockReportTime += factor * blockReportIntervalMs;
> } else {
>   // If the difference between the present time and the scheduled
>   // time is very less, the factor can be 0, so in that case, we can
>   // ignore that negligible time, spent while sending the BRss and
>   // schedule the next BR after the blockReportInterval.
>   nextBlockReportTime += blockReportIntervalMs;
> }
> {code}
> We should convert it to AtomicLong to take care of concurrent assignments 
> while making sure that it is assigned atomically.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to