You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Abhay Kulkarni <ak...@hortonworks.com> on 2018/08/10 00:58:58 UTC
Review Request 68286: RANGER-2186: Increment service-specific policy
and tag versions after update transaction is committed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/
-----------------------------------------------------------
Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
Bugs: RANGER-2186
https://issues.apache.org/jira/browse/RANGER-2186
Repository: ranger
Description
-------
Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
Similar considerations apply for tag updates.
Diffs
-----
agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
Diff: https://reviews.apache.org/r/68286/diff/1/
Testing
-------
Passed all unit tests
Thanks,
Abhay Kulkarni
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
> On Aug. 10, 2018, 11:58 a.m., Zsombor Gegesy wrote:
> > security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
> > Lines 122 (patched)
> > <https://reviews.apache.org/r/68286/diff/1/?file=2070899#file2070899line125>
> >
> > Why this complicated is machinery with the thread locals and lists and registration is needed?
> >
> > You can easily have the same functionality - running a 'Runnable' object after a transaction finish, with just like this:
> >
> > ```java
> > public void executeOnTransactionCommit(Runnable runnable) {
> > TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() {
> > public void afterCommit() {
> > runnable.run();
> > }
> > }
> > }
> > ```
The main purpose of the 'machinery' is to group several tasks together and accomplish them as one atomic unit after transaction is committed. The tasks themselves have only one thing in common; that they need to be all done or all rolled back. Such atomicity cannot be achieved with the shown code fragment.
- Abhay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207072
-----------------------------------------------------------
On Aug. 10, 2018, 12:58 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2018, 12:58 a.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/1/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Zsombor Gegesy <zs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207072
-----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
Lines 122 (patched)
<https://reviews.apache.org/r/68286/#comment290255>
Why this complicated is machinery with the thread locals and lists and registration is needed?
You can easily have the same functionality - running a 'Runnable' object after a transaction finish, with just like this:
```java
public void executeOnTransactionCommit(Runnable runnable) {
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() {
public void afterCommit() {
runnable.run();
}
}
}
```
- Zsombor Gegesy
On Aug. 10, 2018, 12:58 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2018, 12:58 a.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/1/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207382
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On Aug. 15, 2018, 10:06 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2018, 10:06 p.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/RangerDaoManager.java 2788a6109
> security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java da89e041c
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/3/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/
-----------------------------------------------------------
(Updated Aug. 15, 2018, 10:06 p.m.)
Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
Changes
-------
Addressed review comment
Bugs: RANGER-2186
https://issues.apache.org/jira/browse/RANGER-2186
Repository: ranger
Description
-------
Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
Similar considerations apply for tag updates.
Diffs (updated)
-----
agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
security-admin/src/main/java/org/apache/ranger/db/RangerDaoManager.java 2788a6109
security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java da89e041c
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
Diff: https://reviews.apache.org/r/68286/diff/3/
Changes: https://reviews.apache.org/r/68286/diff/2-3/
Testing
-------
Passed all unit tests
Thanks,
Abhay Kulkarni
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207336
-----------------------------------------------------------
Fix it, then Ship it!
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
Line 93 (original), 122 (patched)
<https://reviews.apache.org/r/68286/#comment290692>
Instead of calling runRunnables() twice, for RUNNABLES_AFTER_COMMIT & RUNNABLES, consider creatig a single list and call once. It can avoid one additional transaction.
- Madhan Neethiraj
On Aug. 15, 2018, 4:23 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2018, 4:23 p.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/RangerDaoManager.java 2788a6109
> security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java da89e041c
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/2/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/
-----------------------------------------------------------
(Updated Aug. 15, 2018, 4:23 p.m.)
Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
Changes
-------
Addressed review comments
Bugs: RANGER-2186
https://issues.apache.org/jira/browse/RANGER-2186
Repository: ranger
Description
-------
Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
Similar considerations apply for tag updates.
Diffs (updated)
-----
agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
security-admin/src/main/java/org/apache/ranger/db/RangerDaoManager.java 2788a6109
security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java da89e041c
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
Diff: https://reviews.apache.org/r/68286/diff/2/
Changes: https://reviews.apache.org/r/68286/diff/1-2/
Testing
-------
Passed all unit tests
Thanks,
Abhay Kulkarni
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
> On Aug. 14, 2018, 8:31 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java
> > Line 124 (original), 130 (patched)
> > <https://reviews.apache.org/r/68286/diff/1/?file=2070900#file2070900line130>
> >
> > Instead of creating multiple Runnable objects, one for each serviceVersionInfo, consider handling all updates within a single Runnable.
All post-commit/post-completion tasks are run in a single transaction. Therefore, overhead of running multiple Runnable tasks is not significantly higher than running one consolidated task. However, the complexity of consolidation code (in all conditions) does not justify potential savings of having one consolidated task.
- Abhay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207252
-----------------------------------------------------------
On Aug. 10, 2018, 12:58 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2018, 12:58 a.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/1/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 68286: RANGER-2186: Increment service-specific
policy and tag versions after update transaction is committed
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68286/#review207252
-----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Lines 2821 (patched)
<https://reviews.apache.org/r/68286/#comment290584>
INCREMENT_POLICY_VERSION ==> POLICY_VERSION
INCREMENT_TAG_VERSION ==> TAG_VERSION
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Line 2864 (original), 2856 (patched)
<https://reviews.apache.org/r/68286/#comment290582>
Instead of creating multiple Runnable objects, one for each referringService, consider a single runnable that loops through referringServices.
Also, consider adding VERSION_TYPE_INCREMENT_POLICY_AND_TAG_VERSION so that both can be updated in a single call.
final VERION_TYPE versionToUpdate;
if (filterForServicePlugin && isTagVersionUpdateNeeded) {
versionToUpdate = VERSION_TYPE.INCREMENT_POLICY_AND_TAG_VERSION;
} else {
versionToUpdate = VERSION_TYPE.INCREMENT_POLICY_VERSION;
}
commitWork = new Runnable() {
@Override
public void run() {
for(XXService referringService : referringServices) {
persistVersionChange(daoMgr, referringService.getId(), versionToUpdate);
}
}
};
transactionSynchronizationAdapter.executeOnTransactionCommit(commitWork);
security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
Lines 171 (patched)
<https://reviews.apache.org/r/68286/#comment290585>
Consider avoiding afterCommit() method at the base class. Instead:
- have afterCompletion(status) call this method - only when status == STATUS_COMMITTED
- clear RUNNABLES_AFTER_COMMIT in afterCompletion(status). Currently this list might not be cleared if the transaction fails in commit.
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java
Line 124 (original), 130 (patched)
<https://reviews.apache.org/r/68286/#comment290583>
Instead of creating multiple Runnable objects, one for each serviceVersionInfo, consider handling all updates within a single Runnable.
- Madhan Neethiraj
On Aug. 10, 2018, 12:58 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68286/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2018, 12:58 a.m.)
>
>
> Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2186
> https://issues.apache.org/jira/browse/RANGER-2186
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Policy updates to different policies within a service, when successful, update the service's policy version. If the update transactions are concurrent, and executed on different ranger-admin servers (in HA configuration), then it is possible that policy-version of the transaction that commits later overwrites policy-version of earlier transaction, effectively losing track of the first change.
>
> If policy-version is updated after update to policy is committed, then the window of such loss is greatly reduced.
>
> Similar considerations apply for tag updates.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 69ded6dc8
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 0773616f9
> security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java 2a62fb408
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java e1003297a
> security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java cb496ea8b
>
>
> Diff: https://reviews.apache.org/r/68286/diff/1/
>
>
> Testing
> -------
>
> Passed all unit tests
>
>
> Thanks,
>
> Abhay Kulkarni
>
>