yuqi1129 opened a new pull request, #11174:
URL: https://github.com/apache/gravitino/pull/11174
### What changes were proposed in this pull request?
> **Note**: This PR is stacked on top of #10996
(`feat/jcasbin-cache-refactor`). Only the last two commits (`039bdd7d3` +
`232053523`) are new in this PR; everything earlier in the diff comes from
#10996 and should be reviewed there.
Two related changes:
**Commit 1 — batch user + group version probes (`039bdd7d3`)**
On the `authorize()` hot path with all caches warm, the per-request version
probes for the current user + each of the user's groups ran as `1 + N`
separate `getUserUpdatedAt` / `getGroupUpdatedAt` SELECTs. Collapse them
into a single `UNION ALL` query:
- New `AuthSubjectVersion` POJO carrying one row of the UNION result.
- New `UserMetaMapper.batchGetUserAndGroupUpdatedAt` mapper method, one
base SQL provider, no per-backend override; empty `groupNames`
degrades to a user-only SELECT so we avoid an empty IN clause across
H2 / MySQL / PostgreSQL.
- New `JcasbinAuthorizer.prefetchUserAndGroupInfo` runs the batch once
at the top of `loadPrivilegeAndAuthorize` and primes both the
per-request `userInfoCache` and `groupInfoCache` via
`computeXxxIfAbsent`. Subsequent `loadUserInfo` / `loadGroupInfo`
calls in the same request hit the per-request cache (0 DB queries).
For N groups, hot path drops from `N+2` queries to `2`.
**Commit 2 — route `isSelf(ROLE)` through the cache (`232053523`, fixes
#11088)**
`isSelf(ROLE)` was the last call site bypassing the version-validated
`userRoleCache` / `groupRoleCache` and going straight to
`entityStore.relationOperations().listEntitiesByRelation(ROLE_USER_REL)`
plus a batchGet over the user's groups. Switch the ROLE branch to:
1. `loadUserInfo` for `user_id` + `updated_at`,
2. `loadUserRoles` for the direct role list (cached, version-validated),
3. `loadGroupRoles` per group from the principal (same cache path).
Repeated `isSelf(ROLE)` calls in the same request — and `isSelf` following
`authorize()` — now resolve the role list from the process-wide cache
instead of re-querying `user_role_rel` / `group_meta`.
### Why are the changes needed?
`isSelf(ROLE)` is invoked from `AuthorizationExpressionEvaluator` for
every authorization expression that references role identity, so the
redundant role-list fetch landed on every such request. Issue #11088
called out the inconsistency with `authorize()` / `isOwner()`, which
were already using the cache. The batch-probe optimization in commit 1
is the prerequisite that makes the per-request user + group fan-out
cheap enough to also serve repeated `isSelf` calls without measurable
overhead.
`isSelf` does not take an `AuthorizationRequestContext`, so the new
implementation allocates a fresh per-call context. This scopes only the
lightweight version probes (`getUserUpdatedAt` / `getGroupUpdatedAt`);
the role-list queries the issue calls out are still deduplicated across
all calls by the shared cache, which is what the acceptance criterion
asks for. Threading the context through would ripple into the
`GravitinoAuthorizer` interface, `PassThroughAuthorizer`, and the
`AuthorizationExpressionConverter` template; left as a follow-up.
Fix: #11088
### 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.*'` — 39/39 pass (9 lookup +
23 authorizer + 2 cacheHelpers + 5 poller).
- New `testIsSelfRoleReusesCacheAcrossCalls` asserts that two `isSelf(ROLE)`
calls in a row issue exactly one `listRolesByUserId` (the #11088 AC).
- New `testIsSelfRoleDoesNotCallListEntitiesByRelation` asserts the old
EntityStore bypass is gone.
- New `TestUserMetaService.batchGetUserAndGroupUpdatedAt` IT covers
user-only, user+groups, partial-missing, and all-missing scenarios on H2
(10/10). The MySQL/PG branches of the parameterized test could not be exercised
in my local environment due to testcontainers setup issues; the SQL mirrors the
existing `batchGetRoleUpdatedAt` pattern that already works across all three
backends in CI.
--
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]
