> On Feb. 17, 2025, 8:11 p.m., Ramesh Mani wrote: > > security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java > > Lines 111 (patched) > > <https://reviews.apache.org/r/75363/diff/1/?file=2297663#file2297663line111> > > > > Can this check be done before the locking outside of try block?
Performing the check getRolesVersion() >= rolesVersionInDB outside the try block may lead to thread safety issues. The rolesVersion may not reflect the updated version written by another thread. To ensure thread safety, it's recommended to perform this check while holding the lock. This guarantees that the version check is atomic and reflects the latest updates. - Vyom ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75363/#review227257 ----------------------------------------------------------- On Feb. 17, 2025, 4:43 p.m., gooch gooch wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75363/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2025, 4:43 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/1/ > > > Testing > ------- > > Tested locally. > > > Thanks, > > gooch gooch > >
