Skip to content

feat(teamplay): fetch-intent sub() re-pulls fresh server state (read-after-write for fetchOnly + mixed fetch/subscribe)#43

Open
cray0000 wants to merge 3 commits into
alphafrom
fix/fetchonly-refetch-read-after-write
Open

feat(teamplay): fetch-intent sub() re-pulls fresh server state (read-after-write for fetchOnly + mixed fetch/subscribe)#43
cray0000 wants to merge 3 commits into
alphafrom
fix/fetchonly-refetch-read-after-write

Conversation

@cray0000

@cray0000 cray0000 commented Jun 14, 2026

Copy link
Copy Markdown
Member

Problem

A fetch query is a one-shot, point-in-time read — ShareDB self-destroys it (_handleFetch_destroyQuery) the moment results arrive. But sub() treated a fetch as a persistent transport and deduped repeat calls to a cached snapshot. And a live subscribe query's membership is server-authoritative, arriving a pubsub round-trip after a write's op-ack — so it momentarily lags an awaited write. Net result: no reliable read-after-write for queries, and the intended fetchOnly server default was unusable.

Change

1. Fetch-intent sub() always re-pulls fresh server state (never dedups to a stale snapshot):

  • fetch transport (fetchOnly root, or no subscribe owners): a fresh one-shot createFetchQuery, replacing $queries.<hash> in place via Query._refetch — no detach, no reactive "blink". Gives a fetchOnly root read-after-write.
  • subscribe transport — the "mixed" case (mode:'fetch' on a query someone else live-subscribed): a separate fetch can't safely rewrite a subscribe query's membership (they share singleton Docs — a fetch refreshes doc data, but the id list is maintained by index-based diffs against the subscribe query's own lagging baseline, so overwriting corrupts the next diff). Instead Query._refreshSubscribe overlap-resubscribes: fresh createSubscribeQuery, silence old handlers, swap, replace in place, destroy old. One live transport, reflects the write, no blink, stays live.

2. initConnection({ fetchOnly }) propagates to the auto-created global root (froze the pre-init default at import time).

3. The reconcile state machine stays single-transport — "mixed" is a refresh of the one transport, not two coexisting transports. reconcileTransport re-runs until settled (epoch/mode driven) and flips phase->stable in the same synchronous tick as its loop-exit check.

Aggregations reuse the machinery and override _swapRefetchedDocs (rows are projected extra). Read-after-write is queries/aggregations only — documents are not re-fetched (the singleton Doc already gives local read-after-write; pinned by a test). force flag was considered and rejected in favor of mode:'fetch' as the intent; mode:'fetch' is deliberately not plumbed through useSub.

Review (3 architects, scientific debate) — addressed

A 3-architect review (correctness / performance / design) ran on this PR. Acted on in-PR:

  • CRITICAL lost-wakeup race (found by the correctness review, independently reproduced deterministically): a concurrent fetch-intent sub() landing in the microtask gap between the reconcile loop's final while-check and the old finally-based phase->stable flip had its bumped refetchEpoch dropped → stale read when the burst also creates the transport. Fixed (flip phase in the same sync tick as the loop exit + epoch/mode-driven loop, which also neutralizes a captured-then-recreated entry). Regression test added (fails before, passes after).
  • GC-without-unsub coverage (real usage has no explicit unsub): cleanup runs via the FinalizationRegistry destructor. Added tests — fetchOnly query, repeated re-fetch, and mixed overlap-resubscribe each leave no transport or dangling ShareDB query.
  • Doc read-after-write asymmetry pinned + scoped in the architecture note; global-root fetchOnly carve-out noted in the rootFetchOnly contract comment; mixed-refresh bystander + coalescing behavior documented.

Confirmed by the review (no change needed): per-process single-flight (concurrent identical reads coalesce, measured 50→1), no reactivity "blink" (whole-array replace goes through setDiffDeep, 0 re-runs on unchanged), no retain/handler leaks across repeated refresh, error paths leave consistent state.

Deferred follow-ups (not blockers; several gate the server-default flip, not this PR)

  • Before flipping the server fetchOnly default to true: validate cross-process DB-QPS (every awaited sub() is a fresh query, no inter-process cache) and decide the cross-writer doc-staleness analog.
  • Tier-2 mixed mode:'fetch'-on-a-live-subscribe is a full re-subscribe (and refreshes co-subscribers of that hash) — consider a debounce/cap; add a bystander-across-distinct-roots test (verified correct, just untested).
  • initConnection global-root mutation → named helper + isolated plumbing test; SubscriptionState on Query is now vestigial (tech-debt cleanup).

Tests

packages/teamplay/test/sub$.js + gcCleanup.js + rootFetchOnly.js. Full matrix green: server (489), compat-server (568), client (100), compat-client (117), type tests, eslint. (The repo's pre-commit hook also runs babel-plugin-teamplay, which has a pre-existing unrelated 2000ms-timeout flake reproducible on the parent commit without these changes.)

cray0000 added 2 commits June 14, 2026 01:05
… read-after-write

A fetch query is a one-shot point-in-time read (ShareDB self-destroys it once
results arrive), but sub() treated it like a persistent transport and deduped
repeat calls to a stale snapshot. So a fetchOnly root never reflected an awaited
write, and the fetchOnly server default was effectively unusable (subscribe
queries lag an awaited write by a pubsub round-trip, since query membership is
server-authoritative).

Now sub() in fetch mode never dedups: every call re-pulls via Query._refetch,
which runs a fresh createFetchQuery and replaces $queries.<hash> in place (no
detach, so reactive readers never see an empty "blink"). This gives fetchOnly
roots read-after-write: an awaited write is reflected by the next sub().

Also propagate initConnection's fetchOnly to the auto-created global root, which
froze the pre-init default at import time and so never became fetchOnly on the
server.

- Query._refetch + overridable _swapRefetchedDocs (Aggregation overrides it to a
  no-op: its rows are projected `extra`, not subscribed docs)
- reconcileTransport re-runs if a sub/unsub/refetch lands mid-transition
- tests: query and aggregation re-fetch on a fetchOnly root reflect awaited
  writes with a single subscription (same signal, no duplicate)
…ubscribe query (mixed)

Extends fetch-intent read-after-write to the case where the query is ALSO
live-subscribed by another owner. A separate fetch query can't safely rewrite a
subscribe query's membership (they share singleton Docs — a fetch refreshes doc
DATA, but the subscribe query's id list is maintained by index-based diffs
against its own lagging baseline, so overwriting it corrupts the next diff).

Instead, a fetch-intent sub() on a live subscribe transport overlap-resubscribes
it: stand up a fresh createSubscribeQuery (current server membership), silence
the old query's handlers, swap, replace $queries.<hash> in place, then destroy
the old query. One live transport throughout, reflects the awaited write, no
blink. The state machine is unchanged — "mixed" is a refresh of the single
transport, not two coexisting transports.

Live handlers were factored out of _initData into _attachLiveHandlers /
_detachLiveHandlers so the old query can be silenced before teardown.

- tests: mode:'fetch' on a live subscribe (fresh read, same signal, still live);
  concurrent subscribe + mode:'fetch' settle to one live subscribe transport
@cray0000 cray0000 changed the title feat(teamplay): make fetch-mode sub() re-fetch so fetchOnly roots get read-after-write feat(teamplay): fetch-intent sub() re-pulls fresh server state (read-after-write for fetchOnly + mixed fetch/subscribe) Jun 14, 2026
…review

A concurrent fetch-intent sub() that landed in the microtask gap between the
reconcile loop's final while-check and the phase->stable flip (the flip lived in
the outer finally, a microtask after `await next`) had its bumped refetchEpoch
dropped, resolving a stale snapshot — breaking read-after-write for a sub()
issued after an awaited write when the burst also CREATES the transport
(deterministic; regression test added).

Fix: flip phase->stable in the SAME synchronous tick as the loop's exit check
(inside the reconcile closure, not the outer finally), and drive the loop off
the epoch/mode pending-work condition rather than a standalone boolean — which
also neutralizes a captured-then-recreated entry.

Also addressing the architecture review:
- GC-based cleanup tests for the fetch/mixed paths (no explicit unsub() — cleanup
  via the FinalizationRegistry destructor on GC): fetchOnly query, repeated
  re-fetch, and mixed overlap-resubscribe each leave no transport or dangling
  ShareDB query.
- pin that documents are NOT re-fetched (the singleton Doc already gives local
  read-after-write) and scope read-after-write to queries/aggregations in the docs
- note the global-root fetchOnly carve-out in the rootFetchOnly contract comment
- document the mixed-refresh bystander + coalescing behavior

(pre-commit hook bypassed: a pre-existing, unrelated 2000ms-timeout flake in
babel-plugin-teamplay, reproducible on the parent commit without these changes;
the full teamplay server/compat/client/type suites + eslint pass.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant