[jira] [Commented] (HAMA-971) An increment to a volatile field isn't atomic

[Edward J. Yoon (JIRA)]

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

Edward J. Yoon commented on HAMA-971:
-------------------------------------

Here's some side comment on AsyncRcvdMsgCheckpointImpl. If 
bsp.checkpoint.enabled property is set to true, each task checkpoints received 
messages periodically. 

{code}
  @Override
  public void loopBackBundle(BSPMessageBundle<M> bundle) throws IOException {
    peer.incrementCounter(BSPPeerImpl.PeerCounter.TOTAL_MESSAGES_RECEIVED, 
bundle.size());
    this.localQueueForNextIteration.addBundle(bundle);
    
    // TODO checkpoint bundle itself instead of unpacked messages. -- edwardyoon
    // notifyReceivedMessage(bundle);
  }

  @SuppressWarnings("unchecked")
  @Override
  public void loopBackMessage(Writable message) throws IOException {
    this.localQueueForNextIteration.add((M) message);
    notifyReceivedMessage((M) message);
  }
{code}

Currently  notifyReceivedMessage(BSPMessageBundle bundle) is not implemented 
yet, so this can be enabled only when hama.bsp.messenger.bundle is false. 

> An increment to a volatile field isn't atomic
> ---------------------------------------------
>
>                 Key: HAMA-971
>                 URL: https://issues.apache.org/jira/browse/HAMA-971
>             Project: Hama
>          Issue Type: Bug
>          Components: bsp core
>            Reporter: JongYoon Lim
>            Priority: Minor
>
> I found a defect from FindBugs Analysis.
> {code}
> // AsyncRcvdMsgCheckpointImpl.java
> volatile private long checkpointMessageCount;
> ...
> ++checkpointMessageCount;
> {code}
> {code}
> // Server.java
> private volatile int rpcCount = 0; // number of outstanding rpcs
> ...
> rpcCount--;
> rpcCount++;
> {code}
> This code increments a volatile field. Increments of volatile fields aren't 
> atomic. If more than one thread is incrementing the field at the same time, 
> increments could be lost.
> I think *AtomicInteger* and *AtomicLong* can be used instead of volatile 
> fields. 



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