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 <[email protected]>
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">
