RANGER-3900: performance improvement in roles deletion

ramackri · mneethiraj · commit 017c7402773f · 2022-10-11T00:23:02.000-07:00
Signed-off-by: Madhan Neethiraj &lt;madhan@apache.org&gt;
diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java
@@ -165,17 +165,15 @@ public Boolean cleanupRefTables(RangerRole rangerRole) {
 		XXRoleRefGroupDao xRoleGroupDao = daoMgr.getXXRoleRefGroup();
 		XXRoleRefRoleDao xRoleRoleDao = daoMgr.getXXRoleRefRole();
 
-		for (XXRoleRefUser xxRoleRefUser : xRoleUserDao.findByRoleId(roleId)) {
-			xRoleUserDao.remove(xxRoleRefUser);
-		}
+		List<Long> xxRoleRefUserIds = xRoleUserDao.findIdsByRoleId(roleId);
+		xRoleUserDao.deleteRoleRefUserByIds(xxRoleRefUserIds);
 
-		for (XXRoleRefGroup xxRoleRefGroup : xRoleGroupDao.findByRoleId(roleId)) {
-			xRoleGroupDao.remove(xxRoleRefGroup);
-		}
+		List<Long> xxRoleRefGroupByIds = xRoleGroupDao.findIdsByRoleId(roleId);
+		xRoleGroupDao.deleteRoleRefGroupByIds(xxRoleRefGroupByIds);
+
+		List<Long> xxRoleRefRoleIds = xRoleRoleDao.findIdsByRoleId(roleId);
+		xRoleRoleDao.deleteRoleRefRoleByIds(xxRoleRefRoleIds);
 
-		for (XXRoleRefRole xxRoleRefRole : xRoleRoleDao.findByRoleId(roleId)) {
-			xRoleRoleDao.remove(xxRoleRefRole);
-		}
 		return true;
 	}
 
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
@@ -34,6 +34,7 @@
 import javax.persistence.Table;
 import javax.persistence.TypedQuery;
 
+import org.apache.ranger.authorization.hadoop.config.RangerAdminConfig;
 import org.apache.ranger.biz.RangerBizUtil;
 import org.apache.ranger.common.AppConstants;
 import org.apache.ranger.db.RangerDaoManager;
@@ -43,6 +44,24 @@
 
 public abstract class BaseDao<T> {
 	private static final Logger logger = LoggerFactory.getLogger(BaseDao.class);
+	private static final String PROP_BATCH_DELETE_BATCH_SIZE    = "ranger.admin.dao.batch.delete.batch.size";
+	private static final int    DEFAULT_BATCH_DELETE_BATCH_SIZE = 1000;
+	private static       int    BATCH_DELETE_BATCH_SIZE;
+
+	static {
+		try {
+			BATCH_DELETE_BATCH_SIZE = RangerAdminConfig.getInstance().getInt(PROP_BATCH_DELETE_BATCH_SIZE, DEFAULT_BATCH_DELETE_BATCH_SIZE);
+
+			if (BATCH_DELETE_BATCH_SIZE > DEFAULT_BATCH_DELETE_BATCH_SIZE) {
+				logger.warn("Configuration {}={}, which is larger than default value {}", PROP_BATCH_DELETE_BATCH_SIZE, BATCH_DELETE_BATCH_SIZE, DEFAULT_BATCH_DELETE_BATCH_SIZE);
+			}
+		} catch(Exception e) {
+			// When we get the Number format exception due to the invalid value entered into the config file.
+			BATCH_DELETE_BATCH_SIZE = DEFAULT_BATCH_DELETE_BATCH_SIZE;
+		}
+
+		logger.info(PROP_BATCH_DELETE_BATCH_SIZE + "=" + BATCH_DELETE_BATCH_SIZE);
+	}
 
 	protected RangerDaoManager daoManager;
 
@@ -111,6 +130,32 @@ public List<T> batchCreate(List<T> obj) {
 		return ret;
 	}
 
+	public void batchDeleteByIds(String namedQuery, List<Long> ids, String paramName) {
+		if (BATCH_DELETE_BATCH_SIZE <= 0) {
+			getEntityManager()
+				.createNamedQuery(namedQuery, tClass)
+				.setParameter(paramName, ids).executeUpdate();
+		} else {
+			for (int fromIndex = 0; fromIndex < ids.size(); fromIndex += BATCH_DELETE_BATCH_SIZE) {
+				int toIndex = fromIndex + BATCH_DELETE_BATCH_SIZE;
+
+				if (toIndex > ids.size()) {
+					toIndex = ids.size();
+				}
+
+				if (logger.isDebugEnabled()) {
+					logger.debug("batchDeleteByIds({}, idCount={}): deleting fromIndex={}, toIndex={}", namedQuery, ids.size(), fromIndex, toIndex);
+				}
+
+				List<Long> subList = ids.subList(fromIndex, toIndex);
+
+				getEntityManager()
+						.createNamedQuery(namedQuery, tClass)
+						.setParameter(paramName, subList).executeUpdate();
+			}
+		}
+	}
+
 	public T update(T obj) {
 		em.merge(obj);
 		if (!RangerBizUtil.isBulkMode()) {
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefAccessTypeDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefAccessTypeDao.java
@@ -114,8 +114,6 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefAccessType.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
+		batchDeleteByIds("XXPolicyRefAccessType.deleteByIds", ids, "ids");
 	}
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefConditionDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefConditionDao.java
@@ -125,8 +125,6 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefCondition.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
+		batchDeleteByIds("XXPolicyRefCondition.deleteByIds", ids, "ids");
 	}
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefDataMaskTypeDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefDataMaskTypeDao.java
@@ -101,8 +101,6 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefDataMaskType.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
+		batchDeleteByIds("XXPolicyRefDataMaskType.deleteByIds", ids, "ids");
 	}
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefGroupDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefGroupDao.java
@@ -113,8 +113,6 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefGroup.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
+		batchDeleteByIds("XXPolicyRefGroup.deleteByIds", ids, "ids");
 	}
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefResourceDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefResourceDao.java
@@ -79,10 +79,7 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefResource.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
-
+		batchDeleteByIds("XXPolicyRefResource.deleteByIds", ids, "ids");
 	}
 
 	 @SuppressWarnings("unchecked")
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefRoleDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefRoleDao.java
@@ -125,9 +125,7 @@ public void deleteByPolicyId(Long policyId) {
             return;
         }
 
