You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Madhan Neethiraj <ma...@apache.org> on 2022/12/10 21:02:06 UTC

Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/
-----------------------------------------------------------

(Updated Dec. 10, 2022, 9:02 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.


Changes
-------

addressed suggestions from Vikas (in 74220)


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 (updated)
-----

  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 


Diff: https://reviews.apache.org/r/74221/diff/3/

Changes: https://reviews.apache.org/r/74221/diff/2-3/


Testing
-------

Called KMS APIs simultaneously from multiple threads and verified that failures seen with concurrent requests prior to this patch are resolved with this patch.


Thanks,

Madhan Neethiraj


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.

> On Jan. 5, 2023, 1:54 p.m., Vikas Kumar wrote:
> > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java
> > Line 86 (original), 87 (patched)
> > <https://reviews.apache.org/r/74221/diff/6/?file=2273446#file2273446line90>
> >
> >     Hi Madhan, This is not a code review comment, being new to KMS, just trying to understand the context of using these locks. Please correct me if I am wrong.
> >     
> >     As per new code, we are acquiring WriteLock on any CUD operation (like create, delete, rollover, flush) and ReadLock on redaing API like getKeyVersion() etc.
> >     
> >     There will be just one instance of "RangerkeyStoreprovider" per KMS process and we are taking one ReadWriteLock. That means, one CUD API operation will block the entire traffic even if more container threads are available. Although I understand that frequency of CUD APIs will be very less.
> >     
> >     If we are trying to fix the ConcurrentModificationException, then, simply making that DS thread safe should work like ConcurrentHashMap. 
> >     
> >     I agree that in such cases, still chances of getting older values (like thread1 reads and thread2 was about to update that map/DS) exists. But we are reading the last updated value. If we are trying to ensure this part, then, what if GET and CUD API requests land on two different KMS instances?
> >     
> >     Can you please help with few cases where we are getting this concurrency issue. My only concern is, acquiring locks at top level service API may impact the number of request one KMS instance can serve.

Vikas - I suggest running stress_kms.py (included in this patch) to reproduce failures in KMS handling of multiple simultaneous callers. For example, consider execution of following REST API calls simultaneously:
1. KMS.createKey()
2. KMS.rolloverKey(...)
3. KMS.invalidateCache(...)

Each of these methods update RangerKeyStoreProvider.cache and RangerKeyStore.keyEntries - either to add/remove/clear. Protecting calls to these object with use of ConcurrentHashMap doesn't address failures in application handling of changes performed in other threads. Hence the lock should be moved up to a higher level.


- Madhan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225061
-----------------------------------------------------------


On Jan. 5, 2023, 2:17 a.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74221/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2023, 2:17 a.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/6/
> 
> 
> Testing
> -------
> 
> - added test script to call KMS APIs from multiple threads
> - verified that stress_kms.py scripts completes successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Vikas Kumar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225061
-----------------------------------------------------------




kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java
Line 86 (original), 87 (patched)
<https://reviews.apache.org/r/74221/#comment313869>

    Hi Madhan, This is not a code review comment, being new to KMS, just trying to understand the context of using these locks. Please correct me if I am wrong.
    
    As per new code, we are acquiring WriteLock on any CUD operation (like create, delete, rollover, flush) and ReadLock on redaing API like getKeyVersion() etc.
    
    There will be just one instance of "RangerkeyStoreprovider" per KMS process and we are taking one ReadWriteLock. That means, one CUD API operation will block the entire traffic even if more container threads are available. Although I understand that frequency of CUD APIs will be very less.
    
    If we are trying to fix the ConcurrentModificationException, then, simply making that DS thread safe should work like ConcurrentHashMap. 
    
    I agree that in such cases, still chances of getting older values (like thread1 reads and thread2 was about to update that map/DS) exists. But we are reading the last updated value. If we are trying to ensure this part, then, what if GET and CUD API requests land on two different KMS instances?
    
    Can you please help with few cases where we are getting this concurrency issue. My only concern is, acquiring locks at top level service API may impact the number of request one KMS instance can serve.



kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java
Line 166 (original), 115 (patched)
<https://reviews.apache.org/r/74221/#comment313870>

    Again same question, what race condition we are trying to avoid here ?


- Vikas Kumar


On Jan. 5, 2023, 2:17 a.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74221/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2023, 2:17 a.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/6/
> 
> 
> Testing
> -------
> 
> - added test script to call KMS APIs from multiple threads
> - verified that stress_kms.py scripts completes successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Vikas Kumar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225062
-----------------------------------------------------------



Hi Madhan, I have reviewed from the concurrency perspective and provided all the inputs that I had. Thanks.

- Vikas Kumar


On Jan. 5, 2023, 2:17 a.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74221/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2023, 2:17 a.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/6/
> 
> 
> Testing
> -------
> 
> - added test script to call KMS APIs from multiple threads
> - verified that stress_kms.py scripts completes successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by bhavik patel <bh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225100
-----------------------------------------------------------


Ship it!




Ship It!

- bhavik patel


On Jan. 5, 2023, 2:17 a.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74221/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2023, 2:17 a.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/6/
> 
> 
> Testing
> -------
> 
> - added test script to call KMS APIs from multiple threads
> - verified that stress_kms.py scripts completes successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/
-----------------------------------------------------------

(Updated Jan. 5, 2023, 2:17 a.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.


Changes
-------

minor updates


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 (updated)
-----

  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/6/

Changes: https://reviews.apache.org/r/74221/diff/5-6/


Testing
-------

- added test script to call KMS APIs from multiple threads
- verified that stress_kms.py scripts completes successfully


Thanks,

Madhan Neethiraj


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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.


Changes
-------

addressed review comments


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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/74221/diff/4-5/


Testing
-------

- added test script to call KMS APIs from multiple threads
- verified that stress_kms.py scripts completes successfully


Thanks,

Madhan Neethiraj


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Vikas Kumar <ta...@gmail.com>.

> On Dec. 23, 2022, 9:33 p.m., Madhan Neethiraj wrote:
> > Vikas, Abhay - reminder to review the patch. Thanks!

I am doing the review from my end.


- Vikas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225025
-----------------------------------------------------------


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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225025
-----------------------------------------------------------



Vikas, Abhay - reminder to review the patch. Thanks!

- Madhan Neethiraj


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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.

> On Jan. 4, 2023, 3:23 p.m., Vikas Kumar wrote:
> > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java
> > Lines 106 (patched)
> > <https://reviews.apache.org/r/74221/diff/4/?file=2272656#file2272656line108>
> >
> >     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.

Thanks for reviewing and the suggestion that lock is not needed in RangerKeyStore, as the caller already has obtained necesary lock. Your observation is correct. I removed lock/unlock in RangerKeyStore.


- Madhan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225055
-----------------------------------------------------------


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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Vikas Kumar <ta...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.

> 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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Vikas Kumar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74221/#review225053
-----------------------------------------------------------




kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java
Line 131 (original), 109 (patched)
<https://reviews.apache.org/r/74221/#comment313862>

    This constructor should further call the third contructor.
    
    this(daoManager, false, null)



kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java
Lines 1110 (patched)
<https://reviews.apache.org/r/74221/#comment313863>

    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.


- 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
> 
>


Re: Review Request 74221: RANGER-3989: fix for failure in KMS APIs due to ConcurrentModificationException

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/74221/diff/3-4/


Testing (updated)
-------

- added test script to call KMS APIs from multiple threads
- verified that stress_kms.py scripts completes successfully


Thanks,

Madhan Neethiraj