You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ab...@apache.org on 2021/12/08 01:20:47 UTC
[ranger] branch master updated: RANGER-3538: Reduce the granularity of locking when building/retrieving a policy-engine within Ranger admin service
This is an automated email from the ASF dual-hosted git repository.
abhay pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push:
new d3af747 RANGER-3538: Reduce the granularity of locking when building/retrieving a policy-engine within Ranger admin service
d3af747 is described below
commit d3af7476dcab3719b8a75b506b10400640f3bf3e
Author: Abhay Kulkarni <ab...@apache.org>
AuthorDate: Tue Dec 7 16:58:25 2021 -0800
RANGER-3538: Reduce the granularity of locking when building/retrieving a policy-engine within Ranger admin service
---
.../apache/ranger/biz/RangerPolicyAdminCache.java | 124 +++++++++++++--------
.../RangerPolicyAdminCacheForEngineOptions.java | 15 ++-
2 files changed, 89 insertions(+), 50 deletions(-)
diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java
index 5a69231..47fa99c 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java
@@ -22,6 +22,8 @@ package org.apache.ranger.biz;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -37,9 +39,25 @@ import org.apache.ranger.plugin.util.RangerRoles;
import org.apache.ranger.plugin.util.ServicePolicies;
public class RangerPolicyAdminCache {
+
+ static class RangerPolicyAdminWrapper {
+ final RangerPolicyAdmin policyAdmin;
+ final Lock lock = new ReentrantLock();
+
+ RangerPolicyAdminWrapper(RangerPolicyAdmin policyAdmin) {
+ this.policyAdmin = policyAdmin;
+ }
+ RangerPolicyAdmin getPolicyAdmin() {
+ return policyAdmin;
+ }
+ Lock getLock() {
+ return lock;
+ }
+ }
+
private static final Log LOG = LogFactory.getLog(RangerPolicyAdminCache.class);
- private final Map<String, RangerPolicyAdmin> policyAdminCache = Collections.synchronizedMap(new HashMap<>());
+ private final Map<String, RangerPolicyAdminWrapper> policyAdminCache = Collections.synchronizedMap(new HashMap<>());
final RangerPolicyAdmin getServicePoliciesAdmin(String serviceName, ServiceStore svcStore, RoleStore roleStore, SecurityZoneStore zoneStore, RangerPolicyEngineOptions options) {
@@ -49,13 +67,13 @@ public class RangerPolicyAdminCache {
return null;
}
- RangerPolicyAdmin ret = policyAdminCache.get(serviceName);
-
long policyVersion;
long roleVersion;
RangerRoles roles;
boolean isRolesUpdated = true;
+ RangerPolicyAdminWrapper ret = policyAdminCache.get(serviceName);
+
try {
if (ret == null) {
policyVersion = -1L;
@@ -68,8 +86,8 @@ public class RangerPolicyAdminCache {
}
}
} else {
- policyVersion = ret.getPolicyVersion();
- roleVersion = ret.getRoleVersion();
+ policyVersion = ret.getPolicyAdmin().getPolicyVersion();
+ roleVersion = ret.getPolicyAdmin().getRoleVersion();
roles = roleStore.getRoles(serviceName, roleVersion);
if (roles == null) { // No changes to roles
@@ -82,70 +100,88 @@ public class RangerPolicyAdminCache {
if (policies != null) {
ret = addOrUpdatePolicyAdmin(ret, policies, roles, options);
- } else {
+
if (ret == null) {
- LOG.error("getPolicyAdmin(" + serviceName + "): failed to get any policies from service-store");
+ LOG.error("getPolicyAdmin(" + serviceName + "): failed to build engine from policies from service-store");
} else {
if (isRolesUpdated) {
- ret.setRoles(roles);
+ ret.getPolicyAdmin().setRoles(roles);
}
}
}
} catch (Exception exception) {
LOG.error("getPolicyAdmin(" + serviceName + "): failed to get latest policies from service-store", exception);
}
+
if (ret == null) {
LOG.error("Policy-engine is not built! Returning null policy-engine!");
} else {
- ret.setServiceStore(svcStore);
+ ret.getPolicyAdmin().setServiceStore(svcStore);
}
- return ret;
+ return ret == null ? null : ret.getPolicyAdmin();
+
}
- private RangerPolicyAdmin addOrUpdatePolicyAdmin(RangerPolicyAdmin policyAdmin, ServicePolicies policies, RangerRoles roles, RangerPolicyEngineOptions options) {
- final RangerPolicyAdmin ret;
- RangerPolicyAdminImpl oldPolicyAdmin = (RangerPolicyAdminImpl) policyAdmin;
-
- synchronized(this) {
- Boolean hasPolicyDeltas = RangerPolicyDeltaUtil.hasPolicyDeltas(policies);
- boolean isPolicyEngineShared = false;
-
- if (hasPolicyDeltas != null) {
- if (hasPolicyDeltas.equals(Boolean.TRUE)) {
- if (oldPolicyAdmin != null) {
- ret = RangerPolicyAdminImpl.getPolicyAdmin(oldPolicyAdmin, policies);
- if (ret != null) {
- ret.setRoles(roles);
- isPolicyEngineShared = true;
+ private RangerPolicyAdminWrapper addOrUpdatePolicyAdmin(RangerPolicyAdminWrapper policyAdminWrapper, ServicePolicies policies, RangerRoles roles, RangerPolicyEngineOptions options) {
+ final RangerPolicyAdminWrapper ret;
+
+ RangerPolicyAdmin policyAdmin = null;
+ boolean isPolicyEngineShared = false;
+
+ RangerPolicyAdminImpl oldPolicyAdmin = policyAdminWrapper == null ? null : (RangerPolicyAdminImpl) policyAdminWrapper.getPolicyAdmin();
+ Boolean hasPolicyDeltas = RangerPolicyDeltaUtil.hasPolicyDeltas(policies);
+
+ if (hasPolicyDeltas != null) {
+ if (hasPolicyDeltas.equals(Boolean.TRUE)) {
+ if (oldPolicyAdmin != null) {
+ boolean isLocked = false;
+
+ try {
+ policyAdminWrapper.getLock().lockInterruptibly();
+ isLocked = true;
+ } catch (Exception e) {
+ // Ignore
+ }
+
+ if (isLocked) {
+ try {
+ policyAdmin = RangerPolicyAdminImpl.getPolicyAdmin(oldPolicyAdmin, policies);
+ if (policyAdmin != null) {
+ policyAdmin.setRoles(roles);
+ isPolicyEngineShared = true;
+ }
+ } finally {
+ policyAdminWrapper.getLock().unlock();
}
- } else {
- LOG.error("Old policy engine is null! Cannot apply deltas without old policy engine!");
- ret = null;
}
} else {
- ret = addPolicyAdmin(policies, roles, options);
+ LOG.error("Old policy engine is null! Cannot apply deltas without old policy engine!");
}
} else {
- LOG.warn("Provided policies do not require policy change !! [" + policies + "]. Keeping old policy-engine!");
- ret = null;
+ policyAdmin = addPolicyAdmin(policies, roles, options);
}
+ } else {
+ LOG.warn("Provided policies do not require policy change !! [" + policies + "]. Keeping old policy-engine!");
+ policyAdmin = oldPolicyAdmin;
+ }
- if (ret != null) {
- if (LOG.isDebugEnabled()) {
- if (oldPolicyAdmin == null) {
- LOG.debug("Adding policy-engine to cache with serviceName:[" + policies.getServiceName() + "] as key");
- } else {
- LOG.debug("Replacing policy-engine in cache with serviceName:[" + policies.getServiceName() + "] as key");
- }
- }
- policyAdminCache.put(policies.getServiceName(), ret);
- if (oldPolicyAdmin != null && oldPolicyAdmin != ret) {
- oldPolicyAdmin.releaseResources(!isPolicyEngineShared);
+ if (policyAdmin != null) {
+ if (LOG.isDebugEnabled()) {
+ if (oldPolicyAdmin == null) {
+ LOG.debug("Adding policy-engine to cache with serviceName:[" + policies.getServiceName() + "] as key");
+ } else {
+ LOG.debug("Replacing policy-engine in cache with serviceName:[" + policies.getServiceName() + "] as key");
}
- } else {
- LOG.warn("Could not build new policy-engine.");
}
+ ret = new RangerPolicyAdminWrapper(policyAdmin);
+ policyAdminCache.put(policies.getServiceName(), ret);
+ if (oldPolicyAdmin != null && oldPolicyAdmin != policyAdmin) {
+ oldPolicyAdmin.releaseResources(!isPolicyEngineShared);
+ }
+ } else {
+ LOG.warn("Could not build new policy-engine.");
+ ret = null;
}
return ret;
diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java
index 224bdc2..771d320 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java
@@ -55,15 +55,18 @@ public class RangerPolicyAdminCacheForEngineOptions {
}
private RangerPolicyAdmin getServicePoliciesAdmin(String serviceName, ServiceStore svcStore, RoleStore roleStore, SecurityZoneStore zoneStore, RangerPolicyEngineOptions options) {
- RangerPolicyAdminCache policyAdminCache;
- synchronized (this) {
- policyAdminCache = policyAdminCacheForEngineOptions.get(options);
+ RangerPolicyAdminCache policyAdminCache = policyAdminCacheForEngineOptions.get(options);
- if (policyAdminCache == null) {
- policyAdminCache = new RangerPolicyAdminCache();
+ if (policyAdminCache == null) {
+ synchronized (this) {
+ policyAdminCache = policyAdminCacheForEngineOptions.get(options);
- policyAdminCacheForEngineOptions.put(options, policyAdminCache);
+ if (policyAdminCache == null) {
+ policyAdminCache = new RangerPolicyAdminCache();
+
+ policyAdminCacheForEngineOptions.put(options, policyAdminCache);
+ }
}
}