-        getEntityManager()
-                .createNamedQuery("XXPolicyRefRole.deleteByIds", tClass)
-                .setParameter("ids", ids).executeUpdate();
+        batchDeleteByIds("XXPolicyRefRole.deleteByIds", ids, "ids");
 	}
 }
 
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefUserDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyRefUserDao.java
@@ -125,8 +125,6 @@ public void deleteByPolicyId(Long policyId) {
 			return;
 		}
 
-		getEntityManager()
-				.createNamedQuery("XXPolicyRefUser.deleteByIds", tClass)
-				.setParameter("ids", ids).executeUpdate();
+		batchDeleteByIds("XXPolicyRefUser.deleteByIds", ids, "ids");
 	}
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefGroupDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefGroupDao.java
@@ -24,6 +24,7 @@
 
 import javax.persistence.NoResultException;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.ranger.common.db.BaseDao;
 import org.apache.ranger.entity.XXRoleRefGroup;
 import org.springframework.stereotype.Service;
@@ -61,6 +62,22 @@ public List<XXRoleRefGroup> findByGroupId(Long groupId) {
         }
     }
 
+    public List<Long> findIdsByRoleId(Long roleId) {
+        List<Long> ret = Collections.EMPTY_LIST;
+
+        if (roleId != null) {
+            try {
+                ret = getEntityManager()
+                        .createNamedQuery("XXRoleRefGroup.findIdsByRoleId", Long.class)
+                        .setParameter("roleId", roleId).getResultList();
+            } catch (NoResultException e) {
+                ret = Collections.EMPTY_LIST;
+            }
+        }
+
+        return ret;
+    }
+
     public List<XXRoleRefGroup> findByGroupName(String groupName) {
         if (groupName == null) {
             return Collections.EMPTY_LIST;
@@ -72,4 +89,11 @@ public List<XXRoleRefGroup> findByGroupName(String groupName) {
             return Collections.EMPTY_LIST;
         }
     }
+
+    public void deleteRoleRefGroupByIds(List<Long> ids) {
+        if (CollectionUtils.isNotEmpty(ids)) {
+            batchDeleteByIds("XXRoleRefGroup.deleteRoleRefGroupByIds", ids, "ids");
+        }
+    }
+
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefRoleDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefRoleDao.java
@@ -51,6 +51,22 @@ public List<XXRoleRefRole> findByRoleId(Long roleId) {
         }
     }
 
