Repository navigation
WithRetry, WithKeyPrefix and the group database ride the context/executor core - #3338
Merged
Merged
Conversation
…utor 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
… 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.
First of a two-PR series (followed by the transaction-retry PR) replacing the old-API wrapper databases with context/executor composition. Net: +691 / −7,177 across 44 files.
What goes
RetryDatabase:WithRetry(IDatabaseAsync)now re-contexts viaRespRetryExecutor, a decorator on the executor chain that replays the already-rendered frame — no capture, no generated per-command state. asyncState is now carried (previously dropped); a foreignIDatabaseAsyncpasses through unchanged.RetryTransactionstays for now, built byCreateTransactionwhen the chain retries — it goes in the follow-up PR.KeyPrefixed*family (KeyPrefixedDatabase/KeyPrefixedBatch/KeyPrefixedTransaction/KeyPrefixed<T>+ partials):WithKeyPrefix(IDatabase)re-contexts viaAppendKeyPrefix+AppendChannelPrefix, so the prefix is applied at render time. A foreignIDatabasenow throwsNotSupportedException— silently not prefixing is the wrong keyspace.MultiGroupDatabase's 628 generated members: it is now a slimRedisDatabasesubclass overRespGroupExecutor. Scans pin a whole enumeration to the member active at call time (a cursor only means something to the server that issued it);GetDatabase(-1)resolves from configuration, with members validated to agree onDefaultDatabase.Gaps closed along the way
The render-time prefix misses anything that doesn't travel as a key hole, so the places the old wrapper handled specially are handled explicitly:
@keyin aLuaScriptbecomesARGV[n]— a value. Extraction prefixes both the KEYS array and the ARGV copies, and the send goes through an unprefixed context view (WithoutKeyPrefix), else KEYS would be double-prefixed. This was a real bug caught by the existingScriptingTests.BY/GETpatterns map under the prefix (nosort/#exempt), as the old wrapper'sSortByToInner/SortGetToInnerdid.NotSupportedException), matching the old wrapper.IsConnected,IdentifyEndpoint*,GetConnectionState) ask about the prefixed key.AppendKeyPrefixnow carries the topology cell + cache resolver it previously dropped.Tests
The five member-by-member NSubstitute wrapper files (~4,100 lines) are replaced by
KeyPrefixContextTests: RESP-measuring tests over theFakeExecutorharness that assert the actual bytes on the wire (prefix composition/nesting, SORT patterns, script KEYS vs ARGV, refusals, routing, the async surface). Full net10.0 suite green against the docker topology.