Repository navigation
A retrying transaction replays rendered frames: RetryTransaction and the [AutoDatabase] generator go - #3339
Merged
Conversation
… RetryTransaction and the [AutoDatabase] generator go RespRetryTransactionExecutor records each queued command's rendered RespRequest plus a durable caller proxy, then replays the frames into a fresh one-shot RespTransactionExecutor per attempt, sharing the database's RetryController. Fresh attempt operations per replay are what dodge the real blocker: enrolled pooled operations are faulted directly by connection teardown, so re-enrolling one races pool recycling. With its last consumer gone, the capture-and-replay tier is deleted whole: RetryTransaction, AutoDatabase.cs, CapturedArgs.cs, the AutoDatabaseGenerator, SER352, and IInternalTransaction. Nothing on IDatabase is source-generated any more. Deliberate behavior changes: a retrying transaction now carries asyncState (previously refused) and exposes a working context that queues into the recorded unit (previously threw). Reuse after execute is still refused. Fire-and-forget parity is now real rather than claimed: both transaction executors hand every declined reply the one shared completed-default task, and TaskBridge passes a completed fire-and-forget task through untouched instead of rewrapping it.
mgravell
added a commit
that referenced
this pull request
Nov 13, 2026
…utor core (#3338) * WithRetry, WithKeyPrefix and the group database ride the context/executor core: RetryDatabase and the KeyPrefixed* family go - WithRetry(IDatabaseAsync) re-contexts via RespRetryExecutor (asyncState now carried; foreign database passes through unchanged); RetryTransaction stays, built by CreateTransaction when the chain retries - MultiGroupDatabase is a slim RedisDatabase over RespGroupExecutor; the 628 generated members go, scans pin to the active member, GetDatabase(-1) resolves from configuration - WithKeyPrefix(IDatabase) re-contexts via AppendKeyPrefix + AppendChannelPrefix and THROWS for a foreign database (silently not prefixing is the wrong keyspace); KeyPrefixed*/KeyPrefixedBatch/KeyPrefixedTransaction deleted - gaps closed: RANDOMKEY refuses under a prefix; SORT BY/GET patterns map (nosort/# exempt); Lua @key ARGV copies prefix via extraction over a WithoutKeyPrefix view; routing questions ask about the prefixed key; AppendKeyPrefix carries topology + cache resolver - the member-by-member wrapper tests (~4,100 lines) become KeyPrefixContextTests over the FakeExecutor RESP-measuring harness * A retrying transaction replays rendered frames, like everything else: RetryTransaction and the [AutoDatabase] generator go (#3339) RespRetryTransactionExecutor records each queued command's rendered RespRequest plus a durable caller proxy, then replays the frames into a fresh one-shot RespTransactionExecutor per attempt, sharing the database's RetryController. Fresh attempt operations per replay are what dodge the real blocker: enrolled pooled operations are faulted directly by connection teardown, so re-enrolling one races pool recycling. With its last consumer gone, the capture-and-replay tier is deleted whole: RetryTransaction, AutoDatabase.cs, CapturedArgs.cs, the AutoDatabaseGenerator, SER352, and IInternalTransaction. Nothing on IDatabase is source-generated any more. Deliberate behavior changes: a retrying transaction now carries asyncState (previously refused) and exposes a working context that queues into the recorded unit (previously threw). Reuse after execute is still refused. Fire-and-forget parity is now real rather than claimed: both transaction executors hand every declined reply the one shared completed-default task, and TaskBridge passes a completed fire-and-forget task through untouched instead of rewrapping it.
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.
Second of the two-PR series, stacked on #3338 — review just this diff; it retargets to
v4when #3338 lands (I'll rebase onto the squash-mergedv4at that point rather than letting the stale base replay). Net: +760 / −1,804 across 23 files.The idea
A transaction is RESP blobs paired to results, the same as everything else — so it retries the same way everything else does: by replaying rendered frames, not by capturing and replaying method calls.
RespRetryTransactionExecutorrecords each queued command's renderedRespRequestplus a durable caller proxy at queue time, and conditions semantically with per-attempt verdict buffers. Each attempt builds a fresh one-shotRespTransactionExecutorand replays the frames into it; on a clean commit the attempt's results are forwarded into the durable proxies, on a retryable fault the attempt is discarded whole and replayed. It shares the database'sRetryController, so budgets, failover tracking and watch-conflict policy are the ones a plain command sees; the failover token is captured before the first attempt.ITransactionExecutoris the seamRedisTransactionconsumes, so plain and retrying transactions are the same type over different executors.Fresh attempt operations per replay are what dodge the concrete blocker that kept the capture-replay wrapper alive: enrolled pooled operations are faulted directly by connection teardown, so re-enrolling one races pool recycling.
What goes
With its last consumer gone, the capture-and-replay tier is deleted whole:
RetryTransaction,AutoDatabase.cs,CapturedArgs.cs, theAutoDatabaseGenerator, diagnostic SER352 (+ its docs page), andIInternalTransaction. Nothing onIDatabaseis source-generated any more;DatabaseSurfaceTestsnow pins that nothing creeps back.Deliberate behavior changes
NotImplementedException— the wrapper recorded method calls, so frames had nowhere to go; the frame recorder takes them natively). Reuse after execute is still refused.TaskBridgepasses a completed fire-and-forget task through untouched instead of rewrapping it — strictly fewer allocations for the plain transaction too.Verification
Full net10.0 suite green (7,628 tests) against the docker topology, including the retry acceptance suite (
WithRetry_Transaction_*: LOADING replay, watch-conflict budgets, group failover mid-transaction) and the transaction/queued-result behavioral pins. Full traversal build across all TFMs with analyzers: 0 warnings.