+    public List<Long> findIdsByRoleId(Long roleId) {
+        List<Long> ret = Collections.EMPTY_LIST;
+
+        if (roleId != null) {
+            try {
+                ret = getEntityManager()
+                        .createNamedQuery("XXRoleRefRole.findIdsByRoleId", Long.class)
+                        .setParameter("roleId", roleId).getResultList();
+            } catch (NoResultException e) {
+                ret = Collections.EMPTY_LIST;
+            }
+        }
+
+        return ret;
+    }
+
     public List<XXRoleRefRole> findBySubRoleId(Long subRoleId) {
         if(subRoleId == null) {
             return Collections.EMPTY_LIST;
@@ -104,4 +120,10 @@ public Set<Long> getContainingRoles(Long subRoleId) {
 
         return ret;
     }
+
+    public void deleteRoleRefRoleByIds(List<Long> ids) {
+        if (CollectionUtils.isNotEmpty(ids)) {
+            batchDeleteByIds("XXRoleRefRole.deleteRoleRefRoleByIds", ids, "ids");
+        }
+    }
 }
diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefUserDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXRoleRefUserDao.java
@@ -24,6 +24,7 @@
 
 import javax.persistence.NoResultException;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.ranger.common.db.BaseDao;
 import org.apache.ranger.entity.XXRoleRefUser;
 import org.springframework.stereotype.Service;
@@ -48,6 +49,22 @@ public List<XXRoleRefUser> findByRoleId(Long roleId) {
         }
     }
 
+    public List<Long> findIdsByRoleId(Long roleId) {
+        List<Long> ret = Collections.EMPTY_LIST;
+
+        if (roleId != null) {
+            try {
+                ret = getEntityManager()
+                        .createNamedQuery("XXRoleRefUser.findIdsByRoleId", Long.class)
+                        .setParameter("roleId", roleId).getResultList();
+            } catch (NoResultException e) {
+                ret = Collections.EMPTY_LIST;
+            }
+        }
+
+        return ret;
+    }
+
     public List<XXRoleRefUser> findByUserId(Long userId) {
         if(userId == null) {
             return Collections.EMPTY_LIST;
@@ -72,4 +89,10 @@ public List<XXRoleRefUser> findByUserName(String userName) {
             return Collections.EMPTY_LIST;
         }
     }
+
+    public void deleteRoleRefUserByIds(List<Long> ids) {
+        if (CollectionUtils.isNotEmpty(ids)) {
+            batchDeleteByIds("XXRoleRefUser.deleteRoleRefUserByIds", ids, "ids");
+        }
+    }
 }
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
@@ -1767,6 +1767,10 @@
         <query>select obj from XXRoleRefUser obj  where obj.roleId = :roleId</query>
     </named-query>
 
+		<named-query name="XXRoleRefUser.findIdsByRoleId">
+				<query>select obj.id from XXRoleRefUser obj  where obj.roleId = :roleId</query>
+		</named-query>
+
     <named-query name="XXRoleRefUser.findByUserId">
         <query>select obj from XXRoleRefUser obj where obj.userId = :userId</query>
     </named-query>
@@ -1779,6 +1783,10 @@
         <query>select obj from XXRoleRefGroup obj  where obj.roleId = :roleId</query>
     </named-query>
 
+		<named-query name="XXRoleRefGroup.findIdsByRoleId">
+			<query>select obj.id from XXRoleRefGroup obj  where obj.roleId = :roleId</query>
+		</named-query>
+
     <named-query name="XXRoleRefGroup.findByGroupId">
         <query>select obj from XXRoleRefGroup obj  where obj.groupId = :groupId</query>
     </named-query>
@@ -1791,6 +1799,10 @@
         <query>select obj from XXRoleRefRole obj  where obj.roleId = :roleId</query>
     </named-query>
 
+		<named-query name="XXRoleRefRole.findIdsByRoleId">
+				<query>select obj.id from XXRoleRefRole obj  where obj.roleId = :roleId</query>
+		</named-query>
+
     <named-query name="XXRoleRefRole.findBySubRoleId">
         <query>select obj from XXRoleRefRole obj  where obj.subRoleId = :subRoleId</query>
     </named-query>
@@ -1803,6 +1815,16 @@
         <query>select count(obj.roleId) from XXRoleRefRole obj where obj.subRoleName = :subRoleName </query>
     </named-query>
 
+	<named-query name="XXRoleRefGroup.deleteRoleRefGroupByIds">
+				<query>DELETE FROM XXRoleRefGroup obj WHERE obj.id IN :ids </query>
+	</named-query>
+	<named-query name="XXRoleRefRole.deleteRoleRefRoleByIds">
+				<query>DELETE FROM XXRoleRefRole obj WHERE obj.id IN :ids </query>
+	</named-query>
+	<named-query name="XXRoleRefUser.deleteRoleRefUserByIds">
+				<query>DELETE FROM XXRoleRefUser obj WHERE obj.id IN :ids </query>
+	</named-query>
+
 
 	<!-- XXPolicyRefRole -->
 	<named-query name="XXPolicyRefRole.findByPolicyId">

Original file line number	Diff line number	Diff line change
`@@ -114,8 +114,6 @@ public void deleteByPolicyId(Long policyId) {`
`114`	`114`	`return;`
`115`	`115`	`}`
`116`	`116`
`117`		`- getEntityManager()`
`118`		`- .createNamedQuery("XXPolicyRefAccessType.deleteByIds", tClass)`
`119`		`- .setParameter("ids", ids).executeUpdate();`
	`117`	`+ batchDeleteByIds("XXPolicyRefAccessType.deleteByIds", ids, "ids");`
`120`	`118`	`}`
`121`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -125,8 +125,6 @@ public void deleteByPolicyId(Long policyId) {`
`125`	`125`	`return;`
`126`	`126`	`}`
`127`	`127`
`128`		`- getEntityManager()`
`129`		`- .createNamedQuery("XXPolicyRefCondition.deleteByIds", tClass)`
`130`		`- .setParameter("ids", ids).executeUpdate();`
	`128`	`+ batchDeleteByIds("XXPolicyRefCondition.deleteByIds", ids, "ids");`
`131`	`129`	`}`
`132`	`130`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,8 +101,6 @@ public void deleteByPolicyId(Long policyId) {`
`101`	`101`	`return;`
`102`	`102`	`}`
`103`	`103`
`104`		`- getEntityManager()`
`105`		`- .createNamedQuery("XXPolicyRefDataMaskType.deleteByIds", tClass)`
`106`		`- .setParameter("ids", ids).executeUpdate();`
	`104`	`+ batchDeleteByIds("XXPolicyRefDataMaskType.deleteByIds", ids, "ids");`
`107`	`105`	`}`
`108`	`106`	`}`
Original file line number	Diff line number	Diff line change
`@@ -113,8 +113,6 @@ public void deleteByPolicyId(Long policyId) {`
`113`	`113`	`return;`
`114`	`114`	`}`
`115`	`115`
`116`		`- getEntityManager()`
`117`		`- .createNamedQuery("XXPolicyRefGroup.deleteByIds", tClass)`
`118`		`- .setParameter("ids", ids).executeUpdate();`
	`116`	`+ batchDeleteByIds("XXPolicyRefGroup.deleteByIds", ids, "ids");`
`119`	`117`	`}`
`120`	`118`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,7 @@ public void deleteByPolicyId(Long policyId) {`
`79`	`79`	`return;`
`80`	`80`	`}`
`81`	`81`
`82`		`- getEntityManager()`
`83`		`- .createNamedQuery("XXPolicyRefResource.deleteByIds", tClass)`
`84`		`- .setParameter("ids", ids).executeUpdate();`
`85`		`-`
	`82`	`+ batchDeleteByIds("XXPolicyRefResource.deleteByIds", ids, "ids");`
`86`	`83`	`}`
`87`	`84`
`88`	`85`	`@SuppressWarnings("unchecked")`
Original file line number	Diff line number	Diff line change
`@@ -125,9 +125,7 @@ public void deleteByPolicyId(Long policyId) {`
`125`	`125`	`return;`
`126`	`126`	`}`
`127`	`127`
`128`		`- getEntityManager()`
`129`		`- .createNamedQuery("XXPolicyRefRole.deleteByIds", tClass)`
`130`		`- .setParameter("ids", ids).executeUpdate();`
	`128`	`+ batchDeleteByIds("XXPolicyRefRole.deleteByIds", ids, "ids");`
`131`	`129`	`}`
`132`	`130`	`}`
`133`	`131`