fix(cache): invalidate persona AI context caches across pods - #31656
Conversation
PersonaContextCache.local and SubjectCache.USER_CONTEXT_CACHE are per-JVM and were never reached by the CacheInvalidationPubSub subscriber, so on a multi-replica deployment a persona change was visible on one pod and stale on the others for up to that cache's TTL (30 min / 15 min). - PersonaContextCache implements Invalidatable and is registered with CacheBundle. A peer can't know the definition hash the local entry is keyed by, so it drops by "personaId:" prefix and clears generationStates. The drop is local only: the publishing pod already wrote (or deleted) the authoritative Redis copy, and a peer deleting those keys would throw away a document that was just rebuilt. - refresh() publishes explicitly. It rewrites Redis but mutates no entity, so nothing broadcast it before and an admin regenerate was silently a no-op on every pod but the one that served the request. It goes out under a non-entity type so peers don't bump the persona write epoch or evict entity caches for a document-only change. - SubjectCache registers an Invalidatable too: a user write drops that user's context (matched case-insensitively, since the FQN is lower-cased while the cache is keyed by the principal name), a persona write drops all of them. Without this, SubjectContext.getActivePersona() kept discarding a freshly assigned persona as "not assigned" for up to 15 minutes, which reads as a persona switch that intermittently doesn't take. - PersonaRepository.postCreate publishes; creates don't broadcast otherwise, so a persona created with users assigned never reached peers. - Cap the local context TTL at 60s while Redis is up. Asset drift (a retagged table, an edited KB article) has no entity to key an invalidation off, so the cap is what bounds it. With Redis down the local entry is the whole cache, so the definition's TTL still stands there. Fixes open-metadata/openmetadata-collate#5847 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
Ingestion
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| private static final Invalidatable INVALIDATOR = | ||
| (type, id, fqn) -> { | ||
| if (Entity.PERSONA.equals(type)) { | ||
| invalidateAllUserContexts(); | ||
| } else if (Entity.USER.equals(type) && fqn != null) { | ||
| invalidateUserContextByFqn(fqn); | ||
| } | ||
| }; |
There was a problem hiding this comment.
⚠️ Edge Case: Peer reload after persona write may still read stale assignments
On a cross-pod persona assignment change the writing pod publishes a PERSONA invalidation; peers run SubjectCache.invalidator -> invalidateAllUserContexts() and then lazily reload each user via Entity.getEntityByName(USER, ..., "...,personas,defaultPersona"). That reload resolves the user's personas through the user-keyed relationship/entity caches (CachedRelationshipDao/CachedEntityDao), but the remote PERSONA invalidation only invalidates the Persona entity (onRemoteCacheInvalidate("persona", personaId, fqn) / cachedReadBundle.invalidate), not the affected users' entity or user-side relationship cache entries. If those are warm on the peer, dropping USER_CONTEXT_CACHE just repopulates it with the same stale persona list, so getActivePersona() can still reject the requested persona — the exact symptom this PR targets. Consider having the writer publish USER invalidations for the affected users (or invalidate the user-side relationship cache on peers) so the context reload sees the new assignments.
Was this helpful? React with 👍 / 👎
| private static void invalidateUserContextByFqn(String fqn) { | ||
| try { | ||
| String userName = FullyQualifiedName.unquoteName(fqn); | ||
| USER_CONTEXT_CACHE.asMap().keySet().removeIf(key -> key.equalsIgnoreCase(userName)); | ||
| } catch (Exception e) { | ||
| LOG.debug("Could not invalidate user context for fqn {}", fqn, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Performance: SubjectCache invalidator scans full context cache on every USER write
invalidateUserContextByFqn does USER_CONTEXT_CACHE.asMap().keySet().removeIf(key -> key.equalsIgnoreCase(userName)), a full linear scan of the context cache. Because the invalidator is registered globally, this runs for every remote USER write across the whole cluster. The cache is bounded so impact is limited, but on a busy deployment with many user writes this is repeated O(n) scans where an exact-key invalidate(userName) (plus its lower-cased form) would be O(1). Consider invalidating the specific key(s) directly instead of scanning.
Was this helpful? React with 👍 / 👎
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
✅ Playwright Results — workflow succeededValidated commit ✅ 796 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 59m 30s ⏱️ Max setup 3m 51s · max shard execution 21m 25s · max shard-job elapsed before upload 24m 55s · reporting 5s 🌐 220.55 requests/attempt · 2.66 app boots/UI scenario · 7.55% common-shard skew Optimization targets still in progress:
🟡 1 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
A team's defaultPersona reaches users as inheritedPersonas, and membership or parent-hierarchy edits change which teams contribute one. Those writes publish type=team, which the invalidator ignored, so a peer kept the old inherited persona for up to the 15-minute TTL — the same symptom this PR fixes for direct assignments. TeamRepository already invalidates locally (SubjectCache.invalidateAll on hierarchy/policy change); this is the peer half. Scoped to user contexts. USER_POLICIES_CACHE has its own 2-minute TTL and peers relying on it is pre-existing behaviour this persona fix shouldn't widen. Also document why invalidateUserContextByFqn scans case-insensitively rather than doing an O(1) exact-key invalidate: SecurityUtil.getUserName splits the principal on [/@] without case-folding, so an IdP emitting John.Doe@corp.com keys the cache under "John.Doe" while the FQN is "john.doe". An exact-key lookup would silently miss those. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — went through all three. One is a real gap and is fixed in c059194; the other two I'm pushing back on, with the reasoning below. ✅ greptile: team updates bypass peer eviction — fixedCorrect, and it's the same bug class as the one this PR targets. A team's Applied the suggestion, plus a test ( I kept it scoped to user contexts rather than ❌ gitar: peer reload may still read stale assignments — not applicableThe premise is that the reload resolves personas "through the user-keyed relationship/entity caches (
So dropping ❌ gitar: O(n) scan in
|
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source
* fix(cache): invalidate persona AI context caches across pods PersonaContextCache.local and SubjectCache.USER_CONTEXT_CACHE are per-JVM and were never reached by the CacheInvalidationPubSub subscriber, so on a multi-replica deployment a persona change was visible on one pod and stale on the others for up to that cache's TTL (30 min / 15 min). - PersonaContextCache implements Invalidatable and is registered with CacheBundle. A peer can't know the definition hash the local entry is keyed by, so it drops by "personaId:" prefix and clears generationStates. The drop is local only: the publishing pod already wrote (or deleted) the authoritative Redis copy, and a peer deleting those keys would throw away a document that was just rebuilt. - refresh() publishes explicitly. It rewrites Redis but mutates no entity, so nothing broadcast it before and an admin regenerate was silently a no-op on every pod but the one that served the request. It goes out under a non-entity type so peers don't bump the persona write epoch or evict entity caches for a document-only change. - SubjectCache registers an Invalidatable too: a user write drops that user's context (matched case-insensitively, since the FQN is lower-cased while the cache is keyed by the principal name), a persona write drops all of them. Without this, SubjectContext.getActivePersona() kept discarding a freshly assigned persona as "not assigned" for up to 15 minutes, which reads as a persona switch that intermittently doesn't take. - PersonaRepository.postCreate publishes; creates don't broadcast otherwise, so a persona created with users assigned never reached peers. - Cap the local context TTL at 60s while Redis is up. Asset drift (a retagged table, an edited KB article) has no entity to key an invalidation off, so the cap is what bounds it. With Redis down the local entry is the whole cache, so the definition's TTL still stands there. Fixes open-metadata/openmetadata-collate#5847 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cache): drop peer user contexts on team writes too A team's defaultPersona reaches users as inheritedPersonas, and membership or parent-hierarchy edits change which teams contribute one. Those writes publish type=team, which the invalidator ignored, so a peer kept the old inherited persona for up to the 15-minute TTL — the same symptom this PR fixes for direct assignments. TeamRepository already invalidates locally (SubjectCache.invalidateAll on hierarchy/policy change); this is the peer half. Scoped to user contexts. USER_POLICIES_CACHE has its own 2-minute TTL and peers relying on it is pre-existing behaviour this persona fix shouldn't widen. Also document why invalidateUserContextByFqn scans case-insensitively rather than doing an O(1) exact-key invalidate: SecurityUtil.getUserName splits the principal on [/@] without case-folding, so an IdP emitting John.Doe@corp.com keys the cache under "John.Doe" while the FQN is "john.doe". An exact-key lookup would silently miss those. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
picked to 2.0 |
Problem
PersonaContextCache.local(Caffeine) andSubjectCache.USER_CONTEXT_CACHE(Guava) are per-JVM and were never reached by theCacheInvalidationPubSubsubscriber — neither implementsInvalidatableand neither is passed toCacheBundle.registerInvalidatable. On a multi-replica deployment a persona change is visible on one pod and stale on the others for up to that cache's TTL (30 min / 15 min), intermittently, depending on which pod the load balancer picks.Symptoms, all reported against Ask Collate:
refresh()rewrites Redis and the calling pod's local entry only. The definition hash doesn't change, so peers keep serving the identically-keyed stale entry until their own TTL.Userwithout it, soSubjectContext.getActivePersona()silently discards theX-OpenMetadata-Personaheader and falls back to the default persona (Requested persona '<id>' is not assigned to user '<user>').Reported downstream as open-metadata/openmetadata-collate#5847, which has the full analysis.
Changes
PersonaContextCache implements Invalidatable, registered withCacheBundle. A peer can't know the definition hash the local entry is keyed by (personaId:definitionHash), so it drops bypersonaId:prefix and clearsgenerationStates.The drop is local only, deliberately: the publishing pod already wrote (or deleted) the authoritative Redis copy, and a peer deleting those keys would throw away a document that was just rebuilt and make every pod re-run the ES-heavy build.
refresh()publishes explicitly. It rewrites Redis but mutates no entity, so nothing broadcast it before. It goes out under a non-entity type (CacheInvalidationPubSub.TYPE_PERSONA_CONTEXT) so peers drop the document without bumping the persona write epoch or evicting entity caches for a change that touched no entity. The handler inCacheBundlegates the entity-cache work on that type; theInvalidatablefan-out still runs.SubjectCacheregisters anInvalidatabletoo. Auserwrite drops that user's context; apersonaorteamwrite drops all of them (a team'sdefaultPersonareaches users asinheritedPersonas, via membership or the parent hierarchy).The all-drop is blunt on purpose — the affected user set isn't derivable from the message, since assignments change through the persona, the user's
defaultPersona, and a team's default alike. Persona writes are rare admin actions and the reload is a cached entity read. A contextrefreshpublishes underTYPE_PERSONA_CONTEXTand so does not land here.The user match is case-insensitive: a user's FQN is the lower-cased quoted name, while the cache is keyed by the principal name as the request presented it.
PersonaRepository.postCreatepublishes.EntityRepository.postCreatecallsCacheBundle.invalidateEntity(local fan-out) but never publishes — only the update and delete paths do. So a persona created with users already assigned never reached peers at all.Local context TTL capped at 60 s while Redis is up. Nothing invalidates when a referenced asset changes (a retagged table, an edited KB article, a new asset matching a rule), and that drift has no single entity to key an invalidation off — the cap is what bounds it, and it also bounds any dropped pub/sub message. Expiry then costs one
mgetof the shared copy, not a rebuild.With Redis down the local entry is the cache, so the definition's
cacheTtlMinutesstill stands there — capping unconditionally would make every pod re-run the build every minute.Testing
PersonaContextCacheTest(2 new) andSubjectCacheTest(3 new) cover the remote-invalidate path: a message in, local entry gone, next read rebuilds; unrelated entity types leave the entry warm.Both were confirmed non-vacuous by short-circuiting the two invalidators and watching all four positive tests fail (
expected: <MISS> but was: <HIT>), then restoring.mvn spotless:checkclean,compileclean.aicontext,cache,policyevaluator, andPersonaRepositoryTest.Not covered
Invalidatableboundary only.generationStatesis still pod-local, soGET /v1/personas/{id}/context/statuscan still reportGENERATINGon one pod andFRESH/STALEon another. Invalidation now clears it, but making the status genuinely shared needs Redis-backed state.🤖 Generated with Claude Code
Fixes open-metadata/openmetadata-collate#5847
Greptile Summary
The PR adds distributed invalidation for persona AI context and user-context caches so persona changes converge across service pods.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (2): Last reviewed commit: "fix(cache): drop peer user contexts on t..." | Re-trigger Greptile
Context used: