RANGER-3899: Policy creation/updation takes more time when there are more users,groups,roles

[ramackri]

What changes were proposed in this pull request?

Optimized PolicyRefUpdater and related DAOs so policy create/update performs fewer database round-trips when a policy references many users, groups, and roles.

Problem

When creating or updating a policy with a large number of principals (users, groups, roles), Ranger Admin was slow because:

1. N+1 lookups — Each user, group, role, resource, access type, condition, and data mask type was resolved with a separate DB query (findByUserName, findByGroupName, findByRoleName, etc.).
2. Full ref-table rebuild on update — On policy update, cleanupRefTables() deleted all rows from x_policy_ref_user, x_policy_ref_group, and x_policy_ref_role (and other ref tables), then re-inserted every principal even when nothing changed.

Changes

1. Batch lookups via named queries — Added IN (:names) JPA named queries and DAO helpers to resolve principals and definitions in bulk:

   • XXUser.getIdsByUserNames
   • XXGroup.getIdsByGroupNames
   • XXRole.getIdsByRoleNames
   • XXResourceDef.findByNamesAndPolicyId
   • XXAccessTypeDef.findByNamesAndServiceId
   • XXPolicyConditionDef.findByServiceDefIdAndNames
   • XXDataMaskTypeDef.findByNamesAndServiceId

2. Pre-resolved principal IDs — PolicyPrincipalAssociator now accepts a pre-fetched principal ID from the bulk map, avoiding per-name lookups when the principal already exists.

3. Selective cleanup on policy update — Introduced cleanupRefTablesForUpdate() that:

   • Loads existing user/role/group ref rows for the policy
   • Deletes only principals removed from the policy
   • Skips re-insert for principals that are unchanged
   • Still performs full delete/rebuild for resources, access types, conditions, and data mask types (typically far fewer entries)

4. Unified create/update flow — createNewPolMappingForRefTable() takes a new isCleanupRefTablesNeeded flag:

   • Create: false (no prior ref rows)
   • Update: true (runs selective cleanup before insert)
   • ServiceDBStore no longer calls cleanupRefTables() separately before mapping

5. Batch insert helper — Added batchInsert() with bulk-mode handling and debug timing logs around dao.batchCreate().

Files changed (15)

Area	Files
Core logic	PolicyRefUpdater.java, ServiceDBStore.java
DAOs	XXUserDao, XXGroupDao, XXRoleDao, XXResourceDefDao, XXAccessTypeDefDao, XXPolicyConditionDefDao, XXDataMaskTypeDefDao, XXPolicyRefUserDao, XXPolicyRefGroupDao, XXPolicyRefRoleDao
Queries	jpa_named_queries.xml
Tests	TestPolicyRefUpdater.java, TestServiceDBStore.java

---

How was this patch tested?

Unit tests

• Updated TestPolicyRefUpdater for the new bulk-lookup APIs and isCleanupRefTablesNeeded parameter
• Added testCleanupRefTablesForUpdate_SelectivePrincipalCleanup to verify selective delete/insert for users, roles, and groups on update
• Updated TestServiceDBStore for the new createNewPolMappingForRefTable signature

mvn -pl security-admin test -Dtest=TestPolicyRefUpdater,TestServiceDBStore

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[mneethiraj]

The caller of findXXResourceDefsByNameAndPolicyId() only needs resourceId (XXResourceDef.id) for each resource name. Retrieving only id from the database will help improve the performance.

1. I suggest updating & renaming the method from:

public Map<String, XXResourceDef> findXXResourceDefsByNameAndPolicyId(Set<String> names, Long policyId)

to:

public Map<String, Long> findResourceDefIdsByNameAndPolicyId(Set<String> names, Long policyId)

2. Update the named query XXResourceDef.findByNamesAndPolicyId to return id instead of XXResourceDef.

3. Review similar updates to following named queries and corresponding Dao methods that use them:

   • XXAccessTypeDef.findByNamesAndServiceId
   • XXPolicyConditionDef.findByServiceDefIdAndNames
   • XXDataMaskTypeDef.findByNamesAndServiceId

[ramackri]

Done

[mneethiraj]

Consider having a single return statement in a method.

[ramackri]

Done

[mneethiraj]

To be consistent, consider using streams to convert results ito userNameIdMap - similar to XXRoleDao.getIdsByRoleNames().

[ramackri]

Done

[mneethiraj]

To be consistent, consider using streams to convert results ito roleNameIdMap - similar to XXRoleDao.getIdsByRoleNames().

[ramackri]

Done

[mneethiraj]

To be consistent, consider using streams to convert results ito groupNameIdMap - similar to XXRoleDao.getIdsByRoleNames().

[ramackri]

Done

[mneethiraj]

Instead of having PolicyPrincipalAssociator take 3 collection parameters and populating one of them in doAssociate() call, consider:

• creating 3 classes PolicyRoleAssociator, PolicyGroupAssociator and PolicyUserAssociator
• add method getPolicyRef() to return XXPolicyRefRole/XXPolicyRefGroup/XXPolicyRefUser when the policy exists
• add method createPolicyRef() to create XXPolicyRefRole/XXPolicyRefGroup/XXPolicyRefUser when the policy exists

Long                 roleId     = roleNameIdMap.get(roleName);
PolicyRoleAssociator associator = new PolicyRoleAssociator(roleName, roleId, xPolicy);

if (roleId != null) {
  XXPolicyRoleRef roleRef = associator.getPolicyRef();

  if (roleRef != null) {
    xPolRoles.add(roleRef);
  }
} else if (createPrincipalsIfAbsent) {
  rangerTransactionSynchronizationAdapter.executeOnTransactionCommit(associator);
} else {
  VXResponse gjResponse = new VXResponse();
  ...
  throw restErrorUtil.generateRESTException(gjResponse);
}

Review handling of groups and users references as well - line 243 and 268.

[ramackri]

Done

[mneethiraj]

Consider moving lines 386, 387 and 388 into cleanupPolicyRefUsers(), cleanupPolicyRefRoles() and cleanupPolicyRefGroups() respectively - to improve readability.

[ramackri]

Done