MemberJunction / MemberJunction/MJ
BaseEngine filtered caches can serve stale reads after a write — LocalCacheManager invalidation is unordered vs. the triggering read
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
# Follow-up: server-side RunView cache-invalidation is unordered vs. read (read-your-writes hole)
> **Context / provenance.** Found while fixing the multi-op installed-apps regression from PR #3018 (adding/removing/reordering several `MJ: User Applications` in one config-dialog save applied "one operation behind" until a page reload). A tactical fix has already shipped on branch `colin-fix-userapps-refresh-race` (event-driven `BaseEngine` refreshes now pass `BypassCache: true`, plus a per-property client-side ordering guard). **This note documents the underlying server-side gap for a proper follow-up — the shipped fix is a read-side workaround, not a cure.** All file:line refs are against MJ `next` (~b99afbbf3a).
## Summary
When a `BaseEngine` config has a `Filter` (full-refresh path), a `BaseEntity.Save()`/`Delete()` triggers an event-driven `RunView` refresh that reads back through the **server-side** `LocalCacheManager` RunView cache. The invalidation the write is supposed to perform on that cached filtered entry is dispatched **fire-and-forget from an MJGlobal event subscriber and is never ordered against the read the same operation triggers**. In a multi-row save burst, the refresh reads — and a cache-miss re-population can re-write — a pre-/intermediate snapshot, which then serves as a stable cache **hit** until the 5-min TTL or a full page reload. The invalidation machinery itself is correct; the gap is a missing happens-before guarantee between write-invalidation and read.
## Root cause (event fires, index is correct, but invalidation is unordered vs. the read)
1. **The entry is a filtered `CacheLocal` server cache write.** `UserInfoEngine._UserApplications` is `Filter: userFilter` + `CacheLocal: true` (`packages/MJCoreEntities/src/engines/UserInfoEngine.ts:208-220`). Server-side `runViewCacheEligible` caches it because `CacheLocal === true` — filtered results are NOT excluded on the explicit-`CacheLocal` path (only the auto-cache path excludes filters) (`packages/MJCore/src/generic/providerBase.ts:1213-1219`; write `:2655-2665`).
2. **Filtered entries ARE indexed** (rules out "not indexed"). `SetRunViewResult` → `addToEntityIndex(fingerprint)` unconditionally (`localCacheManager.ts:1416`); reverse index keyed on the entity-name segment only via `extractEntityFromFingerprint` (`:455-458`, `:503-510`), so connection-prefix/RLS suffixes on the fingerprint (`GenerateRunViewFingerprint` `:1159-1228`) don't affect lookup — rules out key-prefix mismatch and composite-key concerns (UserApplication PK is single `ID`).
3. **The save emits the in-process event and invalidation resolves the filtered entry by entity name.** Server saves: `ResolverBase.CreateRecord`/`UpdateRecord` → `entityObject.Save()` (`packages/MJServer/src/generic/ResolverBase.ts:1145`, comment at `:1148`: "Cache invalidation is now handled globally by the MJGlobal listener in index.ts"). `BaseEntity.RaiseEvent` re-raises to `MJGlobal` (`packages/MJCore/src/generic/baseEntity.ts:1391-1406`). `LocalCacheManager.HandleBaseEntityEvent` looks up fingerprints by entity name (`resolveFingerprintsForEntity` `localCacheManager.ts:543-559`) and blows away filtered entries via `InvalidateRunViewResult` (`processEntityEventForFingerprint:843-864`, `isFilteredFingerprint:466-468`).
4. **The gap: that invalidation is fire-and-forget and un-awaited.** Both the `LocalCacheManager` subscriber (`localCacheManager.ts:594-597`, `.catch()` fire-and-forget) and the `ProviderBase` dedup/linger subscriber (`providerBase.ts:348-363`) are un-awaited MJGlobal handlers; no server code awaits event-driven invalidation on the save path. The refresh reads *through* the cache: `BaseEngine.LoadSingleEntityConfig` → `rv.RunView({ …CacheLocal…, BypassCache })` (`baseEngine.ts:1733-1751`), client forwards verbatim (`graphQLDataProvider.ts:864-884`), server serves any hit with zero DB queries because `TrustLocalCacheCompletely` (`providerBase.ts:2057-2085`). **No happens-before edge** between the write's invalidation and the refresh's read — separate GraphQL requests, separate async tasks.
5. **Why it persists across many refreshes.** A cache-miss refresh re-populates via `PostRunView` (`SetRunViewResult`). In a burst, overlapping refreshes + multiple fire-and-forget invalidations interleave; a refresh whose DB read predates the final commit but whose cache-write lands after the final invalidation re-installs a stale snapshot. Once writes stop, every refresh is a cache **hit** on that stale entry until TTL/reload. Corroborated by the shipped fix needing a **client-side generation guard** (`baseEngine.ts:1707-1724`) *in addition to* `BypassCache` — an ordering problem exists on both sides.
**Confidence:** High that the event fires, the index + filtered-invalidation logic are correct, and the invalidation is fire-and-forget/unordered vs. the triggering read (all verified in code). Medium on the exact dominant persistence mechanism (invalidation losing the race vs. cache-miss re-population writing stale) — static analysis can't isolate the runtime interleaving. Ruled out: `AllowCaching` write/invalidate asymmetry (same `IsCachingEnabledForEntity` gates both — `SetRunViewResult:1340-1350`, `HandleBaseEntityEvent:616`). (A genuinely different sibling failure mode is a write path that bypasses `BaseEntity.Save()` — cf. the `MJ: Record Changes` exemption `providerBase.ts:2957-2972` — but UserApplication writes go through `Save()`, so the event does fire.)
## Why the shipped `BypassCache` workaround is correct but incomplete
The fix (`baseEngine.ProcessEntityEvents` → `LoadSingleConfig(config, user, /*bypassCache*/ true)`) is correct: with `BypassCache: true` the refresh neither reads nor re-populates the server cache, so it always returns true DB state. It's broad within `BaseEngine` — it covers every filtered/ordered engine config's event refresh (UserInfoEngine alone has ~7: `_UserNotifications`, `_Workspaces`, `_UserSettings`, `_UserFavorites`, `_UserRecordLogs`, `_UserNotificationPreferences`, `_UserApplications` — all latently exposed, now all covered).
The same server-side hole **remains** for: non-engine filtered `RunView`/`RunViews` immediately after a save without `BypassCache` (a component's "save then re-query", a resolver reading back a related set); the **initial** engine load in another tab/session after a write elsewhere (`LoadMultipleEntityConfigs` uses `bypassCache=false`, `baseEngine.ts:1867-1881`); cross-server (Redis) propagation (adds async delay on the same fire-and-forget model).
## Classification: architectural root, **medium targeted fix**
It's a property of the foundational "server trusts its cache completely + asynchronous event-driven invalidation" design that every cached entity relies on — the design silently assumes invalidation lands before the next read, which holds for time-separated reads but not read-immediately-after-write. Making invalidation fully synchronous everywhere would be large (save-latency blast radius on all mutations). But the read-your-writes hole is closable with a localized change (est. a few hundred lines + tests in `LocalCacheManager` + `providerBase`). The shipped workaround is genuinely small (dozens of lines) and is the right tactical mitigation.
## Suggested server-side fix directions
1. **Per-entity write fence (recommended).** On save/delete, synchronously stamp a monotonic version/timestamp per entity name in `LocalCacheManager` (in the subscriber that already fires). In `PreRunView`, treat any cache entry whose `cachedAt` predates the entity's last-write stamp as stale → bypass + refresh + re-cache. Closes read-after-write generally; no save-path latency; no write blast radius.
2. **Server-side re-cache generation guard.** Mirror the client fix on the write side: tag each cache-miss re-population with an epoch; skip `SetRunViewResult` if an invalidation for that entity occurred during the DB read — prevents a stale read re-installing itself.
3. **Await invalidation before the mutation returns** — simplest, but only orders the same request; still needs (1) for a *different* request's read, and adds latency to every save. Lowest value.
## Repro / verification
1. Single MJAPI + MJExplorer, no Redis (in-memory server cache). Sign in; open the installed-apps config dialog.
2. In one save action, **add + remove + reorder ≥3 apps** (single-op won't reliably reproduce). On a build with `ProcessEntityEvents`' `bypassCache` reverted to `false`, the app switcher / Home trail "one operation behind" until reload.
3. Instrument `LocalCacheManager.InvalidateRunViewResult` + `SetRunViewResult` (log fingerprint + timestamp) and `PreRunView` hit/miss for `MJ: User Applications`. Expected: a stale `SetRunViewResult` landing *after* the last `InvalidateRunViewResult`, then only cache **hits** on subsequent refreshes.
4. After implementing the write fence, re-run step 2 *without* the `BypassCache` workaround; confirm a fence-forced miss → fresh DB read → UI settles immediately. Add a deterministic case to `packages/MJServer/integration-test-scripts/`: cache a filtered view, do N rapid `BaseEntity.Save()`s, assert a plain (non-`BypassCache`) `RunView` returns the post-write set.
Contributor guide
Research direction
Start with packages/MJCore/src/generic/localCacheManager.ts and providerBase.ts, tracing PreRunView, SetRunViewResult, InvalidateRunViewResult, and HandleBaseEntityEvent; then inspect the integration-test-scripts under packages/MJServer. Reproduce rapid filtered-view reads after several BaseEntity.Save() calls and add a deterministic test showing that a plain RunView returns the post-write set without relying on BypassCache.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend-api-design, performance, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100