Skip to content

feat: recover query caches after a backend restart - #202

Open
gmaclennan wants to merge 5 commits into
mainfrom
feat/backend-restart-recovery
Open

feat: recover query caches after a backend restart#202
gmaclennan wants to merge 5 commits into
mainfrom
feat/backend-restart-recovery

Conversation

@gmaclennan

@gmaclennan gmaclennan commented Aug 13, 2026

Copy link
Copy Markdown
Member

Measurement only — not intended to merge as-is. It answers "what does
node:sqlite cost in the Android lite binary", using the size a consumer
actually pays: the stripped libnode.so that gradle packages into an APK.
Release assets ship the binary unstripped, so that number isn't something
ls on an artifact reproduces.

Two arm64 legs, both configured lite. The second drops --without-sqlite
from the flavor's configure line in android_configure.py; each leg asserts
that SQLite is (or isn't) present in the resulting binary, so a reworded
configure line fails the job rather than quietly measuring the same build
twice. The baseline reuses build-android's cache key verbatim, so it
restores the binary the recipe ships instead of recompiling an approximation
of it, and only the sqlite leg pays for a compile.

The delta lands in the compare job summary, stripped and gzipped.

Adds an optional `subscribeToBackendRestart` prop to `ComapeoCoreProvider`.
When the listener fires, every query under this package's shared key prefix is
invalidated, so per-project API instances bound to the dead backend are
re-fetched instead of being served from a cache that never expires.

Also fixes a `map-share` listener leak: the received map shares store attached
its client API listener at creation and never removed it, so recreating the
store orphaned the previous listener. The listener is now attached from an
effect and removed on cleanup.
`SyncStore` attaches and removes its `sync-state` listener on the project
wrapper it was built from. A backend restart closes that wrapper, and the
removal runs in React effect cleanup, where a throw takes down the tree.
Upstream `@comapeo/ipc` is being changed so that `off` on a closed wrapper is a
no-op, but this package still supports the versions that throw.
A second `listen()` on the same store registered the client API listener twice,
so every incoming share was added to the store twice. It now returns the
existing teardown, and a teardown lets a later `listen()` attach again.

Also notes at `monitor()` that the download event source has no error path, so
a transport failure leaves the share stuck in `downloading`.
Invalidating everything under the shared key prefix does not recover the app.
Project-derived queries (settings, members, documents) refetch immediately with
a `queryFn` still closed over the project instance from the backend that went
away; those calls reject, and a `useSuspenseQuery` with `retry: false` then
latches into `status: 'error'`, which `shouldFetchOptionally` in query-core
never retries. Queries cached with `staleTime: 'static'` are excluded from
invalidation altogether, so the media server origin — whose port is ephemeral —
kept every image URL pointing at a dead port for the life of the app.

The restart listener now resets in three steps. It removes every query read
through a project instance, including the static media server origin, so they
cannot refetch with a dead closure. It then resets the cached project instances
themselves: removal alone is invisible to a mounted observer, which keeps
rendering its last result, whereas a reset suspends the component so it fetches
a fresh instance and re-runs the removed queries against it. Finally it
invalidates the remainder — device info, invites, the project list — which are
read through the client API that survives the restart, so a background refetch
is enough.

The subscribe effect now depends only on the subscribe function, with the query
client parked in a ref, and `SubscribeToBackendRestart` is exported so its
contract (including that it should be referentially stable) documents itself.
The README describes what the reset actually does and what it does not cover.
@gmaclennan

Copy link
Copy Markdown
Member Author

Reworked in ae9042c, dfb2e2c and 01fced6. The probes were right on all three counts, and I reproduced each one before changing anything.

The invalidate was doing the wrong thing. With a mock client API handing out generation-tagged project instances that reject once the generation is bumped, invalidating the root prefix leaves project_settings in status: 'error' with ProjectClosed — refetched with the pre-restart closure, and never retried because shouldFetchOptionally bails on an errored suspense query. media_server_origin stays success on the dead port, exactly as described.

But removeQueries on its own does not recover either, which was the surprise. A mounted observer is never notified when its query is removed: queryCache.remove calls query.destroy(), which dispatches nothing, and useBaseQuery only subscribes to the observer, not the cache. In the probe the cache empties and the component carries on rendering gen-0 data forever — getProject is never called again. Removal only helps a query that gets rebuilt on a later render, which is why #199 worked (the hook was unmounted at the time).

So the reset is three steps rather than two:

  1. removeQueries for everything read through a project instance — below projects/<projectId>, plus the static media_server_origin key — so nothing can refetch with a dead closure, and the staleTime: 'static' entry gets cleared at all.
  2. resetQueries for the project-instance keys. This is the part that makes mounted screens move: a reset does dispatch, so components suspend on useSingleProject and rebuild the queries removed in step 1 with closures over the fresh instance. Its own queryFn calls clientApi.getProject(), and the client API survives the restart, so this refetch is safe.
  3. invalidateQueries on the root for the manager-level remainder.

Ordering holds because every project-derived hook calls useSingleProject first in the same component, so the component is suspended before its own query can refetch. Two probe tests pin it and both fail against the invalidate-only version: the settings query ends up with new-generation data and never enters status: 'error', and the attachment URL moves from port 5000 to 5001.

document_created_by is the other staleTime: 'static' key. It is content-addressed so its data survives, but it sits inside the project scope and gets dropped with the rest rather than carved out — re-reading an immutable mapping is cheaper than the exception. Commented in place.

Also addressed:

  • Subscribe effect now depends only on the subscribe function, with the query client in a ref. SubscribeToBackendRestart is a named exported type, so the docs live on it, the README example is module-scope rather than an inline arrow, and the contract says outright that the function should be referentially stable.
  • SyncStore's on/off are wrapped, since the off runs in effect cleanup and older @comapeo/ipc throws on a closed wrapper.
  • listen() returns the existing teardown instead of double-registering; two tests cover that and re-attaching after teardown.
  • README now says what the reset actually does step by step, and has a "what it does not cover" section for the in-memory map-share state and the lost disconnect-window events. Added the TODO at monitor() for the missing event-source error path next to the existing timeout one.
  • docs/API.md: the unreadable single-line JSDoc dump is gone — extracting SubscribeToBackendRestart moved the prose into its own entry with proper parameter/return rows. The ReceivedMapSharesContext row is still truncated mid-signature (listen(): (...), but that is tsdoc-markdown truncating a long inline type and predates this PR.

92 tests, lint and typecheck green.

A query in flight when the backend's RPC transport drops rejects with
code RPC_TRANSPORT_CLOSED — a read whose response will never arrive.
Without a retry it latches into an error state during the seconds
between the drop and the restart reset, flashing error boundaries for
what is really continued loading. baseQueryOptions now retries exactly
that code (bounded, 1s delay); the retried call sits in the transport's
send queue until the restarted backend answers. Matched by error code,
not instanceof, so a duplicated @comapeo/ipc in the tree can't break
the check. Project-scoped queries reject with a different code on
retry (their instance is closed) and stop — those are recovered by
resetQueriesAfterBackendRestart as before. Mutations keep retry: false.
@gmaclennan

Copy link
Copy Markdown
Member Author

Follow-up (f60af9f): baseQueryOptions now retries queries — and only queries — that reject with code: 'RPC_TRANSPORT_CLOSED' (bounded at 3 attempts, 1s delay). A query in flight when the backend's transport drops is a read whose response will never arrive; without this it latched into an error state (error-boundary flash) during the seconds before the restart reset fires. The retried call waits in the transport's send queue until the restarted backend answers, so the UI just keeps loading. Matched by error code rather than instanceof so a duplicated @comapeo/ipc copy can't break the check; project-scoped queries reject with PROJECT_CLOSED on retry and stop, staying in the reset's domain; mutations keep retry: false. Two new probe tests (retried-and-resolves without touching the error boundary; other errors not retried) — 94/94 passing, tsc/eslint/prettier clean.

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