dennismdejong opened a new pull request, #13710: URL: https://github.com/apache/gravitino/pull/13710
### What changes were proposed in this pull request? - Remove segment-lock acquisition from the Caffeine eviction/expiration callback. Prefix-index bookkeeping is serialized by a new per-cache index lock instead. - Keep the ordering one-way: segment lock -> index lock. Index lock holders only do a lock-free `policy().getIfPresentQuietly` lookup and radix-tree updates; no holder takes a segment lock or performs a mutating Caffeine operation. - `invalidateExpiredItem` removes the index entry only when the key is quietly absent, so a stale eviction notification cannot unindex a key that was put back. Explicit and cascading invalidation use the same guarded removal, and `clear()` swaps the tree under the index lock. - No new thread, queue, or config: `CaffeineEntityCache(Config, Executor)` is package-private and only for tests, so a direct/deferred executor can reproduce Caffeine's inline notification path. ### Why are the changes needed? Caffeine runs maintenance and removal notifications inline while holding its eviction lock, either through `ThreadPoolExecutor.CallerRunsPolicy` on the bounded cleanup queue or through its own inline fallback when submission fails. The removal listener took an entity segment lock. A writer that held a segment lock could at the same time wait for the eviction lock inside `cacheData.put` (Caffeine cleans up synchronously when its write buffer is full), so the two locks formed a cycle and the JVM reported a Java-level deadlock between the writer and `CaffeineEntityCache-Cleanup`. Raising the queue capacity does not break the cycle. Deferring index cleanup to a separate unbounded queue would also break the cycle, but lets a slow segment holder block cleanup while evicted keys and tasks accumulate outside the cache capacity. The independent index lock avoids both problems, and its critical sections contain only a quiet cache lookup and in-memory radix-tree updates. The guarded removal also fixes a correctness bug: an unconditional late removal lost the index entry for a reinserted key, so ancestor invalidation would miss it. Fix: #13580 ### Does this PR introduce _user-facing_ change? No API or configuration change. `size()` remains an estimate and may briefly count entries whose removal notification has not run yet. ### How was this patch tested? New `TestCaffeineEntityCacheEviction` covers: - Inline eviction while both relevant segment locks are held. - Stale removal after reinsertion, followed by ancestor (metalake) invalidation. - 2,000 distinct insertions while one segment stays locked: the index tracks live entries and evicted keys do not accumulate. - `clear()` followed by reinsertion before the stale notification runs. - Eight concurrent writers using the production executor with `maxEntries=64`. Mutation check: restoring the segment-lock callback makes 4 of the 5 new tests fail (the held-segment test reports the eviction timeout), so the tests bind to this fix. Passed: full `org.apache.gravitino.cache.*` (83 tests) plus `TestEntityCacheChangeLogListener` and `TestEntityCacheCrossNodeInvalidation`; `:core:spotlessJavaCheck` and `:core:javadoc` succeed. -- 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]
