> On Jan. 4, 2023, 1:43 p.m., Vikas Kumar wrote: > > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java > > Lines 1110 (patched) > > <https://reviews.apache.org/r/74221/diff/4/?file=2272656#file2272656line1408> > > > > Concept of using ReeadWriteLock this way is really good, this way it > > is getting automatically unlocked due to AutoCloseable interface. > > > > But this is redundant in many files, at least the boilerplate code I > > have seen in three files. > > > > Can't we use this as wrapper over these locks, something like following > > (just an extension to the existing logic): > > > > class AutocloseableLocker<T extends Lock> implements AutoCloseable{ > > > > private T lock; > > public AutocloseableLocker(T lock){ > > this.lock = lock; > > } > > > > @Override > > public void close() throws Exception { > > lock.unlock(); > > } > > } > > > > > > Make this as utility class and create the instance of the same class > > whereever required and keep the ref of the instance in the required class. > > This way, we will have just one class for Read & Write and no need to keep > > these in all classes where such logic is required.
Good suggestion. Added AutoClosableLock in plugins-common library and updated KMS to use this class. - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74221/#review225053 ----------------------------------------------------------- On Jan. 4, 2023, 11:50 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74221/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2023, 11:50 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 > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/util/AutoClosableLock.java > PRE-CREATION > 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/5/ > > > Testing > ------- > > - added test script to call KMS APIs from multiple threads > - verified that stress_kms.py scripts completes successfully > > > Thanks, > > Madhan Neethiraj > >
