Retry pure user-DO reads once across a reset - #263
Open
ndisidore wants to merge 1 commit into
Open
Conversation
ndisidore
force-pushed
the
nathan/chore-user-do-retry
branch
from
August 18, 2026 23:27
f172c11 to
c356bec
Compare
user_do.reset.surfaced volume (from #133/#162, which deliberately deferred retries) showed the one call in flight at reset time is worth recovering instead of surfacing to the browser. do-telemetry.ts becomes do-retry.ts and gains retryOnDoReset(thunk): exactly one retry, full jitter (a mass reset fails every in-flight call at once), and identity rethrow so the workerd flags the frontend classifier reads survive. The retry predicate is narrower than isDoResetError: durableObjectReset retries even when flagged overloaded (the incarnation is dead, so the queue that overloaded it died with it -- this is the shape production storage-timeout resets arrive in, {remote, overloaded, durableObjectReset}), while bare retryable retries only if the object isn't shedding load. The replay-safety judgment lives at each call site as an explicit, greppable wrapper -- no method-name allowlist. A reset can't distinguish "never applied" from "applied, response lost", so only strictly pure reads are wrapped: the 14 read delegations in server.ts and the read sites on the fresh-stub session getters in overseer.ts (whoami, getChatContext, listModels, plus the two OverseerImpl-internal fresh-stub reads). The thunk re-evaluates the fresh-stub getter, so the second attempt reaches the new incarnation for free. Reads with hidden writes (listOutputs, listProvidedAccounts) and every write keep today's behavior; getChatContext's doc now pins that it must stay write-free. Successful retries log user_do.reset.recovered; correlated against user_do.reset.surfaced (which still fires per attempt, with operation attribution) this gives the retry win rate and the signal for wrapping more sites later.
ndisidore
force-pushed
the
nathan/chore-user-do-retry
branch
from
August 18, 2026 23:46
c356bec to
2440a15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#133 and #162 made user-DO resets survivable by minting a fresh stub per call, but deliberately left retries out until telemetry showed they were worth adding. The
user_do.reset.surfacedvolume has now made that case: the one call in flight at the moment the DO resets still fails all the way to the browser.This change retries that call once. A reset can't tell the caller whether the failed call was applied or not, so only calls that are safe to replay may retry. It requires an explicit
retryOnDoReset(() => this.#user.x())wrapper around strictly pure reads. The thunk re-evaluates the fresh-stub getter, so the second attempt reaches the new incarnation; errors are never re-wrapped, so the workerd flags the browser's classifier reads stay intact.Successful retries log
user_do.reset.recovered, which correlated againstuser_do.reset.surfacedgives the retry win rate and tells us whether more sites are worth wrapping.