susheel-gupta commented on PR #5380: URL: https://github.com/apache/hadoop/pull/5380#issuecomment-1531437238
> Hi @susheel-gupta , Just took a look at your changes in this PR. > > I compared the old version of ParentQueue vs. AbstractParentQueue and found some differences: > > * the private int field with name 'numApplications' was volatile, now it is changed to AtomicInteger. What is the reason of this? > * In the method called 'addApplication', the writelock was locked during incrementing numApplications, now I can see it removed. Do you know why this was necessary? > * Same for method called 'removeApplication' > > Otherwise LGTM @szilard-nemeth There was this [warning](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5380/7/artifact/out/new-spotbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html) where they asked to fix the spotbugs, so I added this [commit](https://github.com/apache/hadoop/pull/5380/commits/0f0d8f21823c23d49baa620a62111e7c3a269193) . With the above changes in this [commit](https://github.com/apache/hadoop/pull/5380/commits/0f0d8f21823c23d49baa620a62111e7c3a269193) , the numApplications field is an instance of AtomicInteger, which provides an atomic incrementAndGet method to increment the value of the counter atomically. Since AtomicInteger provides atomic operations, we don't need to use locks or synchronization to ensure atomicity. With these changes, the "VO_VOLATILE_INCREMENT: An increment to a volatile field isn't atomic" issue should be resolved, and the code should now increment the numApplications count atomically. References: - https://www.digitalocean.com/community/tutorials/atomicinteger-java - https://www.baeldung.com/java-atomic-variables -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
