----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74221/#review225055 -----------------------------------------------------------
Fix it, then Ship it! kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java Lines 106 (patched) <https://reviews.apache.org/r/74221/#comment313864> Just trying to understand, do we really need this ReadWriteLock ? If we are using lock to protect keyEntries's put & get, the why this map is ConcurrentHashMap ? Further, I have traced the usage of this class, everywhere instance of this class is being created inside a method during request execution (not instance memeber) and going out of scope once method completes. And these methods are single threaded, so do we really need these locks in this class ? In one class only, RangerKeyStoreProvider, instance of this class is kept as member of the class. But call to this instance's methods are protected by ReadWrite lock in RangerKeyStoreProvider only. Please re-consider the need of these locks in this class. - Vikas Kumar On Dec. 11, 2022, 11:47 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74221/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2022, 11:47 p.m.) > > > Review request for ranger, Ankita Sinha, Don Bosco Durai, Kishor > Gollapalliwar, Abhay Kulkarni, Mehul Parikh, Monika Kachhadiya, Pradeep > Agrawal, Ramesh Mani, Siddhesh Phatak, Sailaja Polavarapu, Subhrat Chaudhary, > Vikas Kumar, and Velmurugan Periasamy. > > > Bugs: RANGER-3989 > https://issues.apache.org/jira/browse/RANGER-3989 > > > Repository: ranger > > > Description > ------- > > - addressed concurrency issues in dealing with simultaneous requests > - JPA object loading improvements, to avoid cache overheads > - code readability improvements > > > Diffs > ----- > > kms/config/kms-webapp/kms-logback.xml 16e82a656 > > kms/src/main/java/org/apache/hadoop/crypto/key/AzureKeyVaultClientAuthenticator.java > 13897ac3e > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKMSDB.java 812e7dfc4 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java > 8dc129069 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java > cb5739f61 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java > c37e98ee5 > kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMS.java > c83382d64 > > kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java > d9f1b5b8e > kms/src/main/java/org/apache/ranger/kms/biz/RangerKMSStartUp.java aae722b39 > kms/src/main/java/org/apache/ranger/kms/dao/BaseDao.java f97154cb9 > kms/src/main/java/org/apache/ranger/kms/dao/DaoManager.java c28e78535 > kms/src/main/java/org/apache/ranger/kms/dao/RangerKMSDao.java cb64310c2 > kms/src/main/java/org/apache/ranger/kms/dao/RangerMasterKeyDao.java > 22edc9a13 > kms/src/main/resources/META-INF/kms_jpa_named_queries.xml 94d5fa67c > ranger-tools/src/main/python/stress/stress_kms.py PRE-CREATION > > > Diff: https://reviews.apache.org/r/74221/diff/4/ > > > Testing > ------- > > - added test script to call KMS APIs from multiple threads > - verified that stress_kms.py scripts completes successfully > > > Thanks, > > Madhan Neethiraj > >
