You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A cached query result can be stored under a namespace version that is newer than the data the result actually reflects, causing a stale read that persists until the entry's TTL expires. This is a pre-existing property of the version-manager cache design (it affects the structured-query path as well as pipes) — it is not introduced by #343, but it surfaced while reviewing the pipe cache-invalidation work there, and #343's new view-cascade bumps are subject to the same window.
How the cache versioning works (context)
WaveHouse's cache is version-keyed, not delete-based. Every result is stored under sha(query+params) | folded(dependency-namespace versions). Each dependency namespace — (table, scope) — has a monotonic version counter in the VersionManager. A write doesn't delete anything: the ingest worker calls Invalidate, which bumps the relevant version; because that version is folded into the key, the bump changes the key so the old entry simply stops matching (lazy eviction). So an entry keyed at version V is an implicit claim: "this value reflects every write up to version V of these namespaces." Correctness rests on that claim staying true.
The race
LocalCache.Get and LocalCache.Set each recompute the key independently via VersionManager.QueryKey(sha, deps), reading whatever the dependency versions are at that instant (internal/cache/local.go). Nothing carries the version snapshot observed at Get through to Set.
Timeline for a single request on a cache miss:
t0 — Get computes QueryKey with dep version v0 → miss.
t1 — the query executes and reads DB state as of t1.
t2 — a write to a dependency table lands; the ingest worker calls Invalidate → the version bumps v0 → v1.
t3 — Set recomputes QueryKey, now reads v1, and stores the pre-write result (from t1) under the v1 key.
t4 — a later Get computes QueryKey with v1 → HIT → returns the stale, pre-write result until TTL.
The entry ends up filed under a version it doesn't actually reflect: it claims v1 but holds v0 data, so the version's "reflects writes up to V" guarantee is broken for any write landing in the (t1, t3) window — i.e. any write that overlaps a query's execution.
Scope / severity
Affects both structured queries (internal/api/structured_query.go) and pipes (internal/api/pipes.go) — same Get/Set/QueryKey pattern.
Bounded by the entry's TTL (QueryTimeToTTL, and the 10s floor for unresolved pipe deps), so it self-heals; it is not unbounded staleness.
Window scales with query duration — slow queries under a steady write stream are the worst case.
Capture the dependency versions at Get and thread that snapshot through to Set, so the result is filed under the version it was computed against — not the version current at store time. If a bump lands mid-flight, Set then writes under the now-superseded key, the next Get (using the new version) misses, and the query re-runs against fresh data. Cost is at most a redundant recompute; the "reflects writes up to V" guarantee is restored.
Mechanically this means Set should take the versioned key (or the version snapshot) produced at Get rather than recomputing it. Worth auditing whether the singleflight coalescing changes anything here (multiple waiters share one Set).
Related
Two sibling instances of the same "compute against a version snapshot, commit after it was invalidated" hazard live in PR #343's pipe path (flagged in the review there): the unserialized concurrent-refresh cascade install, and the pipe deps-memo re-insert after a concurrent ClearResolvedDeps. Those are memo/cascade-local with their own fixes; this issue is the cache-layer root form, and the one that also affects structured queries.
Context
Found during the local review of #343 (pipe cache invalidation via table-dependency resolution). Filing separately because it is a cache-layer issue independent of that PR and should be fixed without blocking it.
Summary
A cached query result can be stored under a namespace version that is newer than the data the result actually reflects, causing a stale read that persists until the entry's TTL expires. This is a pre-existing property of the version-manager cache design (it affects the structured-query path as well as pipes) — it is not introduced by #343, but it surfaced while reviewing the pipe cache-invalidation work there, and #343's new view-cascade bumps are subject to the same window.
How the cache versioning works (context)
WaveHouse's cache is version-keyed, not delete-based. Every result is stored under
sha(query+params) | folded(dependency-namespace versions). Each dependencynamespace—(table, scope)— has a monotonic version counter in theVersionManager. A write doesn't delete anything: the ingest worker callsInvalidate, which bumps the relevant version; because that version is folded into the key, the bump changes the key so the old entry simply stops matching (lazy eviction). So an entry keyed at versionVis an implicit claim: "this value reflects every write up to versionVof these namespaces." Correctness rests on that claim staying true.The race
LocalCache.GetandLocalCache.Seteach recompute the key independently viaVersionManager.QueryKey(sha, deps), reading whatever the dependency versions are at that instant (internal/cache/local.go). Nothing carries the version snapshot observed atGetthrough toSet.Timeline for a single request on a cache miss:
t0—GetcomputesQueryKeywith dep versionv0→ miss.t1— the query executes and reads DB state as oft1.t2— a write to a dependency table lands; the ingest worker callsInvalidate→ the version bumpsv0 → v1.t3—SetrecomputesQueryKey, now readsv1, and stores the pre-write result (fromt1) under thev1key.t4— a laterGetcomputesQueryKeywithv1→ HIT → returns the stale, pre-write result until TTL.The entry ends up filed under a version it doesn't actually reflect: it claims
v1but holdsv0data, so the version's "reflects writes up toV" guarantee is broken for any write landing in the(t1, t3)window — i.e. any write that overlaps a query's execution.Scope / severity
internal/api/structured_query.go) and pipes (internal/api/pipes.go) — sameGet/Set/QueryKeypattern.QueryTimeToTTL, and the 10s floor for unresolved pipe deps), so it self-heals; it is not unbounded staleness.Suggested fix
Capture the dependency versions at
Getand thread that snapshot through toSet, so the result is filed under the version it was computed against — not the version current at store time. If a bump lands mid-flight,Setthen writes under the now-superseded key, the nextGet(using the new version) misses, and the query re-runs against fresh data. Cost is at most a redundant recompute; the "reflects writes up toV" guarantee is restored.Mechanically this means
Setshould take the versioned key (or the version snapshot) produced atGetrather than recomputing it. Worth auditing whether the singleflight coalescing changes anything here (multiple waiters share oneSet).Related
Two sibling instances of the same "compute against a version snapshot, commit after it was invalidated" hazard live in PR #343's pipe path (flagged in the review there): the unserialized concurrent-refresh cascade install, and the pipe deps-memo re-insert after a concurrent
ClearResolvedDeps. Those are memo/cascade-local with their own fixes; this issue is the cache-layer root form, and the one that also affects structured queries.Context
Found during the local review of #343 (pipe cache invalidation via table-dependency resolution). Filing separately because it is a cache-layer issue independent of that PR and should be fixed without blocking it.