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 2025/03/22 00:24:57 UTC
(ranger) branch ranger-2.7 updated: RANGER-5134: Fix policy label processing in concurrent sessions (#545)
This is an automated email from the ASF dual-hosted git repository.
abhi pushed a commit to branch ranger-2.7
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.7 by this push:
new f04e8ddc0 RANGER-5134: Fix policy label processing in concurrent sessions (#545)
f04e8ddc0 is described below
commit f04e8ddc0de97117db11453fc3aa3aff3960a472
Author: Abhishek Kumar <ab...@apache.org>
AuthorDate: Fri Mar 21 17:02:25 2025 -0700
RANGER-5134: Fix policy label processing in concurrent sessions (#545)
(cherry picked from commit e93c017c68376f8f8b815de2ba1ce4708f666e26)
---
.../java/org/apache/ranger/biz/ServiceDBStore.java | 49 ++++++++++++++--------
.../RangerTransactionSynchronizationAdapter.java | 32 +++++++-------
.../org/apache/ranger/db/XXPolicyLabelMapDao.java | 18 +++++++-
.../main/resources/META-INF/jpa_named_queries.xml | 7 +++-
4 files changed, 69 insertions(+), 37 deletions(-)
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 7b2b918e1..5eefd0b94 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
@@ -2177,34 +2177,49 @@ public void run() {
getOrCreateLabel();
}
- private void getOrCreateLabel() {
- if (LOG.isDebugEnabled()) {
- LOG.debug("==> AssociatePolicyLabel.getOrCreateLabel(policyId=" + xPolicy.getId() + ", label=" + policyLabel + ")");
- }
+ private boolean doesPolicyExist(XXPolicy xPolicy) {
+ return daoMgr.getXXPolicy().getById(xPolicy.getId()) != null;
+ }
- XXPolicyLabel xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
+ private void getOrCreateLabel() {
+ LOG.debug("==> AssociatePolicyLabel.getOrCreateLabel(policyId={}, label={})", xPolicy.getId(), policyLabel);
- if (xxPolicyLabel == null) {
- xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
+ if (doesPolicyExist(xPolicy)) {
+ LOG.debug("Searching for policyLabel: {}", policyLabel);
+ XXPolicyLabel xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
+ LOG.debug("Search returned: {}", xxPolicyLabel);
if (xxPolicyLabel == null) {
+ LOG.debug("Creating policyLabel: {}", policyLabel);
xxPolicyLabel = new XXPolicyLabel();
+
xxPolicyLabel.setPolicyLabel(policyLabel);
+
xxPolicyLabel = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabel);
xxPolicyLabel = daoMgr.getXXPolicyLabels().create(xxPolicyLabel);
}
- }
- if (xxPolicyLabel != null) {
- XXPolicyLabelMap xxPolicyLabelMap = new XXPolicyLabelMap();
- xxPolicyLabelMap.setPolicyId(xPolicy.getId());
- xxPolicyLabelMap.setPolicyLabelId(xxPolicyLabel.getId());
- xxPolicyLabelMap = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
- daoMgr.getXXPolicyLabelMap().create(xxPolicyLabelMap);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("<== AssociatePolicyLabel.getOrCreateLabel(policyId=" + xPolicy.getId() + ", label=" + policyLabel + ")");
+ // doing a find to check if the label is already associated with the policy (may happen in concurrent sessions)
+ List<XXPolicyLabelMap> xxPolicyLabelMapList = daoMgr.getXXPolicyLabelMap().findByPolicyIdAndLabelId(xPolicy.getId(), xxPolicyLabel.getId());
+
+ if (xxPolicyLabelMapList != null && !xxPolicyLabelMapList.isEmpty()) {
+ LOG.info("Policy with id {} already linked to label with id = {}", xPolicy.getId(), xxPolicyLabel.getId());
+ } else {
+ XXPolicyLabelMap xxPolicyLabelMap = new XXPolicyLabelMap();
+
+ xxPolicyLabelMap.setPolicyId(xPolicy.getId());
+ xxPolicyLabelMap.setPolicyLabelId(xxPolicyLabel.getId());
+
+ xxPolicyLabelMap = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
+
+ LOG.debug("Creating a link for policy Id = {} to labelId = {}", xPolicy.getId(), xxPolicyLabel.getId());
+ daoMgr.getXXPolicyLabelMap().create(xxPolicyLabelMap);
+ }
+ } else {
+ LOG.info("Policy with id = {} does not exist, skipping to link label to the policy", xPolicy.getId());
}
+
+ LOG.debug("<== AssociatePolicyLabel.getOrCreateLabel(policyId={}, label={})", xPolicy.getId(), policyLabel);
}
}
diff --git a/security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java b/security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
index bbf41e2a2..f5d338d68 100644
--- a/security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
+++ b/security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java
@@ -173,14 +173,15 @@ private void runRunnables(final List<Runnable> runnables, final boolean isParent
LOG.debug("Executing {" + runnables.size() + "} runnables");
}
for (Runnable runnable : runnables) {
- boolean isThisTransactionCommitted = false;
+ boolean isThisTransactionCommitted;
do {
+ Object result = null;
try {
- //Create new transaction
+ //Create new transaction
TransactionTemplate txTemplate = new TransactionTemplate(txManager);
txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
- Object result = txTemplate.execute(new TransactionCallback<Object>() {
+ result = txTemplate.execute(new TransactionCallback<Object>() {
public Object doInTransaction(TransactionStatus status) {
Object result = null;
if (LOG.isDebugEnabled()) {
@@ -194,7 +195,7 @@ public Object doInTransaction(TransactionStatus status) {
}
} catch (OptimisticLockException optimisticLockException) {
if (LOG.isDebugEnabled()) {
- LOG.debug("Failed to execute runnable " + runnable + "because of OpmimisticLockException");
+ LOG.debug("Failed to execute runnable " + runnable + "because of OptimisticLockException");
}
} catch (Throwable e) {
if (LOG.isDebugEnabled()) {
@@ -204,18 +205,6 @@ public Object doInTransaction(TransactionStatus status) {
return result;
}
});
-
- isThisTransactionCommitted = result == runnable;
- if (isParentTransactionCommitted) {
- if (!isThisTransactionCommitted) {
- LOG.info("Failed to commit runnable:[" + runnable + "]. Will retry!");
- } else {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Committed runnable:[" + runnable + "].");
- }
- }
- }
-
} catch (OptimisticLockException optimisticLockException) {
if (LOG.isDebugEnabled()) {
LOG.debug("Failed to commit TransactionService transaction for runnable:[" + runnable + "]");
@@ -229,6 +218,17 @@ public Object doInTransaction(TransactionStatus status) {
LOG.debug("Failed to commit TransactionService transaction, throwable:[" + e + "]");
}
}
+
+ isThisTransactionCommitted = result == runnable;
+ if (isParentTransactionCommitted) {
+ if (!isThisTransactionCommitted) {
+ LOG.info("Failed to commit runnable:[" + runnable + "]. Will retry!");
+ } else {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Committed runnable:[" + runnable + "].");
+ }
+ }
+ }
} while (isParentTransactionCommitted && !isThisTransactionCommitted);
}
} else {
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyLabelMapDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyLabelMapDao.java
index 942cd1117..c8854a611 100644
--- a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyLabelMapDao.java
+++ b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyLabelMapDao.java
@@ -45,18 +45,32 @@ public List<XXPolicyLabelMap> findByPolicyId(Long policyId) {
}
}
- public XXPolicyLabelMap findByPolicyLabelId(Long policyLabelId) {
+ public List<XXPolicyLabelMap> findByPolicyLabelId(Long policyLabelId) {
if (policyLabelId == null) {
return null;
}
try {
- return (XXPolicyLabelMap) getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyLabelId", tClass)
+ return getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyLabelId", tClass)
.setParameter("policyLabelId", policyLabelId).getResultList();
} catch (NoResultException e) {
return null;
}
}
+ public List<XXPolicyLabelMap> findByPolicyIdAndLabelId(Long policyId, Long policyLabelId) {
+ if (policyId == null || policyLabelId == null) {
+ return null;
+ }
+
+ try {
+ return getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyIdAndLabelId", tClass)
+ .setParameter("policyId", policyId)
+ .setParameter("policyLabelId", policyLabelId).getResultList();
+ } catch (NoResultException e) {
+ return null;
+ }
+ }
+
public List<XXPolicyLabelMap> findByServiceId(Long serviceId) {
if (serviceId == null) {
return null;
diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
index f6253da03..6c2f40ef7 100755
--- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
+++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
@@ -111,7 +111,7 @@
</named-query>
<named-query name="XXPolicyLabelMap.findByPolicyLabelId">
- <query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.id = :policyLabelId
+ <query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.policyLabelId = :policyLabelId
</query>
</named-query>
@@ -120,7 +120,10 @@
order by obj.policyId, obj.id
</query>
</named-query>
-
+ <named-query name="XXPolicyLabelMap.findByPolicyIdAndLabelId">
+ <query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.policyLabelId = :policyLabelId AND obj.policyId = :policyId
+ </query>
+ </named-query>
<!-- XXPortalUserRole -->
<named-query name="XXPortalUserRole.findByRoleUserId">