msamirkhan commented on a change in pull request #2198:
URL: https://github.com/apache/hadoop/pull/2198#discussion_r466692858
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/UsersManager.java
##########
@@ -581,15 +581,29 @@ public Resource
getComputedResourceLimitForAllUsers(String userName,
return userSpecificUserLimit;
}
+ protected long getLatestVersionOfUsersState() {
+ readLock.lock();
+ try {
+ return latestVersionOfUsersState;
+ } finally {
+ readLock.unlock();
+ }
+ }
+
/*
* Recompute user-limit under following conditions: 1. cached user-limit does
* not exist in local map. 2. Total User count doesn't match with local
cached
* version.
*/
private boolean isRecomputeNeeded(SchedulingMode schedulingMode,
String nodePartition, boolean isActive) {
- return (getLocalVersionOfUsersState(nodePartition, schedulingMode,
- isActive) != latestVersionOfUsersState.get());
+ readLock.lock();
+ try {
+ return (getLocalVersionOfUsersState(nodePartition, schedulingMode,
+ isActive) != latestVersionOfUsersState);
+ } finally {
+ readLock.unlock();
+ }
Review comment:
Previously it didn't readLock here but it calls
getLocalVersionOfUsersState which does lock so doesn't save anything. Also,
without lock latestVersionOfUsersState can possibly return negative value when
it is being reset to 0 in another thread in userLimitNeedsRecompute().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
