This is an automated email from the ASF dual-hosted git repository.
abhi 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 e93c017c6 RANGER-5134: Fix policy label processing in concurrent
sessions (#545)
e93c017c6 is described below
commit e93c017c68376f8f8b815de2ba1ce4708f666e26
Author: Abhishek Kumar <[email protected]>
AuthorDate: Fri Mar 21 17:02:25 2025 -0700
RANGER-5134: Fix policy label processing in concurrent sessions (#545)
---
.../java/org/apache/ranger/biz/ServiceDBStore.java | 34 +++++++++++++++-------
.../RangerTransactionSynchronizationAdapter.java | 32 ++++++++++----------
.../org/apache/ranger/db/XXPolicyLabelMapDao.java | 18 ++++++++++--
.../main/resources/META-INF/jpa_named_queries.xml | 7 +++--
4 files changed, 60 insertions(+), 31 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 089f06c37..4f81e6b15 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
@@ -6741,15 +6741,20 @@ public void run() {
getOrCreateLabel();
}
+ private boolean doesPolicyExist(XXPolicy xPolicy) {
+ return daoMgr.getXXPolicy().getById(xPolicy.getId()) != null;
+ }
+
private void getOrCreateLabel() {
LOG.debug("==> AssociatePolicyLabel.getOrCreateLabel(policyId={},
label={})", xPolicy.getId(), policyLabel);
- XXPolicyLabel xxPolicyLabel =
daoMgr.getXXPolicyLabels().findByName(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);
@@ -6757,17 +6762,24 @@ private void getOrCreateLabel() {
xxPolicyLabel =
rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabel);
xxPolicyLabel =
daoMgr.getXXPolicyLabels().create(xxPolicyLabel);
}
- }
- if (xxPolicyLabel != null) {
- XXPolicyLabelMap xxPolicyLabelMap = new XXPolicyLabelMap();
+ // 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.setPolicyId(xPolicy.getId());
+ xxPolicyLabelMap.setPolicyLabelId(xxPolicyLabel.getId());
- xxPolicyLabelMap =
rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
+ xxPolicyLabelMap =
rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
- daoMgr.getXXPolicyLabelMap().create(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 dfb922784..c59b4f9e0 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
@@ -155,16 +155,17 @@ private void runRunnables(final List<Runnable> runnables,
final boolean isParent
LOG.debug("Executing {{}} runnables", runnables.size());
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(status -> {
+ result = txTemplate.execute(status -> {
Object result1 = null;
LOG.debug("Executing runnable {{}}", runnable);
@@ -176,23 +177,13 @@ private void runRunnables(final List<Runnable> runnables,
final boolean isParent
LOG.debug("executed runnable {}", runnable);
} catch (OptimisticLockException
optimisticLockException) {
- LOG.debug("Failed to execute runnable
{}because of OpmimisticLockException", runnable);
+ LOG.debug("Failed to execute runnable {}
because of OptimisticLockException", runnable);
} catch (Throwable e) {
LOG.debug("Failed to execute runnable {}",
runnable, e);
}
return result1;
});
-
- isThisTransactionCommitted = result == runnable;
-
- if (isParentTransactionCommitted) {
- if (!isThisTransactionCommitted) {
- LOG.info("Failed to commit runnable:[{}]. Will
retry!", runnable);
- } else {
- LOG.debug("Committed runnable:[{}].",
runnable);
- }
- }
} catch (OptimisticLockException optimisticLockException) {
if (LOG.isDebugEnabled()) {
LOG.debug("Failed to commit TransactionService
transaction for runnable:[{}]", runnable);
@@ -206,8 +197,17 @@ private void runRunnables(final List<Runnable> runnables,
final boolean isParent
LOG.debug("Failed to commit TransactionService
transaction, throwable:[{}]", String.valueOf(e));
}
}
- }
- while (isParentTransactionCommitted &&
!isThisTransactionCommitted);
+
+ isThisTransactionCommitted = result == runnable;
+
+ if (isParentTransactionCommitted) {
+ if (!isThisTransactionCommitted) {
+ LOG.info("Failed to commit runnable:[{}]. Will
retry!", runnable);
+ } else {
+ LOG.debug("Committed runnable:[{}].", runnable);
+ }
+ }
+ } while (isParentTransactionCommitted &&
!isThisTransactionCommitted);
}
} else {
LOG.debug("No runnables to execute");
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 9ac6d6c2e..6c8ab022d 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,13 +45,27 @@ 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;
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 56eefba40..05ba59f28 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">
