You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Ramachandran Krishnan <ra...@gmail.com> on 2022/12/10 10:12:19 UTC
Re: Review Request 74112: RANGER-3903:Improvement in RangerPolicyDeltaUtil--> applyDeltas method
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74112/
-----------------------------------------------------------
(Updated Dec. 10, 2022, 10:12 a.m.)
Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
Summary (updated)
-----------------
RANGER-3903:Improvement in RangerPolicyDeltaUtil--> applyDeltas method
Bugs: RANGER-3903
https://issues.apache.org/jira/browse/RANGER-3903
Repository: ranger
Description
-------
After going through the below code snippets in the master branch
while (iter.hasNext()) {
RangerPolicy policy = iter.next();
if (policyId.equals(policy.getId()) && (changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
deletedPolicies.add(policy);
iter.remove();
}
}
switch (changeType) {
case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE:
{
if (CollectionUtils.isNotEmpty(deletedPolicies)) {
LOG.warn("Unexpected: found existing policy for CHANGE_TYPE_POLICY_CREATE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
case RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE:
{
if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_UPDATE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
case RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE:
{
if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_DELETE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
default:
break;
}
1st#improvment:
From the above code, we iterate delta policies and check if this policy exists in the existing policy, we add that to deletePolicies list.
The delta change type condtion for created/updated/deleted is added on top of the if condition so adding the condition again is not necessary
2nd#improvment:
From the above code, we see for each element in the deltas ,we iterate policies and check if this delta policy exists in the existing policy, we add that to deletePolicies list.
Solution:
We need to use Map instead of iterating policies for every element of deltas --> Map<Long,List<RangerPolicy>> policiesIdMap
The building index map key will be policyId and the value will be no of policies associated with the same policy Id
For each policy in the deltas ,we check in the policiesIdMap whether the same policyId is present or not ?.
if yes ,we will add all the associated policy list into deletePolicies and remove the policyId from policiesIdMap
After end of the iteration ,we will iterate the policiesIdMap and get all the policylist assicated with policyId and add into result set
This will give better performance when the policies list is huge.
Diffs
-----
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerPolicyDeltaUtil.java e9223fe69
Diff: https://reviews.apache.org/r/74112/diff/5/
Testing
-------
Tested the below Rest API's to make sure everything works fine
1. ServiceREST Rest API :GET /plugins/policies/download/{serviceName}
2. ServiceREST Rest API :GET /plugins/secure/policies/download/{serviceName}
Thanks,
Ramachandran Krishnan
Re: Review Request 74112: RANGER-3903:Improvement in RangerPolicyDeltaUtil--> applyDeltas method
Posted by Ramachandran Krishnan <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74112/
-----------------------------------------------------------
(Updated Dec. 11, 2022, 7:04 a.m.)
Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
Bugs: RANGER-3903
https://issues.apache.org/jira/browse/RANGER-3903
Repository: ranger
Description (updated)
-------
After going through the below code snippets in the master branch
while (iter.hasNext()) {
RangerPolicy policy = iter.next();
if (policyId.equals(policy.getId()) && (changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
deletedPolicies.add(policy);
iter.remove();
}
}
switch (changeType) {
case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE:
{
if (CollectionUtils.isNotEmpty(deletedPolicies)) {
LOG.warn("Unexpected: found existing policy for CHANGE_TYPE_POLICY_CREATE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
case RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE:
{
if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_UPDATE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
case RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE:
{
if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_DELETE: " + Arrays.toString(deletedPolicies.toArray()));
}
break;
}
default:
break;
}
1st#improvement:
From the above code, we iterate delta policies and check if this policy exists in the existing policy, we add that to deletePolicies list.
The delta change type condition for created/updated/deleted is added on top of the if the condition so adding the condition again is not necessary
2nd#improvement:
From the above code, we see for each element in the deltas,we iterate policies and check if this delta policy exists in the existing policy, we add that to deletePolicies list.
Solution:
We need to use Map instead of iterating policies for every element of deltas --> Map<Long,List<RangerPolicy>> policiesIdMap
The building index map key will be policyId and the value will be no of policies associated with the same policy Id
For each policy in the deltas , we check in the policiesIdMap whether the same policyId is present or not ?.
if yes ,we will add all the associated policy list into deletePolicies and remove the policyId from policiesIdMap
After end of the iteration,we will iterate the policiesIdMap and get all the policylist associated with policyId and add into the result set
This will give better performance when the policies list is huge.
Diffs
-----
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerPolicyDeltaUtil.java e9223fe69
Diff: https://reviews.apache.org/r/74112/diff/5/
Testing
-------
Tested the below Rest API's to make sure everything works fine
1. ServiceREST Rest API :GET /plugins/policies/download/{serviceName}
2. ServiceREST Rest API :GET /plugins/secure/policies/download/{serviceName}
Thanks,
Ramachandran Krishnan