bharos opened a new pull request, #12171: URL: https://github.com/apache/gravitino/pull/12171
### What changes were proposed in this pull request? Draft for discussion of the fix direction for #12170. `RelationalEntityStore.batchListEntitiesByRelation` runs the backend batch-list DB round-trip **inside** `cache.withMultipleKeyCacheLock(...)`, which acquires all the cache segment locks for the batch's keys and holds them across the DB call. Under a mixed list+create workload this serializes the large multi-key owner preload (list authorization, `preloadOwner` → `batchListEntitiesByRelation(OWNER_REL, ...)`) against every concurrent owner-relation invalidation (`createTable` → `insertRelation` → `CaffeineEntityCache.invalidate` → `SegmentedLock.withLock`), stalling writers for the full DB latency and collapsing create throughput. This PR restructures the method as plain **cache-aside** that never holds a cache lock across backend I/O: 1. read the cache lock-free (`getIfPresent`) to partition cached vs uncached identifiers; 2. run `backend.batchListEntitiesByRelation(...)` for the uncached identifiers with **no** cache lock held; 3. populate via `cache.put` (which takes its own brief per-key `SegmentedLock`). Caffeine is already thread-safe, so no lock needs to span the DB call. The outer `withMultipleKeyCacheLock` wrapper and the now-unused `EntityCacheKey` import are removed. ### Why are the changes needed? Fix: #12170 Any deployment with authorization + the entity cache enabled can see create/alter operations stall (seconds to minutes, or hang under sustained lists) whenever large `listTables` requests run concurrently, because the list's owner preload holds a near-global cache lock across a DB call that create's cache invalidation must wait on. ### Does this PR introduce _any_ user-facing change? No. Behavior is unchanged; concurrent create/alter operations are no longer blocked behind a list's DB round-trip. This trades the previous whole-batch atomicity for standard cache-aside semantics (a bounded owner-cache staleness window, healed by TTL/invalidation, consistent with the existing cross-node eventual-consistency model). Single-key relation paths (`get`, single `listEntitiesByRelation`, `getEntityByRelation`) are unchanged and remain atomic. ### How was this patch tested? Adds `TestRelationalEntityStore.testBatchListDoesNotHoldCacheLockAcrossBackendCall`: uses a real `CaffeineEntityCache`, blocks the mocked backend mid-call on a latch, and asserts a concurrent `cache.invalidate` on the same key still completes within a short timeout — proving no cache lock is held across the backend call. Verified as a genuine regression guard: it passes with the fix and fails (times out) when the fix is reverted. Load-tested on a large schema: in a deterministic 25-lister/25-creator run, creates went from **0 completions in 5 min → ~1,450**, with no per-request regression to individual lists. > Note: a separate, residual list-latency item (the per-object DB work list authorization now performs) is out of scope here and can be tracked independently. -- 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]
