yuqi1129 opened a new pull request, #12174: URL: https://github.com/apache/gravitino/pull/12174
### What changes were proposed in this pull request? Privilege probes on the JCasbin authorization hot path no longer call `enforcer.enforce`. They resolve against a per-role index instead: - `loadedRoles` now caches `roleId → CachedRolePolicies` (a `role_meta.updated_at` version sentinel plus a `Map<PolicyKey, Effect>` index), built in `loadPolicyByRoleEntity`. - `authorizeByJcasbin` / the new `resolveRoleEffect`, the active-role narrowing path, and `hasDenyPolicy` all resolve via index lookups over the roles the caller holds (`indexEffect`). - The enforcer keeps only the user/group→role grouping graph (`g` rows); it no longer stores privilege (`p`) policies. The now-redundant second (`deny`) enforcer, `clearRolePolicies`, and the removal listener in `JcasbinLoadedRolesCache` are removed. - New types: `PolicyKey`, `Effect`, `CachedRolePolicies`. Semantics preserved: DENY wins within a role and globally across the caller's roles; active-role narrowing (role assumption) still narrows allows while deny stays global; `role_meta.updated_at` version validation is unchanged; OWNER still short-circuits to the owner cache. #### Why `JcasbinLoadedRolesCache` no longer needs a removal listener Policy ownership changes in this PR: | | Before this PR | After this PR | |---|---|---| | `loadedRoles` value | `roleId → updatedAt` | `roleId → CachedRolePolicies(updatedAt, policyIndex)` | | Privilege-policy storage | JCasbin enforcer `p` rows | `CachedRolePolicies.index` | | Enforcer responsibility | `g` bindings and `p` policies | `g(user/group, role)` bindings only | | Work on cache eviction | Delete the separate `p` rows | None; the index is removed with the cache entry | Previously, evicting the version-only `loadedRoles` entry left a second copy of the role's policies in the enforcers. The removal listener therefore had to clear those `p(roleId, ...)` rows. The recently merged fix correctly changed that cleanup from `deleteRole`, which also removes independently managed `g` bindings, to policy-only cleanup. This PR removes enforcer `p` rows entirely. The complete privilege index and its version sentinel now live in the same `CachedRolePolicies` cache value, so eviction atomically discards the policy data itself. On the next request, `versionCheckAndLoadRoles` observes the cache miss, reloads the role from the database, and rebuilds the index. There is no second policy copy for a listener to clean. The enforcer's remaining `g` bindings have a separate lifecycle managed by the user/group role caches. Deleting them when a policy-index entry expires would be incorrect. A residual `g` binding cannot grant access without a corresponding `loadedRoles` index: a missing index resolves to no privilege and is reloaded or rejected by version validation before authorization. ### Why are the changes needed? In jcasbin 1.99.0, `enforce` linearly scans **every** policy line loaded in the enforcer and evaluates the aviator matcher per line — `O(total_policies)` per probe — and the enforcer accumulates policies for up to `roleCacheSize` (default 10000) roles across all metalakes on a node. A benchmark against the real `jcasbin_model.conf` (one user, one role, one object) shows the scan is over all policies regardless of the user's bindings: | roles | p-rules | `enforce` avg | |---|---|---| | 1,000 | 60,000 | ~12.8 ms | | 10,000 | 600,000 | ~106 ms | The index makes each probe `O(roles_per_user)` hash lookups (sub-microsecond, flat in total policy count). One `authorize` triggers many probes (schema chain, per-object list checks, allow+deny, per-active-role narrowing), so the saving compounds. Fix: #12173 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? `./gradlew :server-common:test --tests "org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer" -PskipITs` — 57 tests pass. - `testPrivilegeProbesUseIndexNotEnforcerPolicies` asserts that the index carries the granted privilege while the enforcer contains no `p` policies. - `testRoleCacheInvalidationPreservesUserRoleBindings` asserts that invalidating the policy-index entry removes it without deleting the enforcer's independently managed `g` binding. - The deleted-role/version-miss test asserts that a residual `g` binding grants nothing after its `loadedRoles` index is evicted. ### Optimization approach and scope Authorization no longer calls JCasbin enforce for privilege probes. allowEnforcer retains only the user/group-to-role (g) relationship graph. Privilege policies are stored in a per-role PolicyKey-to-Effect index in loadedRoles. A request first resolves the roles held by the user, then performs hash lookups only in those roles' indexes; DENY takes precedence globally, while active-role narrowing applies to ALLOW decisions. This primarily addresses nodes with many cached roles and policies, where the old JCasbin path scanned all loaded p policy lines for every probe. It does not eliminate the cost of a single very large role: that role's complete policy index still needs to be loaded and retained. Likewise, a role shared by many users still has one shared index and the user-role (g) relationship remains a separate scale consideration. These cases are outside this PR's scope. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
