----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75363/#review227265 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java Lines 100 (patched) <https://reviews.apache.org/r/75363/#comment315505> The control reaches this point only when an exception occurs in roleCacheWrapper.getLatestRangerRoles(). In the modified code, we're returning roleCacheWrapper.getRoles() twice. Can you simplify this using a method-level reference variable (ret) that gets updated as needed, and finally return this reference? This approach will significantly improve code readability. - Vyom Tiwari On Feb. 20, 2025, 3:42 p.m., gooch gooch wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75363/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2025, 3:42 p.m.) > > > Review request for ranger. > > > Repository: ranger > > > Description > ------- > > This issue will occur when the number of Ranger plugin clients is > sufficiently large. When the server performs an update operation on roles, > which in turn causes the role version numbers in the database to be updated, > multiple clients will concurrently obtain the latest information of all > roles. During this process, the update of the cached roles in the Ranger > Admin Server is triggered multiple times. In a concurrent scenario, this > operation will generate a large number of unnecessary table-scanning updates. > The relevant code is located at > > ``` > org.apache.ranger.common.RangerRoleCache.RangerRoleCacheWrapper#getLatestRangerRoles. > > ``` > > The problem lies in the fact that after lock.tryLock, the thread that > acquires the lock will definitely call the roleDBStore.getRoles method once. > In a concurrent scenario, the previous threads have already triggered the > update. > > Specifically, I only made one modification to the role, and the role version > number increased by one. At this time, one thread holds the lock and is > performing a table-scanning update, while there are 10 new threads waiting to > compete for the lock. Subsequently, after each of these 10 threads acquires > the lock, they will update the cache again. > > The way this patch solves the problem: > After acquiring the lock, a check is performed to determine whether the role > version number in the cache is already greater than or equal to the version > number retrieved from the database. If this condition evaluates to true, the > cached data can be directly returned. > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java > 247a48b46 > > > Diff: https://reviews.apache.org/r/75363/diff/2/ > > > Testing > ------- > > Tested locally. > > > Thanks, > > gooch gooch > >