Repository: ranger Updated Branches: refs/heads/master ea9d66a24 -> e0c1e355a
RANGER-1826: Import of bulk policies is causing OOM and Apparent Deadlock Project: http://git-wip-us.apache.org/repos/asf/ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/ranger/commit/e0c1e355 Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/e0c1e355 Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/e0c1e355 Branch: refs/heads/master Commit: e0c1e355a94cdecdf60e6d9eb0c54ff6d3bd412d Parents: ea9d66a Author: pradeep <[email protected]> Authored: Mon Oct 9 17:26:21 2017 +0530 Committer: pradeep <[email protected]> Committed: Tue Oct 10 14:37:18 2017 +0530 ---------------------------------------------------------------------- .../org/apache/ranger/biz/ServiceDBStore.java | 63 +++++++++++++++++++- .../common/RangerServicePoliciesCache.java | 2 +- .../org/apache/ranger/common/db/BaseDao.java | 19 ++++++ .../org/apache/ranger/rest/ServiceREST.java | 12 +++- .../apache/ranger/biz/TestServiceDBStore.java | 28 ++++----- 5 files changed, 105 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ranger/blob/e0c1e355/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java index bb43c53..89743ae 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java @@ -1657,9 +1657,11 @@ public class ServiceDBStore extends AbstractServiceStore { } List<XXPolicy> policies = daoMgr.getXXPolicy().findByServiceId(service.getId()); + RangerPolicy rangerPolicy =null; for(XXPolicy policy : policies) { LOG.info("Deleting Policy, policyName: " + policy.getName()); - deletePolicy(policy.getId()); + rangerPolicy = getPolicy(policy.getId()); + deletePolicy(rangerPolicy); } XXServiceConfigMapDao configDao = daoMgr.getXXServiceConfigMap(); @@ -2008,6 +2010,34 @@ public class ServiceDBStore extends AbstractServiceStore { LOG.info("Policy Deleted Successfully. PolicyName : " + policyName); } + public void deletePolicy(RangerPolicy policy) throws Exception { + if(policy == null) { + return; + } + if(LOG.isDebugEnabled()) { + LOG.debug("==> ServiceDBStore.deletePolicy(" + policy.getId() + ")"); + } + RangerService service = getServiceByName(policy.getService()); + if(service == null) { + throw new Exception("service does not exist - name='" + policy.getService()); + } + Long version = policy.getVersion(); + if(version == null) { + version = Long.valueOf(1); + LOG.info("Found Version Value: `null`, so setting value of version to 1, While updating object, version should not be null."); + } else { + version = Long.valueOf(version.longValue() + 1); + } + policy.setVersion(version); + List<XXTrxLog> trxLogList = policyService.getTransactionLog(policy, RangerPolicyService.OPERATION_DELETE_CONTEXT); + deleteExistingPolicyItemsNative(policy); + deleteExistingPolicyResourcesNative(policy); + daoMgr.getXXPolicy().deletePolicyIDReference("id",policy.getId()); + handlePolicyUpdate(service, true); + dataHistService.createObjectDataHistory(policy, RangerDataHistService.ACTION_DELETE); + bizUtil.createTrxLog(trxLogList); + } + @Override public RangerPolicy getPolicy(Long id) throws Exception { return policyService.read(id); @@ -3019,6 +3049,37 @@ public class ServiceDBStore extends AbstractServiceStore { return true; } + private Boolean deleteExistingPolicyItemsNative(RangerPolicy policy) { + if(policy == null) { + return false; + } + XXPolicyItemDao policyItemDao = daoMgr.getXXPolicyItem(); + List<XXPolicyItem> policyItems = policyItemDao.findByPolicyId(policy.getId()); + for(XXPolicyItem policyItem : policyItems) { + Long polItemId = policyItem.getId(); + daoMgr.getXXPolicyItemRowFilterInfo().deletePolicyIDReference("policy_item_id", polItemId); + daoMgr.getXXPolicyItemDataMaskInfo().deletePolicyIDReference("policy_item_id", polItemId); + daoMgr.getXXPolicyItemGroupPerm().deletePolicyIDReference("policy_item_id", polItemId); + daoMgr.getXXPolicyItemUserPerm().deletePolicyIDReference("policy_item_id", polItemId); + daoMgr.getXXPolicyItemCondition().deletePolicyIDReference("policy_item_id", polItemId); + daoMgr.getXXPolicyItemAccess().deletePolicyIDReference("policy_item_id", polItemId); + } + daoMgr.getXXPolicyItem().deletePolicyIDReference("policy_id", policy.getId()); + return true; + } + + private Boolean deleteExistingPolicyResourcesNative(RangerPolicy policy) { + if(policy == null) { + return false; + } + List<XXPolicyResource> resources = daoMgr.getXXPolicyResource().findByPolicyId(policy.getId()); + for(XXPolicyResource resource : resources) { + daoMgr.getXXPolicyResourceMap().deletePolicyIDReference("resource_id", resource.getId()); + daoMgr.getXXPolicyResource().deletePolicyIDReference("id", resource.getId()); + } + return true; + } + @Override public Boolean getPopulateExistingBaseFields() { return populateExistingBaseFields; http://git-wip-us.apache.org/repos/asf/ranger/blob/e0c1e355/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java b/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java index 7d1f28c..eb20f69 100644 --- a/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java +++ b/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java @@ -270,7 +270,7 @@ public class RangerServicePoliciesCache { policy.setCreateTime(null); policy.setUpdatedBy(null); policy.setUpdateTime(null); - policy.setGuid(null); + // policy.setGuid(null); /* this is used by import policy */ // policy.setName(null); /* this is used by GUI in policy list page */ // policy.setDescription(null); /* this is used by export policy */ policy.setResourceSignature(null); http://git-wip-us.apache.org/repos/asf/ranger/blob/e0c1e355/security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java b/security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java index c2832ea..51c0de5 100644 --- a/security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java +++ b/security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java @@ -250,6 +250,25 @@ public abstract class BaseDao<T> { } } + public boolean deletePolicyIDReference(String paramName,long oldID) { + Table table = tClass.getAnnotation(Table.class); + if(table != null) { + String tableName = table.name(); + String query = "delete from " + tableName + " where " +paramName+"=" + oldID; + if (logger.isDebugEnabled()) { + logger.debug("Delete Query:" + query); + } + int count=getEntityManager().createNativeQuery(query).executeUpdate(); + getEntityManager().flush(); + if(count>0){ + return true; + } + }else{ + logger.warn("Required annotation `Table` not found"); + } + return false; + } + public String getDBVersion(){ String dbVersion="Not Available"; String query ="SELECT 1"; http://git-wip-us.apache.org/repos/asf/ranger/blob/e0c1e355/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java index d8f217d..e4d8a57 100644 --- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java +++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java @@ -2243,6 +2243,7 @@ public class ServiceREST { int totalDeletedPilicies = 0; if (CollectionUtils.isNotEmpty(sourceServices) && CollectionUtils.isNotEmpty(destinationServices)) { + RangerPolicyValidator validator = validatorFactory.getPolicyValidator(svcStore); for (int i = 0; i < sourceServices.size(); i++) { if (!destinationServices.get(i).isEmpty()) { RangerPolicyList servicePolicies = null; @@ -2252,12 +2253,17 @@ public class ServiceREST { if (CollectionUtils.isNotEmpty(rangerPolicyList)) { for (RangerPolicy rangerPolicy : rangerPolicyList) { if (rangerPolicy != null) { - if (rangerPolicy.getId() != null){ - deletePolicy(rangerPolicy.getId()); + try { + validator.validate(rangerPolicy.getId(), Action.DELETE); + ensureAdminAccess(rangerPolicy.getService(), rangerPolicy.getResources()); + svcStore.deletePolicy(rangerPolicy); + totalDeletedPilicies = totalDeletedPilicies + 1; if (LOG.isDebugEnabled()) { LOG.debug("Policy " + rangerPolicy.getName() + " deleted successfully." ); + LOG.debug("TotalDeletedPilicies: " +totalDeletedPilicies); } - totalDeletedPilicies = totalDeletedPilicies + 1; + } catch(Throwable excp) { + LOG.error("deletePolicy(" + rangerPolicy.getId() + ") failed", excp); } } } http://git-wip-us.apache.org/repos/asf/ranger/blob/e0c1e355/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java index 976fd0c..c51aa2e 100644 --- a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java +++ b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java @@ -1197,8 +1197,8 @@ public class TestServiceDBStore { policyItem.setUpdateTime(new Date()); policyItemList.add(policyItem); - List<XXPolicyItemDataMaskInfo> policyItemDataMaskInfoList = new ArrayList<XXPolicyItemDataMaskInfo>(); - List<XXPolicyItemRowFilterInfo> policyItemRowFilterInfoList = new ArrayList<XXPolicyItemRowFilterInfo>(); + //List<XXPolicyItemDataMaskInfo> policyItemDataMaskInfoList = new ArrayList<XXPolicyItemDataMaskInfo>(); + //List<XXPolicyItemRowFilterInfo> policyItemRowFilterInfoList = new ArrayList<XXPolicyItemRowFilterInfo>(); List<XXPolicyItemCondition> policyItemConditionList = new ArrayList<XXPolicyItemCondition>(); XXPolicyItemCondition policyItemCondition = new XXPolicyItemCondition(); @@ -1311,34 +1311,34 @@ public class TestServiceDBStore { .thenReturn(policyItemList); Mockito.when(daoManager.getXXPolicyItemDataMaskInfo()).thenReturn(xxPolicyItemDataMaskInfoDao); - Mockito.when(xxPolicyItemDataMaskInfoDao.findByPolicyItemId(policyItem.getId())).thenReturn(policyItemDataMaskInfoList); + //Mockito.when(xxPolicyItemDataMaskInfoDao.findByPolicyItemId(policyItem.getId())).thenReturn(policyItemDataMaskInfoList); Mockito.when(daoManager.getXXPolicyItemRowFilterInfo()).thenReturn(xxPolicyItemRowFilterInfoDao); - Mockito.when(xxPolicyItemRowFilterInfoDao.findByPolicyItemId(policyItem.getId())).thenReturn(policyItemRowFilterInfoList); + //Mockito.when(xxPolicyItemRowFilterInfoDao.findByPolicyItemId(policyItem.getId())).thenReturn(policyItemRowFilterInfoList); Mockito.when(daoManager.getXXPolicyItemCondition()).thenReturn( xPolicyItemConditionDao); - Mockito.when( + /*Mockito.when( xPolicyItemConditionDao.findByPolicyItemId(policyItemCondition .getId())).thenReturn(policyItemConditionList); - + */ Mockito.when(daoManager.getXXPolicyItemGroupPerm()).thenReturn( xPolicyItemGroupPermDao); - Mockito.when( + /*Mockito.when( xPolicyItemGroupPermDao.findByPolicyItemId(policyItem.getId())) .thenReturn(policyItemGroupPermList); - + */ Mockito.when(daoManager.getXXPolicyItemUserPerm()).thenReturn( xPolicyItemUserPermDao); - Mockito.when(xPolicyItemUserPermDao.findByPolicyItemId(Id)).thenReturn( - policyItemUserPermList); + /*Mockito.when(xPolicyItemUserPermDao.findByPolicyItemId(Id)).thenReturn( + policyItemUserPermList);*/ Mockito.when(daoManager.getXXPolicyItemAccess()).thenReturn( xPolicyItemAccessDao); - Mockito.when( + /*Mockito.when( xPolicyItemAccessDao.findByPolicyItemId(policyItemAccess .getId())).thenReturn(policyItemAccessList); - + */ Mockito.when(daoManager.getXXPolicyResource()).thenReturn( xPolicyResourceDao); Mockito.when(xPolicyResourceDao.findByPolicyId(policyResource.getId())) @@ -1346,10 +1346,10 @@ public class TestServiceDBStore { Mockito.when(daoManager.getXXPolicyResourceMap()).thenReturn( xPolicyResourceMapDao); - Mockito.when( + /*Mockito.when( xPolicyResourceMapDao.findByPolicyResId(policyResourceMap .getId())).thenReturn(policyResourceMapList); - + */ Mockito.when(daoManager.getXXService()).thenReturn(xServiceDao); Mockito.when(xServiceDao.getById(Id)).thenReturn(xService);
