mneethiraj commented on code in PR #538:
URL: https://github.com/apache/ranger/pull/538#discussion_r1968368920
##########
security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java:
##########
@@ -69,21 +70,42 @@ public static RangerRoleCache getInstance() {
}
public RangerRoles getLatestRangerRoleOrCached(String serviceName,
RoleDBStore roleDBStore, Long lastKnownRoleVersion, Long rangerRoleVersionInDB)
throws Exception {
- final RangerRoles ret;
+ RangerRoles ret = null;
if (lastKnownRoleVersion == null ||
!lastKnownRoleVersion.equals(rangerRoleVersionInDB)) {
- roleCacheWrapper = new RangerRoleCacheWrapper();
- ret =
roleCacheWrapper.getLatestRangerRoles(serviceName, roleDBStore,
lastKnownRoleVersion, rangerRoleVersionInDB);
- } else {
- ret = null;
+ ret = roleCacheWrapper.getRoles();
+
+ if (roleCacheWrapper.getRolesVersion() < rangerRoleVersionInDB) {
+ boolean lockResult = false;
+ try {
+ lockResult = lock.tryLock(waitTimeInSeconds,
TimeUnit.SECONDS);
+
+ if (lockResult) {
+ if (roleCacheWrapper.getRolesVersion() <
rangerRoleVersionInDB) {
+ ret = roleCacheWrapper.getLatestRangerRoles(
+ serviceName, roleDBStore,
lastKnownRoleVersion, rangerRoleVersionInDB);
+ }
Review Comment:
`ret` should be updated in `else` clause, right?
```
if (roleCacheWrapper.getRolesVersion() < rangerRoleVersionInDB) {
ret = roleCacheWrapper.getLatestRangerRoles(serviceName, roleDBStore,
lastKnownRoleVersion, rangerRoleVersionInDB);
} else {
ret = roleCacheWrapper.getRoles();
}
```
##########
security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java:
##########
@@ -46,6 +46,7 @@ public class RangerRoleCache {
RangerRoleCacheWrapper roleCacheWrapper;
Review Comment:
Consider marking `roleCacheWrapper` as `final`.
--
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: dev-unsubscr...@ranger.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
