Conversation
mgravell
changed the title
mgravell
changed the title
mgravell
force-pushed
the
marc/interpolated-writer-design
branch
from
September 14, 2026 13:12
08633b2 to
24ada5a
Compare
Fifty pragma pairs, all with the same justification, were drowning the command declarations they were attached to. One suppression on the partial class, with the reason written out properly: Every member is an extension method whose first parameter is a group type, so two members sharing a name are only candidates for the same call when their receivers are the same group - and within a group the overloads differ in a parameter that has no default. The names repeat across groups on purpose; ctx.Strings.Length and ctx.Sets.Length are the same word because they are the same idea, which is the whole argument for grouping.
The library's biggest group, and the one with the most formatting the wire cares
about and the caller does not. All of it is shared with the MessageWriter path
rather than restated - GetRange for a score bound's '(' prefix, GetLexRange for
'['/'-'/'+', ReverseLimits for putting a range into the direction its command
walks. A second copy of a bound convention is a silent off-by-one-bound waiting
to happen, and this is the group where that would be hardest to notice.
Where the overloads collapse: SortedSetAdd has six (three condition arities
across single and multiple members), SortedSetUpdate is the same command with
CH, and SortedSetIncrement/SortedSetDecrement are the same command again with
INCR and a sign. Three group methods take all of them - `change` and the
condition are parameters, because on the wire that is exactly what they are.
New spellings of things the surface already knew how to say: SortedSetEntry
implements IRespArgument, so a whole run of members is one hole - writing score
then element, which is the reverse of how the type reads and the one entry type
whose wire order is not its declaration order. RespSortedSetOptions carries
ZADD's six option tokens AND its retry category, because both depend on the same
flags: NX/XX/GT/LT converge on replay, but INCR compounds unless NX makes a
replay a no-op.
Two bugs the re-run of SortedSetTests found, both worth the telling:
**`default(RespLimitRange)` is (0, 0), not "no limit".** Written where "no
window" was meant, it rendered `LIMIT 0 0` and quietly returned an empty range -
36 tests, all of them about ranges. The sentinel is now a named `None`, because
a zero value that means something else is exactly the kind of thing that reads
as correct.
**`Aggregate` has a fourth member.** Sum/Min/Max looked like the whole enum;
Count is real, and the switch threw for it.
SER352 is down from 432 unimplemented members to 354.
Their nine commits (an open span hole, then hashes, sets and sorted sets with their reply handlers, byte-level pinning and re-run proofs) against my three (RespResult sharing its buffer, and two queue entries). A merge rather than another rebase: they had already rebased onto 30d28d7 and I committed three times after that, so rebasing again would just move the target once more. Merge-tree reported no conflicts before this was run.
Replaces a typeof(T) == ladder that had reached 25 entries and that nobody was
obliged to extend. One DefaultHandlers object carries every default as an
explicit implementation, and the lookup is:
private static IRespHandler<T>? Resolve() => DefaultHandlers.Instance as IRespHandler<T>;
memoized in the same static field as before, so it still runs once per closed
T. A handler that compiles is a handler that is reachable; there is no ladder
to keep in step and no list to forget.
It also makes "two defaults for one result type" a COMPILE error, which
neither a typeof chain nor a registration array manages - both pick silently.
That is not hypothetical here: bool has Boolean and Success, RedisValue has
Value and SingletonValue, Lease<byte> has Lease and SingletonLease. Exactly one
of each lives on the singleton; the alternates stay as their own classes and
are named explicitly by the commands that want them, which is the honest way
to say "not the default".
IRespHandler<T> and IRespPayloadHandler<T> lose their covariance, which this
depends on: `as` honours variance, so IRespHandler<out T> would let a string
handler answer SendAsync<object>. Checked that nothing relied on it.
Explicit implementation is forced rather than chosen - one Parse per result
type, differing only by return type, is not a legal implicit overload set - and
conveniently keeps 25 Parse methods off the type's own surface.
Two things the merge surfaced that separate classes had hidden: two handlers
each had a private helper called Shape, now named for what they shape; and two
class-level doc comments were orphaned when their classes moved, one of which
was already stale (it still claimed RespResult copies).
RespSurface.cs: 600 -> 535 lines. 6933 tests green.
Removing the completed handler-registry item sliced from its heading to the next one, and stale-while-revalidate sat between them. Restored with the decisions that were already made - the two thresholds, where the once-only flag lives, the read-your-own-writes carve-out for invalidation-SWR, measuring the window from first notice, and the cap for the hot-written-key case - so none of it needs re-deriving. Caught only because it was asked about, which is an argument for the queue being a file rather than a conversation.
CachePolicy.RefreshAfter is the soft threshold, TimeToLive the hard one. A read between them is served AND claims a refresh, which runs on the thread pool while the caller already has an answer - so an entry never goes from good to gone in one step, which is the moment every concurrent reader of a hot key misses at once. The refresh needs nothing configured: the cache key IS the request, so refreshing means re-sending it. No factory, no captured state, nothing of the caller's retained, and handler-agnostic because the cache stores raw bytes. That is what HybridCache's (TState, Func<TState,TResult>) shape exists to work around, and here it falls out of the design rather than being built. Off by default. Serving a value already known to be old is a choice about correctness, not a tuning knob. Three things that were not obvious until it ran: - the claim has to be ATOMIC with the lookup. TryGet decides "this is ageing" and "you are the one who will fix it" together, and tells exactly one caller; deciding those separately lets every reader past the threshold decide both, which is the stampede in different clothing. - a refresh must REPLACE, not add. TryComplete used TryAdd - right for a first fill, where losing the race means someone answered first and their answer is as good - which made every refresh a silent no-op that still counted as a redundant fill. - swap in place, not remove-then-add. TryRemove returns the value but not the stored KEY, and the key holds a retained request; removing strands that reference, and disposing our own copy instead releases the wrong one. TryUpdate leaves the dictionary's key alone, so only the superseded reply needs releasing - watched by a reference-count test. A refresh takes no in-flight registration: it is not something to wait for. The entry is still being served, so a concurrent miss is asking a different question and should fetch rather than queue. Mutation-tested three ways: no once-only claim, refresh adding instead of replacing, and the superseded reply never released. Also makes RespTrackingTests.TheKeyBytesTheServerSendsAreTheOnesWeRecorded assert its actual claim - that the server named the bytes we wrote - by recording the key names the push carried, rather than inferring it from cache state. Any concurrent FLUSHDB sends an unfilterable flush push that can invalidate the entry before it is stored, which made the old assertion intermittent.
The other half of SWR, and the more valuable one. Age is staggered across entries; an invalidation lands for EVERY reader of a key at the same instant, which is the thundering herd exactly, and no amount of time-based smoothing helps because the trigger was not time. CachePolicy.InvalidationGracePeriod moves the entry's hard expiry to "now plus this", measured FROM THE INVALIDATION. A key under constant access bridges the burst; a key nobody is reading simply expires, because nobody arrives inside the window. The first version measured from first notice instead, which is cheaper - no timestamp needed on the invalidation path - and wrong: it would serve a key invalidated an hour ago to whoever read it next, resurrecting something nobody wanted rather than bridging a burst. Now anchored to the invalidation, with a test for a key nobody reads, and the mutant restoring first-notice fails it. The cost of that is a Stopwatch.GetTimestamp() in OnInvalidate, which is otherwise a few nanoseconds wide and sees every key the server mentions under BCAST - so it is conditional on the policy having asked for a grace period, and the default path is untouched. Read-your-own-writes is enforced structurally rather than by convention: OnLocalWrite stamps the key node with a monotonic ticket, and the grace is refused if any dependency carries a local-write ticket newer than the generation the entry recorded. Monotonic because our own write also echoes back from the server as an ordinary invalidation, and the fact that WE wrote it has to survive that - tested in that order. The window doubles as the cap. Testing that needed the refresh to FAIL: a successful refresh heals the entry, so the first version of that test passed whether or not a cap existed. Now the refresh gets an error reply, which TryComplete refuses, and only the cap can end the stale serving. Mutation-tested four ways: the read-your-own-writes gate, the cap, a later server invalidation erasing the local-write fact, and the grace anchoring.
cache.FlushOnDisconnect(multiplexer) hooks ConnectionFailed and empties the cache; the returned IDisposable unsubscribes. Not a tidy-up. Server-assisted invalidation only works while somebody is listening: anything that changes during a disconnect is never announced, because the server forgets a client it has lost. An entry that survives the gap is wrong with nothing left in the system that will ever say so. There is a test asserting that failure WITHOUT the hook, because watching it happen is more convincing than asserting it would. Flushes on any connection failure rather than reasoning about whether that particular one was carrying invalidations. Over-flushing costs a round trip per key; under-flushing serves wrong data with no bound on how long for, which is the direction this design errs in everywhere else. It does not cover a socket that is quietly dead, because no event is raised for one. That is what CachePolicy.TimeToLive is for, and is the concrete reason it must never be infinite: the hook handles detected failure, the lifetime bounds undetected failure. Explicit for now because the cache hangs off a context rather than being owned by the multiplexer; it becomes part of constructing a cache-aware database when GetDatabase() returns one. A cache nobody remembered to wire up is a cache that goes quietly wrong. Mutation-tested: not flushing, and Dispose not unsubscribing.
The cache had no owner in src: tests built one and attached it per context with WithCache, which is precisely what stopped an invalidation push having anywhere to go. It now hangs off the multiplexer - created in the constructor when ConfigurationOptions.ClientCache names a policy, never replaced, handed to each database's context as it is built. Per-multiplexer is forced rather than chosen: tracking is per connection, and a connection belongs to the multiplexer. PhysicalConnection gains PushKind.Invalidate and handles it before the channel gate, because that gate demands an inline string second element and an invalidation's is an array or a null. It always returns Handled, including with no cache: an invalidation is never the reply to anything we sent, so falling through to command matching would hand it to whoever was at the front of the queue. Anything unreadable over-flushes rather than guesses. OnConnectionFailed flushes first thing - before the disposed check, before handler dispatch, and synchronously - because queueing it leaves a window in which we answer from a cache we already know is suspect. The policy is not part of the connection string: it is durations and correctness choices rather than a name, and null (no cache) is the only safe default. Still by hand: CLIENT TRACKING itself. Negotiation is the next item, and until it lands a caller who sets ClientCache and stops there gets a cache that fills, expires on TTL, and is never invalidated.
Under BCAST the server announces only keys matching a PREFIX, so an entry whose key matches none of them has no invalidation path: nothing will ever say it is wrong, and it is served until TimeToLive alone retires it. That is the RefusedNoKeys argument reached from the other side - there the request declared nothing to depend on, here it declared something the server was never asked to watch - and it gets the same answer. CachePolicy.Prefixes carries the list; TryBeginFill refuses anything outside it, counted as RefusedNotTracked. Every key must be tracked, not merely one: the entry depends on all of them, so one untracked key makes the whole reply uninvalidatable. Matching is on the bytes as written to the wire, which is what the server matched and will name back. An empty list means "everything"; an empty string is rejected, because "" would silently turn a narrow list into a total one. Overlap is rejected at construction because CLIENT TRACKING rejects it at the handshake. The list lives on the policy so that the set the cache admits and the set the server agreed to announce are one set - and so the PREFIX arguments come from here once negotiation lands. This deletes a "control" from the integration test that asserted an out-of-prefix key kept serving a stale value. It did prove the cache was in the path; it also enshrined the bug.
The store side is merely impossible: no reply is observed, so there is nothing
to keep and no way to run the error check. The probe side is the one that
matters. Fire-and-forget promises the caller default; a cache hit would hand
back a real value, so the same call would answer differently depending on
whether something else had happened to read that key first. A cache may make a
call faster - it may not make it return something else.
One mask test alongside NoClientCache, so both suppressors cost a single AND.
The defensive "no reply is coming" branches in RespExecutor stay, but are
re-commented: fire-and-forget no longer reaches them.
Separately, and on the same flag: RespMessageExecutor.Send turned the pipeline's
default - null, for fire-and-forget - into throw new RedisException("No reply."),
so every synchronous fire-and-forget command on this surface threw. The async
twin always passed it straight back; the two now agree.
FakeExecutor honours the flag too. One that answers a reply the caller declined
makes every assertion about it meaningless, which is how the first version of
these tests managed to fail for the wrong reason.
Two kinds of setting were sharing one type. The test for which is which: does it change what we send or what we record, or only how we interpret what we already hold? The first is a fact about the connection and is settled once - prefixes are the clearest case, since they ARE the argument list sent in CLIENT TRACKING and the server was only told once. The second is applied when an entry is read and never stamped when it is stored, which is what will make it safe to vary per call: the entry is shared, so one copy has to serve callers with different tolerances. So CacheOptions takes Prefixes and Enabled, gains DefaultPolicy, and becomes what ConfigurationOptions.ClientCache holds. CachePolicy keeps only entry behaviour - lifetime, refresh threshold, grace period. No behaviour change; the per-context override is the next step, and the upcoming memory budget now has a home that will not have to move.
Two gaps, both of the same shape as the cache having no host: machinery built and tested against fakes, with nothing in src driving it. Sweep() had no caller outside tests, so nothing ever reclaimed a dead entry. It now runs from the multiplexer heartbeat, with the cadence inside the cache so the driver does not need to know its business and a test can call it directly. SweepInterval is a cost knob, not a correctness one: a dead entry is already refused on read. Sweep also only dropped INVALIDATED entries. An entry that merely aged out is refused when read - and for a key nothing comes back for, nothing reads it, so nothing removed it. That is exactly the entry a lifetime cannot help with, because nobody is there to notice it has passed. MaxPayloadBytes is the cheapest bound there is - the reply's size is known before anything is stored - and large replies are both the fastest way to spend a memory budget and the least likely to be read again. Default 1MiB; null for no limit; refusals counted, because a reply silently not being cached should be answerable without a debugger. One mutant survived and is recorded rather than hidden: claiming the sweep timestamp after the work rather than before. The compare-exchange is a cost property, since overlapping sweeps are already safe - only the caller that wins TryRemove disposes anything - and the window is too narrow to assert on. The test now pins the safety it can prove instead of the collapse it cannot.
The two modes are a trade, not a ranking: broadcasting costs the client noise, per-key tracking costs the server memory, and which is cheaper depends on whose resource is scarce - not something a library can know. An enum rather than a bool because the server has two further modes, OPTIN and OPTOUT, and a boolean cannot grow to hold them. They are absent rather than present-and-throwing for an honest reason: both are driven by CLIENT CACHING YES|NO applying to the next command on that connection, which a multiplexer gives a caller no way to control. Offering them needs the pipeline to write the pair atomically, as it already does for a transaction. The enum earns its keep immediately: PREFIX is broadcast-only, so the two settings constrain one another. That check cannot live in an init accessor - an object initializer assigns in the order the caller wrote, so the rule would pass or fail on line ordering - so it runs when the options are first used for something, which is the only point the whole object exists.
MaxPayloadBytes bounded one entry; nothing bounded the total, so a cache of many small replies grew without limit - and each entry pins a pooled array, so the memory is not only heap. Counted as memory HELD, not bytes carried. Each reply is copied into its own rent from ArrayPool<byte>.Shared, which serves from power-of-two buckets, so a 33-byte reply holds 64. Budgeting on payload lengths would under-report by up to a factor of two - the error that lets a quota fail to bind under exactly the workload that needed it to. MaxEntries is not redundant with MaxBytes: many tiny replies spend almost no memory on payloads while still growing the entry and tracked-key tables, whose cost a byte budget cannot see. Eviction samples rather than tracking recency. True LRU needs the moment of last use, which means a write on every READ - and a read is the one path here that is currently free: a dictionary lookup and a refcount bump. Redis reached the same conclusion about its own keyspace. Two details that are easy to get subtly wrong, both now commented: the sample start is chosen so a whole sample is always available, since stopping at the end of the enumeration under-samples the front of the table; and the cursor advances per call rather than coming from the clock, because a burst of evictions outruns Environment.TickCount and would be handed the same window over and over. Dead entries are taken before live ones. The sample is walked anyway, so an already-invalidated entry costs nothing to release and, unlike a live one, nobody wanted it. Eviction runs after the store, so a budget is a target the cache returns to rather than a wall - briefly overshot by one entry. Refusing instead would throw away a reply already paid for in full. The loop is bounded by the entry count rather than by "until it fits": under concurrent stores, evicting for ever is a worse failure than being briefly over budget. Byte accounting funnels through one release method, called only by whoever won the TryRemove, so the count cannot drift and a payload cannot be released twice - which would hand a live buffer back to the pool.
32 array occurrences on the experimental surface, 29 of them ValueTask<T[]> returns, inherited from a surface that had no alternative. The inputs were already done right as ReadOnlySpan<T>, so this is one-sided - and it is the side that allocates per call with nothing able to reclaim it. Recorded with the reasons it should not wait: T[] can never become anything else without a binary break, so the experimental window is the only chance; and every command group added in the old shape is more to undo, while groups are being added now. Also records the one genuine exception - RespAttribute, whose arguments must be arrays because the CLR gives no choice - so nobody "fixes" it later.
Records why the obvious bridge does not work, before someone builds it: an internal "hand me your buffer" hatch on ReadOnlyLease<T> can essentially never fire, because Rent goes to ArrayPool<T>.Shared and returns an oversized array while the old contract promises an exactly-sized one the caller owns. So the variant is not a method on the lease but a question about how the result is built - which is a question about the handler. Preferred shape is one command factored into an internal core taking IRespHandler<TResult>, with the public method passing the lease handler and the transitional adapter passing the array one: command written once, legacy path allocating exactly what it always did, and no sharp edge on a type whose point is unambiguous ownership. The Adopt/TryDetachArray variant is recorded as the fallback rather than dropped.
Correcting the shape recorded yesterday. Not a generic core taking a handler, and certainly not an escape hatch on the lease: a sibling extension method on the typed context, internal, sharing the message construction and differing only in the handler. Three things that buys. It keeps the classic `this` extension form, so the legacy sibling retires the way everything else on this surface does. Being internal it never reaches the public API, so it adds no array site to fix later. And TransitionalDatabase stays a genuine one-line pass-through, which was the point of that class. Sharing the construction wants the Execute -> Render rename first: Render is exactly the primitive both siblings need, since each call wants its own frame and what is shared is the composition rather than the frame.
RespContext.Execute returned a rendered frame and dispatched nothing, while
IDatabase.Execute in this same library sends a command and returns its result.
Two opposite meanings for one verb, in one codebase, is a trap for every reader
after the first - and it kept the ad-hoc surface on ExecuteAsync only, because
async had no clash.
134 call sites, all in tests and benchmarks: src had none outside the context
itself. Renamed by letting the compiler name the lines rather than by pattern,
because ".Execute(" also matches IDatabase/IServer.Execute, which must not move.
Also serialises RespCacheInvalidationTests against RespTrackingTests. A FLUSHDB
sends an UNFILTERABLE flush push to every tracking client on the server, so the
flush test was evicting entries out from under the other class's assertions -
the design working exactly as documented in 6.13, and a test interfering with
another test. Both are now in NonParallelCollection.
Prerequisite for the lease returns: ReadOnlyLease<T> began life as ReadOnlyLease<byte>, where returning a pooled array unwiped is correct and clearing would be pure cost. The moment T is RedisValue, HashEntry or SortedSetEntry that stops being true - a returned array still points at everything that was in it, so each of those objects stays reachable until the buffer happens to be rented and overwritten. It never throws; the heap just quietly fails to shrink, which is much harder to find than a crash. RuntimeHelpers.IsReferenceOrContainsReferences answers this exactly but does not exist on net461/netstandard2.0, so down-level errs towards clearing: a needless wipe costs a memset and a missed one costs retention, which are not the same size of mistake. Both directions are pinned - never-clear fails the reference test, always-clear fails the primitive one - because this is a decision, not a default.
…ling The worked example for getting arrays off this surface. Strings.Get(keys) now returns ReadOnlyLease<RedisValue>, which the caller gives back, and the array form lives on an internal GetArray that TransitionalDatabase calls. A sibling rather than a conversion, and that is the whole point: IDatabase promises an array the caller owns, so bridging through the lease would rent a pooled buffer only to copy out of it and hand it straight back - strictly worse than allocating the array in the first place. Two handlers over one command costs one duplicated interpolated line and no knowledge. Internal because it serves a shape on its way out; when that goes, so does this, with no binary consequence because nothing outside the assembly could ever bind to it. What this saves is the array, not its contents: RedisValue has no lifetime and cannot be given one, so the elements still allocate. On a large MGET the array is the part that reaches gen 2. 28 array returns left, all the same transformation.
Raw bytes off one socket against Redis 8.9.241, RESP3, BCAST PREFIX, so the order on the wire is the answer rather than an inference. Key invalidations trail their replies AND are accumulated across the write cycle rather than per command: two pipelined SETs produce one two-key push after both +OKs. MSET of three keys is one push of three. FLUSHDB is the exception - its invalidate null precedes its own +OK. The consequence that matters: a self-invalidation can never protect read-your-own-writes. SET k v followed by GET k, pipelined, returns the read BEFORE the notification about the write - so a cache holding a stale entry would answer from it and be corrected afterwards. OnLocalWrite is therefore the only mechanism that can close that window, which makes its missing caller in src a correctness gap rather than a nicety. Queued as such. Also: expiry produced no push at all, neither passively after the TTL nor when a later read forced the deletion. That is the case CachePolicy.TimeToLive exists to bound, and the argument now has evidence rather than only reasoning.
OnLocalWrite had no caller in src, and the timing measurements make that a correctness gap rather than a missing optimisation: a write's own invalidation arrives after its reply, and after the replies of anything pipelined behind it, so SET k v followed by GET k returns the read before the notice about the write. Until now that read was answered from a stale entry and corrected afterwards - handing a caller back the value they just replaced, which is the one kind of staleness this design refuses. Stamped on send rather than on reply, because a read's dependencies are captured when it is sent: a stamp landing after our reply can be overtaken by a read issued in between, which would then validate and be stored. Stamping first widens the window, and a window that is too wide costs a miss. If the write then fails we invalidated for nothing, which is the right way to be wrong. Hooked outside the "may I cache this?" gate, because a write is exactly what that gate excludes - which is why nothing called it before. An undeclared retry category counts as a write, matching the judgement the caching side already makes from the other direction: undeclared cannot mean safe, so there it means "do not cache" and here "assume it wrote". A write whose keys cannot be enumerated flushes everything, because a write is precisely where guessing is not allowed.
The previous note said HashImport "needs a connection-local PREPARE injected ahead of it, which is a property of the write path rather than of the command". True, but it points at the implementation rather than the constraint, and it invited the wrong comparison. SELECT is the tempting sibling, because it shares the injection mechanism - and it is a red herring. A database index is a register the client mirrors and is the sole author of, so it can never miss. EVALSHA is the real sibling: an optimistic short reference to a named, cached payload, where the client's belief that the payload is present can be wrong. HIMPORT SET is the same shape. The comparison earns its keep by breaking in one specific place. EVALSHA has a self-contained fallback - EVAL carries the whole script - so a NOSCRIPT is recovered by re-rendering a single message, which is why ResultProcessor deliberately keeps the request buffer alive on that one error. HIMPORT SET has no such form. HSET is NOT it: HIMPORT replaces the hash at the key where HSET merges into it (pinned by ExistingHashIsReplacedNotMerged), so the inline expansion is DEL plus HSET - two commands, not atomic, different failure profile. Recovery is therefore inherently two ordered commands on one socket. Which makes the current design better than the old note implied. Injecting inside the write lock is not a workaround for a missing preamble concept: it is the only point at which the connection is known and nothing has been written, which is exactly the window that recovery needs - and acting there is what lets a field-set avoid pinning a connection at all. A frame surface has no such point, so expressing this outside the bridge would need CONNECTION affinity across a retry, where CommandServerSpecific pins an endpoint. So: EVALSHA is portable to the frame surface (it needs the frame to carry an alternate rendering - real work, but nothing from the connection); HIMPORT is not, until something can say "this retry, that socket".
The first version of the local-write hook answered "key marks overflowed" with OnFlush. Correct, and far too blunt: a frame can only mark keys up to argument 62, so MSET or DEL past that would destroy an unrelated hot cache every time - and bulk writes are exactly the workload that would suffer. Reporting a subset of the keys is forbidden, which is why the frame refuses to report one at all. But the arguments are a SUPERSET of the keys, so stamping all of them is sound and stays inside the command. The cost is one needless miss for any value that happens to equal a cached key, on a command that already named enough keys to overflow the bitmap. Only writes reach this. A 100-key MGET is a read, so it never touches the local-write path; it is simply not cached, by the same keyless rule, because a frame that cannot name its keys cannot be invalidated either.
SER352 goes 58 -> 52. Two signature changes, both taking the shipped spelling's cost into the adapter rather than the new surface. RestoreAsync takes a ReadOnlySpan<byte> where IDatabase.KeyRestore takes byte[]: the payload is written straight into the frame, so nothing here needs an array, and a caller holding a slice pays nothing. MigrateAsync takes host and port where the shipped signature takes an EndPoint and immediately picks it apart with Format.TryGetHostPort, throwing for anything that is not host-and-port - so the EndPoint buys a conversion and a failure mode rather than expressiveness. The span payload needed a new hole: AppendFormatted(ReadOnlySpan<byte>) frames the bytes as ONE argument, where the other span overloads write an argument per element. byte is the case where the span is the argument, which is what any command taking an opaque blob wants; without it the caller loses the interpolated spelling or copies into a RedisValue for no reason. MIGRATE is why the parity tests matter here: its key is the THIRD argument and its two optional tokens come last, so an independent rendering agreeing is worth more than a string copied across. Four option combinations plus the TimeSpan.MaxValue-means-no-expiry rule for RESTORE, all byte-identical.
SER352 goes 52 -> 48, with no new public API: the shipped implementation says these are sugar, and it is right. LockTake IS StringSet with When.NotExists and LockQuery IS StringGet, so the adapter composes over commands the context surface already has. A lock is a pattern over SET and GET rather than a command, and the new surface should not pretend otherwise by growing LockTakeAsync. The null-token guard moves with it, and matters: SET deletes the key for a null value, so without the check LockTake would delete the lock rather than take it. LockRelease and LockExtend stay unmoved deliberately. Each is a single IFEQ-style message on a new enough server, a TRANSACTION otherwise, and a plain DELETE where transactions are unavailable. The context surface has no transactions, so moving them today would mean dropping the fallback or reimplementing it - which is a decision, not a translation.
Both were doing their job, and I pushed before reading the result - the commit was chained to a grep that matched "Failed!" as happily as "Passed!". AnUnmovedCommandThrowsAndSaysSo names a command that has genuinely not moved, and its own comment had predicted LockQuery would last "because the lock group waits on transactions". Half of that group did not: LockQuery is GET and LockTake is SET NX. The exemplar is now LockRelease, which does wait - it is one IFEQ-style message on a new enough server and a TRANSACTION otherwise. EveryImplementedMemberBelongsToATestedGroup requires an implemented member to be covered by a group's tests, and the lock members belong to no group because they are sugar. Listing them needed a real test to point at rather than an exemption, so TheMovedLockMembersAreSugarOverSetAndGet pins that LockTake renders SET k token NX EX 30 and LockQuery renders GET k, plus the null-token guard - without which SET would delete the key and the lock would read as taken-then-vanished. LockRelease and LockExtend are deliberately NOT listed, so they keep failing that test until they move.
SER352 goes 48 -> 36: all sixteen ScriptEvaluate members, RedisResult and RespResult shapes alike. The bridge that unlocks this is RedisResultHandler, and it cost almost nothing: RedisResult.TryCreate takes a PhysicalConnection that turns out to be vestigial - only threaded through the recursion as state, never read, already nullable - so a handler with no connection to offer passes null and reuses the shipped parse rather than writing a second one. The private script flow becomes generic over the reply shape, so EVAL, the SCRIPT LOAD preamble and the registry path all serve RedisResult, RespResult or anything else from one implementation. The important part is what the adapter deliberately does NOT do. The shipped string overload chooses EVALSHA when the string looks like a SHA1 and otherwise sends EVAL with the body, every call, uncached. Scripts.EvaluateAsync instead does SCRIPT LOAD then EVALSHA and keeps the rendering - better, but not the same thing on the wire: it adds a load to the first call and leaves the script in the server's non-evictable cache. Changing that for existing callers is a decision rather than a translation, so EvaluateDirect keeps what they have and new code gets the caching path by calling EvaluateAsync. LuaScript and LoadedLuaScript needed nothing: they extract their parameters and call back through IDatabase, which is this object, so they travel the new path the moment the string and hash forms do. Also adds EvaluateHashAsync to the new surface - EVALSHA from a span, no preamble - because a caller holding only a hash cannot repair a NOSCRIPT, and that is exactly what the shipped byte[] overload has always meant.
RespContext was a 40-byte readonly struct copied into every group wrapper, every builder and every clone. Four of its members - Cache, ScriptCache, ChannelPrefix, MaxCacheAgeTicks - were properties that walked the service chain on each read, and the executor consults two of them on EVERY send; the walk calls Type.IsInstanceOfType per link, a reflection type test rather than an `is T` the JIT turns into a cast check. Database chased the executor. As a class those all resolve once, in the constructor, and become field reads; the chain remains for the niche lookups not worth a field. A flavoured context is now a reference with an accent, and the "have I built it" flags in RedisDatabase/RedisServer collapse to `??=`. KeyPrefixed memoises too: it rebuilt per access, which was a free struct copy and is now an allocation. The public surface is cut to what an external caller (NRedisStack et al) needs to issue a command - Compose, Render, the constructor. CommandMap, KeyPrefix, ServerType, TryGetService and the withers are internal. `in RespContext` is dropped throughout: pointless on a reference type, and RS0036 rejects it on a public signature. MEASURED, and it is not a win on the cache-hit path. A/B by stashing, --inProcess --job medium, KeySize 8, ns: 2. render only 39.3 -> 38.8 6. context.Send 113.8 -> 112.8 5. context.SendAsync 118.8 -> 115.3 4. db.Strings.GetAsync 125.6 -> 133.3 The memoisation works - the send core is flat to slightly faster, render is unchanged - but the group-accessor layer goes 6.8ns -> 18.0ns, and the same appears at 64/128/256 (+12.1, +9.7, +8.6). Dropping the now-pointless `in` on the 8-byte group structs was tried and changed nothing. Cause not yet found; it is ~10ns against a 30-100us round trip, and only visible because this benchmark serves locally, but it is a real regression in the layer that should be free.
SER352 is at 36. The seven hand-written members that forward to Fallback<T>() are invisible to it for the same reason the scans were: the generator cannot tell a real implementation from a forwarding throw. Scans, Scripts and Keys are done; VectorSetRangeEnumerate stayed behind because it is keyset pagination over VRANGE, not a cursor scan.
Twelve of the 36 SER352 members: two IDatabase.StreamRead overloads and four StreamReadGroup ones, each with an async twin. They collapse to one group method each, because the older spellings differ only in which of maxCount/maxSize/noAck/claimMinIdleTime they expose. RespMultiReadReply is one field for two wire shapes. RESP2 answers an array of [name, entries] pairs and RESP3 a map of name to entries, and RespPairAggregate with allowJagged reads both - "every child is a two-element aggregate" is exactly what distinguishes the array form - so this needs no protocol, no flag and no second field. The render is imperative rather than interpolated, because the wire wants the span twice: every key, then every id. An interpolated hole writes one run per element, so the keys-then-ids shape would need two temporary buffers or new vocabulary for something only these two commands have. Compose is what the codebase already reaches for when the argument count is data-dependent. MultiStreamProcessor's inlined walk becomes ResultProcessor.ParseRedisStreams, which the reply object's ToArray also calls - as ParseStreamWithNameSkip already does for the single-stream shape - so the deferred and materialising paths stay two call sites of one parse. That retires RedisStreamInterleavedProcessor. Parity tests render both surfaces and compare bytes. MultiReadPutsAllKeysBefore AllIds is the one worth having: a rendering that walked the positions once would interleave keys and ids and still look plausible, and one stream cannot tell the two orders apart. MultiReadAcceptsNewMessagesUnlikeTheSingleStream Overload pins the logged shipped bug from the other end - $ is rewritten to >, the consumer-group token plain XREAD does not understand. SER352 36 -> 24.
The last six of the stream family. The three processors keep their shape but lose their bodies: ParseStreamConsumerInfo, ParseStreamGroupInfo and TryParseStreamInfo are now statics that both the classic processor and the new handler call, as ParseStreamWithNameSkip and ParseRedisStreams already are for the read shapes. InfoAsync returns a materialised StreamInfo where the read shapes return deferred windows, and that is deliberate rather than an oversight: the reply is a flat name/value map of about a dozen counts, so a window would re-read a frame header per property to save copying an int. The two StreamEntry members are the only part that allocates, and the caller asked for them. GroupInfoAsync and ConsumerInfoAsync return ReadOnlyLease<T>, with internal array twins for the adapter - the arrangement the geo group already uses, one handler instance implementing both interfaces. ConsumerInfoAsync writes the key as a KEY, where the shipped message passes key.AsRedisValue() in the argument array and routes with CreateInKeySlot. The slot was right either way, but the argument never went through the key machinery - harmless only because the KeyPrefixed decorators prefix the key upstream. A context applies its prefix at write time, so a prefixed context needs it to be a key; ConsumerInfoAppliesAKeyPrefix is the half the shipped message cannot do, and ConsumerInfoMatches shows the bytes are identical without a prefix. TransitionalCoverageTests.AnUnmovedGroupIsStillGenerated named "Stream" and started failing here, which is the control working: nothing Stream-shaped is generated any more. It names "Lock" now, which needs the transaction fallback and will be the last group standing. SER352 24 -> 18. The stream family is done.
Four members: Execute(string, params object[]), Execute(string, ICollection<object>, flags) and their async twins. The RedisResultHandler bridge already existed; what was missing was the render. Written out in the adapter rather than added to the context, because every awkward thing about it belongs to the old signature: an ICollection of object, a type test per argument, and a RedisResult tree at the end. ExecuteResp is what the same command looks like when the caller can say what its arguments ARE - which is why that one takes part in routing and caching and this one cannot, and why the new surface is not growing a version of it. The three-way branch is the shipped ExecuteMessage.WriteImpl, kept exactly: a RedisKey through the key path so it is prefixed and contributes a slot, a RedisChannel through the channel path so it is prefixed, anything else parsed as a value or refused. LegacyExecuteKeepsKeysChannelsAndValuesApart is the test that matters - a key that stopped being written as a key would lose its prefix, its slot and its cache identity, and nothing about the bytes would look wrong. The whitespace guard moves from ExecuteMessage into RespRequestBuilder's string constructor, which both ad-hoc routes share. ExecuteResp built a different message and so never had it: "ACL SETUSER x" went out as one unknown token and came back as an opaque server error. Now both refuse it. SER352 18 -> 14.
ArrayGrepRequest was the reason this one was left: it is a builder that renders its own predicates, and it only knew how to render them through MessageWriter. It now has a second spelling per predicate plus a WriteTail for the request itself, so either pipeline can write it. The two writers share no interface - MessageWriter is a class the classic pipeline owns, RespRequestBuilder is a ref struct - so the second spelling is a copy rather than a delegation. What is NOT copied is ArgCount, which moves off the message and onto the request: both writers need it, one for its header and one to size the buffer, and one count means they cannot disagree about how many tokens they are about to write. That leaves only the ORDER free to drift, which is what RespSurfaceArraysParityTests compares - ten request shapes, both renderings, byte for byte. ReversedSwapsTheBoundsAndOpenOnesAreTheTokens is spelled out rather than compared, because a swap both writers got wrong the same way would pass the parity theory and still be a wrong command. WITHVALUES decides the reply shape, so it decides the handler: pairs when asked for, a flat run of indices otherwise. Same fork the shipped path makes, from the same property. SER352 14 -> 12.
…3 -> 19) SER352 is at 12, plus the 7 hand-written members it cannot see. Streams, Execute and ARGREP are done; everything left is behind the plan Marc has for transactions, batch and identity - or is one of the three that turn out to need the same kind of thing: Publish picks a server per call, HashImport injects a PREPARE on the connection, StringGetWithExpiry is two commands in one message, and Ping times from inside the write path.
PingAsync() returns ValueTask and asks for nothing back, which is what most callers of a ping actually want: whether the server answers. PingMeasureAsync() is there for the ones who want the number. The handler is the clock, and is therefore the one per-call handler allocation on this surface: PingMeasureHandler takes its start timestamp in its constructor and reads the elapsed time when the reply arrives, so the duration needs no state threaded through the send. The alternative - an IRespHandler<TState, TResult> with the state handed to Parse - would put a type parameter on every handler in the library to serve one command. EachPingMeasureGetsItsOwnClock is what makes the allocation right rather than merely easy: a shared handler would carry one start timestamp, so the second measurement would be the time since the FIRST call, growing without bound and looking plausible the whole way. CAVEAT, and it is a real one. This measures from just before the send, where IRedis.Ping measures from the write: TimingProcessor reads TimerMessage.StartedWritingTimestamp, stamped inside WriteImpl, so the shipped number excludes whatever the message spent queued. A handler is handed a reader and nothing else, so it cannot see that instant. Identical on an idle connection, larger under a backlog - and IRedis.Ping/PingAsync are wired to it, so anyone alerting on this number will see it move under congestion. Arguably the more useful reading; still a change. The queue records both ways out: carry a write timestamp on the payload for exact parity, or put the two adapter members back on the fallback. SER352 12 -> 10.
Marc, on the fork raised with the split: "arguably on a backlog, the end-to-end time is what they actually care about". So neither of the two ways out - the behaviour stands as shipped in the previous commit, and the docs now say why rather than apologising for it. A caller waiting behind a backlog is waiting for the whole of it, so the end-to-end time is what they are actually experiencing; a ping that hid the queue would look healthy at precisely the moment it matters. The shipped IRedis.Ping has always documented its result as "the observed latency", which is this reading rather than the server-side one, so no interface doc changes.
Four members, and neither needed the new plumbing I claimed it did. VECTORSETRANGEENUMERATE is keyset paging, not a cursor scan - the shipped code avoids "scan" naming "in case a VSCAN command is added later" - so it gets RespKeysetEnumerable rather than RespScanEnumerable: the position is the last VALUE seen rather than a token the server hands back, which is also why it is not an IScanningCursor and does not pretend to be. Both faces, as the scans have, and the same two-token combining. The paging rules are the shipped ones: a short page ends the walk, and so does reaching the end bound - both avoid a final round trip that could only come back empty. HASHIMPORT I said needed write-path preamble injection the context surface did not have. It has had it since the scripts work: SendWithPreambleAsync plus IRespPreambleGate, and RespMessageExecutor implements IRespPreambleExecutor, whose FramePairMessage writes the pair "as one unit, so nothing interleaves and both reach one connection". So this is a gate, not plumbing. HashImportPrepareGate claims in IsNeeded rather than OnEstablished, inverting the interface's usual arrangement, and that is deliberate: it runs inside the write lock, which is the only place where "has this connection prepared it?" and "write it" are one decision, and a burst of imports issued before the first PREPARE's reply landed would otherwise each inject their own. Same reasoning and the same place as the shipped HashImportSetMessage.GetMessages. A claim that then fails to write dies with the connection, which starts empty. TransitionalScanGapTests stops listing what still throws and asserts the opposite: nothing in the uncounted enumerable family forwards any more. Its vector-set case and TransitionalDatabaseTests.AnUnmovedStreamingCommandThrows Too both fired when this landed, which is a tripwire doing its job - there is no unmoved streaming member left to point at, so the second is replaced by tests for the HIMPORT pair. Queued: caching the HIMPORT PREPARE rendering, which is composed per call and discarded unsent on all but the first per connection. The cache key has to notice a different command map. SER352 12 -> 8; 17 members left -> 13.
ctx.WithRetry(policy) returns a context whose executor is wrapped; everything downstream - the groups, the cache, the key prefix - is untouched and unaware. The loop is RetryDatabase.ExecuteAsync's, and every decision in it belongs to RetryController, which both paths now share so they cannot disagree. It is as much simpler as expected. RetryDatabase replays a METHOD CALL, so it generates a state struct per argument signature, owns pooled copies of any Memory<T> the caller might reuse, and disposes the capture when the last attempt is done. Here the thing replayed is a rendered frame that already exists: nothing to capture, nothing to map, nothing to own. Nothing to own in particular. The caller of IRespExecutor.SendAsync - AwaitUncached - holds one reference and disposes it in a finally AFTER the send completes, which for a retrying executor is after the last attempt. So the single caller reference already spans every attempt: a retain per attempt would leak, and a matched retain/release pair would be a no-op. The hazard that would need one - a message still holding the request after its task faulted, writing a buffer already back in the pool - is closed by the backlog dequeuing every timed-out message before completing it, and by a written message no longer needing the buffer. Noted in the queue as a thing to re-check if either moves. Two early exits, both handing the inner task straight back: a controller that can never retry (MaxAttempts == 1), and a send that completed synchronously and successfully. The first is exact because the attempt cap is tested BEFORE the policy is consulted. The bigger short-circuit - reading the command's retry category and skipping CommandRetryNever - is deliberately not taken: RetryPolicy.CanRetry is virtual, and a derived policy may ignore the category, so the executor would be deciding something the policy reserved. RetryDatabase.Context stops throwing: it decorates the inner executor and hands over both the controller and GetNextFailover, so the context path gets failover too. A retrying TRANSACTION still refuses, now with its own reason - a transaction is replayed as a unit, and a per-frame retry executor would re-send individual frames inside a MULTI. A synchronous send through a retrying context throws rather than quietly not retrying, which is the position the shipped retrying database takes by implementing IDatabaseAsync and not IDatabase.
Marc: "the can retry should also compare the flags". It does now - RetryController.CanEverRetry(CommandFlags) answers both halves of "could this ever retry?" before the send: the attempt cap, and the command's own retry category. What reaches the loop is a fault on a replayable command. The category half needed a decision rather than just a check. It was raised as unsafe, because RetryPolicy.CanRetry is virtual and a derived policy could retry a CommandRetryNever command - so an executor short-circuiting on the category would be deciding something the policy had reserved. The fix is to stop it being reserved: the veto moves out of RetryPolicy.CanRetry and into RetryController.CanRetry, above the policy call, so BOTH the database path and the context path apply it and an override cannot lose it. RetryPolicy.CanRetry now documents that it is not consulted for that category, and still repeats the test itself: it is public and virtual, so a caller may invoke it directly and a derived type calling base must get the same answer either way. The behaviour change is narrow and deliberate: a custom policy that used to retry a command categorised CommandRetryNever no longer can. That category means the caller said do not replay this, so the policy was answering a question that was not its to answer. ACommandThatForbidsReplayIsForwardedWithoutTheLoop is the test worth reading: the fixture's fault would OTHERWISE have been retried - a socket failure on a message that never left, so NotApplied would have bypassed the category cap - so a single send says the question was never asked.
ctx.CreateBatch() gives a queueing context and a handle that sends it. SendAsync accumulates; executing issues every queued send before awaiting any of them, then completes each caller's task with its own reply. NO GENERICS ARE NEEDED, which was the design question. The sketch was an untyped base plus Pending<T> : TaskCompletionSource<T> proxying the typed result out - but IRespExecutor has already erased the type: it deals in RespPayload, and the IRespHandler<T> that turns one into a T is applied by RespExecutor.AwaitUncached, above the executor and after it has handed the payload back. So a queued command needs a TaskCompletionSource<RespPayload> and nothing else. (The sketch could not have compiled regardless: a class cannot derive from both an abstract base and TaskCompletionSource<T>.) The good half is kept: the pending entry IS the completion source rather than holding one, so a queued command costs a single allocation. RunContinuationsAsynchronously is load-bearing - without it, executing a batch of a thousand runs all thousand callers' continuations inline on the flushing thread. No retain either, for the reason that now covers retry as well: an executor may use a request until the task it returned completes, and the caller holds its reference until then. A batched send completes at execute, after the bytes were written. Fire-and-forget is the one path that completes before the bytes are used - which is why FrameMessage copies for that case and only that case. The contract is written down in the queue rather than re-derived per feature. Discarding a batch faults what was queued, and that is not tidiness: the caller awaiting a queued command holds the only reference to its rendered frame and releases it when that task completes, so a dropped batch would strand a pooled buffer per command. KNOWN GAP, queued: this pipelines rather than batches. The sends are separate messages, so another caller's command may land between two of them; the shipped RedisBatch.Execute gets contiguity by grouping Messages per bridge and clearing the flush flag on all but the last, which needs the messages rather than the frames. Fire-and-forget also completes at execute here, where the shipped batch completes it immediately. Internal, as asked: the public trigger is undecided - an await using that executes on leaving would read better than an explicit call.
Marc: "we could pre-emptively TrySetResult, but I actually wonder whether two types that share an interface, with the latter having a static shared completed task". Two types sharing IPendingSend, yes - and the forgotten one needs no task at all, shared or otherwise. SendAsync answers a DEFAULT ValueTask, which is already completed and carries a null payload, and a null payload is what RespExecutor.Parse turns into default(T) - which is what fire-and-forget has always returned. So it allocates nothing beyond the entry, and would allocate nothing at all if the entry were pooled. Completing a promise nobody holds still costs the promise; declining to make one costs nothing. That is the whole reason this is two types rather than one with an early TrySetResult. AND IT IS THE ONE PLACE A RETAIN IS REQUIRED. The rule the rest of this surface relies on is that an executor may use a request until the task it returned completes, and the caller holds its reference until then - which is why neither retry nor an awaited batched send needs to take one. Answering EARLY is exactly what breaks it: the caller's finally disposes while the frame is still queued. So PendingForget retains, and releases when the batch is done with it. Confirmed load-bearing rather than assumed: removing the retain fails three tests. RefCountedBuffer throws on a span read after the last reference has gone, so the failure is loud rather than a read of somebody else's rent. The queue now states the contract once, with fire-and-forget named as its only exception and both existing answers to it - FrameMessage copies, PendingForget retains - so the next thing that learns to answer early knows which it needs.
Marc: "should it veto?" Yes. Retry exists to improve an outcome somebody is waiting for, and FireAndForget declares that nobody is. A replayed one cannot be observed to have helped, and the cost - a backoff delay added to a call advertised as returning immediately - can. RespBatchExecutor already makes the same point structurally: a fire-and-forget there is answered BEFORE it is sent, so nothing failing afterwards has anybody to tell. AGAINST THE REAL PIPELINE THIS CHANGES NOTHING, which is the part worth recording. A fire-and-forget message has no result box, so ConnectionMultiplexer.ThrowFailed swallows its write failure and ExecuteAsyncImpl ignores the WriteResult on the synchronous path: a fire-and-forget send cannot fault, so no retry loop was ever engaging for one. The veto says so out loud rather than depending on it - an executor that started faulting them would otherwise quietly begin adding retry delays to the one call shape chosen for not having any. It goes in RetryController.IsVetoed, alongside the category, so the database path and the context path agree; FireAndForget is in Message.UserSelectableFlags and so survives into FaultContext.Flags, which is what lets the same rule serve both the pre-send check on the request and the post-fault check on the exception. FireAndForgetIsNotRetried faults one through a fake, which is the only way to show the veto is real given the pipeline cannot.
… hatch Marc: "usually F+F is used for a side-effect, and usually retry is side-effect free, so I think it compounds to be a reasonable behaviour." Sharper than that, as it turns out. Fire-and-forget is reached for when a command is wanted for its side effect and its answer is not - a counter, a list push, a publish - and those categorise as CommandRetryWriteAccumulating, which sits ABOVE the default cap of CommandRetryWriteLastWins and is therefore refused already. So the commands people fire and forget and the commands the default policy would replay barely overlap to begin with; the veto mostly formalises an overlap that was already close to empty. Also recorded: "if someone complains, we can make it an explicit choice on the retry-policy". The shape already exists - MaxCommandRetryCategory is how the other veto is made configurable - so this stays a controller rule until somebody wants otherwise, which keeps the default the one that needs no explanation. Documentation only.
An OVERLOAD, not a new name: IRespExecutor.SendAsync(ReadOnlySpan<RespRequest>). SendUnit/SendBatch were considered and dropped - the name was carrying a meaning it did not convey, since "must resolve to a single slot" is a doc comment either way. The existing preamble overload becomes its N=2 case: a preamble plus a gate is just "the first frame is conditional". Records what makes it nearly free for transactions - FramePairMessage is an IMultiMessage, and PhysicalBridge expands one INSIDE the write lock, writing every yielded sub-command consecutively, which is exactly the contiguity guarantee; TransactionMessage is the same shape - and what is not free for batches: a batch may span bridges in cluster, which is why RedisBatch.Execute groups per bridge with SetNoFlush instead of being one message. Also retracts a suggestion of mine. Building the per-element completion on IResultBox<T> would aim at the transitional Message shim, which is to be gutted; the durable version is an IValueTaskSource-shaped completion, and ManualResetValueTaskSourceCore<T> is already used in RESPite across the same TFM set, so it works down-level. That would take a queued command - awaited or not - to one allocation and no task at all. Documentation only.
…asks The decided overload, built. IRespRunExecutor.SendAsync takes a run of requests and fills a span with one reply task each; RespMessageExecutor implements it with FrameRunMessage, an IMultiMessage that yields the earlier frames and then ITSELF - not a trick for its own sake, since only the messages a multi-message yields are enqueued for replies, and a wrapper yielding none of itself would never be written and never complete. PhysicalBridge expands it INSIDE the write lock and writes every sub-command consecutively, which is the whole of the contiguity guarantee and what TransactionMessage has always used. The slot is combined across the run, so "one connection" is checkable rather than hoped for: a run spanning two slots is refused by routing instead of being half-written to one server. Splitting a batch across bridges stays a layer up - it needs server selection, which an executor does not have. WHY IT IS NOT A ReadOnlyLease<RespPayload>, which was the question. Contiguity is a WRITE-side guarantee and only that. Between any two of a run's replies the stream may carry a RESP3 out-of-band push, the reply to a SELECT the write path injected, a high-integrity response token, or a preamble's reply: the n'th frame is not the n'th reply, and nothing may assume it is. The replies are N independent events that happen to have been asked for together, so the honest shape is a task each. Gathering them is a convenience built on top, not something the wire supports. RespBatchExecutor uses the run when the inner executor offers one and pipelines when it does not, so the fallback stays covered. The fake in ARunCapableExecutorGetsTheWholeQueueAtOnce throws from the single-command SendAsync, so a pass demonstrates the run path was taken rather than the same answers arriving by another route; ABatchWritesItsQueueAsOneRun drives the real bridge, which is the only thing that exercises the expansion, the per-request result boxes and the combined slot.
…ds one Marc: "we have knowledge of whether the server is in cluster though, for slot calculation; can we group by slot?" Yes, and it needs nothing from the topology: a request already carries the slot it folded while being written, and RespRequestBuilder only folds one when the context is a cluster. So outside cluster every request is NoSlot, they all fall in one group, and the grouping costs a comparison per command. I wrote this up as a compromise against grouping per node - fewer, larger runs, as the shipped RedisBatch.Execute does per bridge. That was backwards, and Marc said why: grouping by server RACES A RESHARD. The slot-to-node map can move between assembling the group and writing it, so a run built for one node may no longer belong there. A slot is a property of the keys rather than of the topology, so a group built from slots stays true however the cluster rearranges itself underneath. Finer splitting is the price, and it is the cheap half. Ordering holds within a slot and not across slots, which is the shipped guarantee too: commands that must be ordered relative to each other touch the same keys, and so share a slot. Also fixes a list pattern that needed System.Index and so failed the net481 build; both test TFMs are green now.
…s provisional Captures Marc's SendAsync(SomeKindOfBatchClass) shape - the collection as the argument, an enumerator that names flush points, index-tagged receive-queue entries, OnResult pairing, closed rather than an interface - plus the assessment. The defect that makes the current committed interface provisional is real and already shipped: RespRetryExecutor cannot implement IRespRunExecutor, because it must await between attempts and the spans cannot survive that. So ctx.WithRetry(p).CreateBatch() silently falls back to the pipelined path today and the batch quietly stops being a batch. The constraint worth settling first: IMultiMessage expansion runs inside the write lock, so an enumerator that blocks for a reply must release it - which makes a pause a contiguity boundary, not just a flush point.
Marc asked whether the executor API could be internal: it already was.
IRespExecutor has always been internal - the public surface is RespExecutor's
extension methods over the contexts, which is all an external caller needs to
issue a command - so this conversion is internal-only with no API change at all
(PublicAPI.Unshipped.txt does not move).
What a class buys beyond closure is the capability. IRespPreambleExecutor
existed only to be type-tested at the call site, so an executor that had never
thought about preambles was SILENTLY assumed not to want them. As a virtual
CanWritePreamble with a default of false plus a virtual method that throws, the
capability is asked rather than inferred - and a decorator can finally give the
right answer, which is its inner's:
public override bool CanWritePreamble => _inner.CanWritePreamble;
RespRetryExecutor could not express that before: it either implemented the
interface unconditionally or not at all.
The conversion immediately found one of these. RespSurfaceScriptsTests'
PairingExecutor overrides the pairing method and had never declared anything,
because overriding WAS the declaration; it now says so explicitly, with a note
about why that is the better trade.
IRespRunExecutor deliberately stays an interface. Its shape is provisional - the
spans forbid an async implementation, which is the defect recorded in the queue
- and a member on the base is a promise to every executor. It becomes a virtual
when the shape settles, at which point the same silent-downgrade fix applies to
retry+batch.
38 test fakes converted mechanically; both test TFMs green.
Marc: "maybe there's a handful of common patterns, probably dominated by GIGO". Dominated is right. Of the 28 fakes taking (params string[] replies), a normalised-body grouping put 17 in two buckets differing ONLY in whether they exposed Sent, Flags, or both, and another 4 wanted nothing but a count. The reply rule was identical in every one of them, clamp-to-last included. So: one Helpers/FakeExecutor with Sent, Flags, Sends, HasSent, and opt-in Keys, and 26 local copies deleted. It is not sealed, because the handful that need one more thing should add that one thing: RespSurfaceScriptsTests' parking executor now derives and overrides OnSent instead of restating the whole class, and its PairingExecutor derives from that. Left alone deliberately, because they are not the same shape: TrackingExecutor (drives a real server), SlowExecutor, FlakyExecutor, the gated and prebuilt ones, the benchmark executors, RunExecutor, and the (string reply) parity fakes whose Sent is a single string rather than a list. Keys capture is opt-in rather than always-on: it costs a KeyRange array per send, and RespAdHocExecuteTests - the file that is about key-ness - is the only caller that wants it. Note for the record: the Debug suite has two failures that reproduce at HEAD without any of this - InterpolatedCustomArgTests.AStructImplementerDoesNotBox (an allocation assertion, and Debug allocates differently) and FailoverTests.SubscriptionsSurvivePrimarySwitchAsync (timing). Release, which is what CI runs, is green.
Marc called it: forcing transactions and batch into the existing Message queue means building a third participant on a shim we are about to replace, using a primitive shape already known to be wrong. So those stay unimplemented and the core gets planned instead. design/message-core-replacement.md covers: what to take from origin/core-respite and what to leave; the operation type that replaces Message AND IResultBox (one IValueTaskSource object rather than a message plus a box plus a Task); the three-executor routing topology; what the new token must carry across; what we deliberately lose; the pooling policy; a phased order; and the open questions. The finding that settled the decision is worth repeating: core-respite already has a BatchConnection that is line-for-line what RespBatchExecutor was re-derived into this week, except built on a primitive where the operation IS the completion - so it needs no TaskCompletionSource per element and no return channel. Its tail already takes a run. The question "how does element 3 of 5 get its result out" has no answer in the current shape because it is the wrong question: element 3 is a RespOperation, and it completes itself. Two things the plan flags hardest, because neither fails a test when dropped: the diagnostics inventory on Message (Status is not merely diagnostic - retry reads NotApplied from it), and that pooling makes carrying it across harder rather than easier. Consequences recorded honestly in the queue: Fallback<T>() does not reach zero and SER352 stays at 8, and both were framed as release gates. Deferred, not cancelled. Planning only; no code.
Two corrections to the core plan, both Marc's. THE CACHE. core-respite predates it entirely, which is the largest gap between that branch and what we need. The win is real: the operation already owns a ref-counted response buffer, so filling the cache becomes a RETAIN rather than the copy PayloadProcessor does per reply today - the payload itself becomes the cached artefact, which is the shape the cache already wants. The exclusion is stronger than a simplification. Probed it: with a fake whose first reply was +QUEUED, a second identical read was served from cache without a send. IsCacheableReply rejects errors and nothing else, so the cache has no guard of its own - anything that hands back a non-final reply for a command, which is exactly a queued command inside MULTI, would poison the entry. That today's transaction path probably completes from the EXEC array rather than from +QUEUED makes the current behaviour accidental rather than designed. Recorded as a deliberate behaviour change, since batched commands participate fully today - and with a middle option worth weighing: probe on the way in, do not fill on the way out. A batched hit is still answered before it is queued, keeping the saved round trip, while the replies never populate the cache, which is where the complexity is. RESPOPERATION IS BATCH-ONLY. This settles what was the plan's hardest open question - whether the parse moves into the operation, at the cost of pushing generics through every layer. It does, but only for batches: the ordinary send keeps a non-generic executor with the handler applied above it, which is where the cache probe already sits correctly. The batch API takes request/handler tuples, so the element completes itself and RespBatchExecutor's TCS-per-element, out-span and scatter-back never arise - and do not arise anywhere else either, because nowhere else needs them. Planning only; the probe was temporary and is not committed.
Marc asked whether a sub-branch that just tries things - starting by deleting
the database implementations - has validity. It does, but the cheapest version
of the experiment answers the sizing question first, so I ran it: one edit to
GetDatabase routing EVERY IDatabase through TransitionalDatabase over the
context, classic database as fallback.
Failed: 110, Passed: 8051, Total: 8324 - 69 distinct tests
~98.7% of the whole suite already runs through the new surface, which was not
knowable by reading. Of the 69 distinct failures, roughly 30 are tests coupled
to the old implementation rather than gaps - 25 are InvalidCastException from
casting IDatabase to the concrete type, and a handful assert the shim's own
buffer-pool behaviour. The genuine gap is around 40, concentrated in behaviour
differences, argument validation and the command map.
Also measured what deleting would actually buy, which changes the proposal:
message construction is 504 sites in RedisDatabase, 122 in RedisServer, 30 in
ServerEndPoint - and ZERO in RedisBatch. So the target is RedisDatabase's
command methods, not "the database implementations": the others are decorators
that free no machinery. And it does not let Message go, because ~150 sites live
in the server paths, where the context surface has one group against IServer's
~70 members.
One constraint for whoever does the cutting: RedisBase must survive it.
RespMessageExecutor holds a RedisBase target and calls ExecuteAsync on it, so
the execute plumbing is what the NEW surface stands on; the cut is the command
builders, not the type that owns dispatch.
The spike was reverted, not committed. Planning only.
This branch has not been deployed
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
1 participant
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.
This started as "can a custom interpolated string handler write RESP?" and is now three halves that are
one design: a write path, the client-side cache that sits on it, and the command surface that reaches
both. It is no longer hypothetical — rendered frames go through the real pipeline to a real server, the
library negotiates
CLIENT TRACKINGitself, and real invalidations arrive.Framing: this is the V4 core, not a parallel option. We cannot run two write models through one
backlog, so the old
Messagemachinery is a deletion rather than a reconciliation — butIDatabasestays, served by the new core.
Messageitself stays too: it is abstract over exactly two members(
ArgCount,WriteImpl), so the core is already pluggable at precisely the rendering step; what goes isthe population of 36 subclasses that exist to write one command each.
Why the halves are inseparable
The cache depends on facts that only exist during the write:
costs a render and no allocation.
writer-side semantics, and invalidation needs it. The writer records it as it writes. This is the
load-bearing fact.
must render byte-identically or the cache silently stores duplicates.
The write path
At no point is a string built — the compiler lowers each hole into
AppendFormatted, writing UTF-8straight into a pooled buffer.
$"SET {key} {value}"renders identically; whitespace contributes nothing.Optional arguments are holes, not branches.
Expiration.DefaultandValueCondition.Alwayscontribute no argument, so all of
SETis one interpolation:That replaces a ~17-branch decision tree in
RedisDatabase.GetStringSetMessage, most of whose branchespick between fixed-arity
Message.Createoverloads. Arity is free here:*Nis accumulated while writingand back-filled into a reserved prologue.
One pass also folds the cluster slot (
MULTIwhen keys disagree), the key marks, and key andchannel prefixes.
Appendexists for a genuine caller-side branch; the append handler is the commandhandler, moved in and back out, so both accept the same things by construction.
Fixed tokens (
EX,NX, container subcommands) are pre-framedRespFragments carrying anArgCount, since multi-token is the common case (~140 call sites). Hand-construction is gated behindSER011— deliberately not inNoWarn— because malformed bytes desync every subsequent command.Extending the vocabulary: extension
AppendFormattedmethods do not bind in a hole (verified on thecurrent compiler), so the vocabulary would be closed and closed to us.
IRespArgument/IRespFormattableArgumentare the way back in — an interface on the argument, deliberately unrelated soimplementing only the formattable one makes the format mandatory. Struct implementers are constrained
calls, so nothing boxes, and an implementer cannot miscount because it writes through the handler's
own counters. There is no alignment overload and must never be one:
$"{key,10}"would pad the payloadand send a different key.
The client-side cache
Two independent lookups rather than a cross-index: table 1 keyed by frame + database, table 2 by
key bytes, no database. A server invalidation touches only table 2 — one hash, one stamp, never an
enumeration. Measured: ~5-6ns, zero allocation, flat from 1 to 100,000 cached keys. The database
asymmetry is protocol-faithful: Redis tracking uses a single key namespace.
The race that produces permanent staleness.
if (!TryGet) { Send; Add; }loses an invalidationarriving during the send — and the server never repeats it, so the entry is stale permanently.
Generations are captured before the send, which is why the cache is a participant in the send, not
the entry point. A cache hit completes synchronously and allocates nothing. Both sides are pooled and
ref-counted, not spans: a span cannot cross an
awaitor sit in a backlog awaiting resend.Everything fails closed: keyless requests are refused (invalidation only ever reports keys, so they
would be permanently stale); an undeclared retry category is refused (zero sorts below read-only, so a
naive
<=reads "nobody said" as "safe");NoClientCachesuppresses the probe as well as the store; andwith prefixes configured, any key outside them is refused, because otherwise there is no invalidation path.
Bounded:
MaxBytes/MaxEntrieswith sampled eviction,MaxPayloadBytes, periodic sweep, andcounters (
Stored,RefusedRaced,RefusedNotTracked, …) because the failure mode this design can stillproduce is silent and durable.
Config splits in two:
CacheOptionsis global per multiplexer (prefixes, tracking mode, quotas);CachePolicyis per-context (TTL, grace).RespClientCacheitself is internal — only the multiplexercan mint one, because a cache the caller attached by hand has no
CLIENT TRACKINGbehind it and wouldfill and never invalidate. The public opt-out is
ctx.WithoutCache().End to end:
ServerEndPointnow issuesCLIENT TRACKING ON [BCAST] [PREFIX …]during the handshake,built from
CacheOptions, and refuses loudly without RESP3 rather than caching without invalidation.PhysicalConnectionroutes theinvalidatepush. The in-process server models tracking too, so the testsexercise the library's negotiation rather than their own.
The command surface
Eleven groups so far (
Strings,Hashes,Sets,SortedSets,Lists,Bitmaps,Keys,Scripts,HyperLogLog,Geospatial,VectorSets), one file each. Three reasons: discoverability (IDatabaseis a flat wall of several hundred methods);module libraries become first class (
ctx.Search.Query(...)composes exactly likectx.Strings.Set,inheriting cacheability and prefixes rather than needing a parallel story); and it is the last break —
after a context exists, every addition is an extension member.
The
Asyncsuffix stays even though only the async API exists. The earlier spelling dropped it on thegrounds that there is nothing to disambiguate from; that was wrong for the ecosystem it lands in — analyzers,
ConfigureAwaitguidance and readers all key off the name, anddb.Strings.Get(key)returning something youmust await reads as a bug at a glance.
Commands are ordinary
this inextension methods, not extension blocks, and that is about retirement:deleting the
thisun-binds new call sites while compiled callers keep working. Only the group accessorsare blocks, since an extension property has no other spelling.
Which leaves down-level consumers, since an extension property needs C# 14. Opt into
StackExchange.Redis.Interpolated.Downleveland the same groups appear as methods —db.Strings().GetAsync(key)— with nothing else moving. Measured on real toolchains rather than by pinning
LangVersion(which is stricterthan reality, and was the first round's mistake): Mono/
net472at C# 7.3, the .NET 6 SDK at C# 10 and the.NET 8 SDK at C# 12 all compile that with the property in scope at the same time, because extension-block
metadata is invisible to a compiler that predates it rather than merely unusable. So the property costs those
consumers nothing, and only C# 14 callers must leave
Downlevelalone (importing both isCS9339, a compileerror, never a silent misbind). Written by hand — a generator would tax every consumer build to save one line
per group — and held to coverage by a test rather than a build step.
Targets are split by what they may offer.
IRespKeyspaceTarget(database, batch, transaction) carriesthe keyspace groups;
IRespServerTargetis for server-scoped ones. Previously every group bound toIRespTarget, whichIRediscarried — soserver.Strings.Get(key)compiled, on a type pinned to oneendpoint. Note what did not need splitting: the executors. A batch's context already queues and a
server's already pins, because the executor's target is the batch/server and
ExecuteAsyncis overriddenon both.
No arrays on the new surface —
ReadOnlyLease<T>and friends instead, with a parallel internalextension where the old API still needs an array. Multi-value replies go further and become windows into the
reply buffer:
MGETof a thousand 32-byte values allocated 56,656 bytes before, 736 after. The array wasnever the expensive part — a
RedisValuehas no lifetime, so it must own its bytes.The context composes in every direction, and never assigns. Services compose (the multiplexer attaches a
cache, a caller adds a probe, neither erases the other); key prefixes compose, matching what
WithKeyPrefixonIDatabasealready does by folding the two prefixes into one decorator; and channel prefixes now do too. Thatlast one was assigning, which is the bug this shape exists to prevent: a context is handed down through code
that does not know what its caller applied, so a library reaching for its own channel namespace could silently
cancel the tenant isolation above it — and nothing looks wrong afterwards, because the frame is well-formed and
goes to the wrong channel. There is deliberately no reset, exactly as you cannot un-prefix a
RedisKeyorunwrap a decorator.
So the prefixes are spelt
AppendKeyPrefix/AppendChannelPrefix, notWith*.Withreads as "the resultdiffers in this respect", which invites "so the second call wins" — the exact misreading that had just been a
live bug.
Appendrather thanPrependbecause the new prefix lands nearest the key:AppendKeyPrefix("a").AppendKeyPrefix("b")sendskasabk. Everything else keepsWith*, which is now areal distinction:
WithDatabase/WithServerTypereplace a value,WithCache/WithScriptCacherebind acapability by shadowing, and only the prefixes accumulate. The shipped
DatabaseExtensions.WithKeyPrefixonIDatabasekeeps its name — it behaves the same way, and renaming it would be a source and binary break tofix a name.
Cancellation is per-call, and refused honestly. It was going to be context state (
WithCancellation(token)),which is wrong twice: a token's lifetime is the operation's, not the connection's, and a context is a value
that gets captured and reused. So it is a
CancellationTokenargument onExecute/ExecuteAsync— and sincethe pipeline cannot yet cancel an in-flight request, a real token throws
NotImplementedExceptionrather thanbeing silently ignored. An already-cancelled token is honoured properly, because that one can be: the command
is recycled and
ThrowIfCancellationRequestedfollows, so an obvious no-op does not leak a pooled buffer.Scripts: composition, not a special frame
EVALSHAis always issued, withSCRIPT LOADcomposed in front of it when needed. The alternative — aframe that re-spells itself as
EVAL <body>after aNOSCRIPT— would have been the first exception toa frame is a pure function of its arguments, which is what makes frames cacheable, routable and
blit-writable. So the cleverness moves to composition instead, and the frame abstraction is untouched.
A registry renders each script's
SCRIPT LOADonce (exact-sized array, pooled rent returned). At writetime a gate asks whether the endpoint is already believed to hold it and skips the preamble — write
time, because the endpoint is not chosen until then, and a resend after
NOSCRIPT, a reconnect or aMOVEDmust re-decide. The belief is the endpoint's existing one, shared with the classic path.IMultiMessage.GetMessagescan now returnnullfor "nothing to compose, write me normally", so thecommon case does not build an enumerator to carry a single element.
Transition
RespFrameWriterrenders realMessageobjects intoRespFrames, so the existing surface feeds the newpipeline without being rewritten; both routes render byte-identically (pinned, because that is a cache
correctness property). The integration is one hook —
MessageWriter.Write(in RedisKey)reports theoffset, the one fact the bytes cannot carry — A/B measured at 66.96ns with vs 68.77ns without on
SET key value, i.e. below the noise floor.TransitionalDatabaseimplementsIDatabaseover the new surface;SER352reports the members stillunimplemented (currently 186) so the gap is a build warning rather than a surprise.
Status
Full
dotnet build Build.csproj -c Releaseclean on all six target frameworks; complete suite green(7514 passing). Frames are validated by parsing them back with RESPite's
RespReaderandDemandEnd(), so over- and under-run both fail. Works onnet461/netstandard2.0— handlers arecompiler lowering, and a source polyfill covers the attributes.
Measured, not argued: the zero-allocation cache hit;
OnInvalidatecost and flatness; theMessageWriterhook A/B; theMGETallocation collapse; invalidation timing against a real server(self-invalidation always trails its reply, which makes local-write eviction a correctness requirement rather
than an optimisation); and what a transaction condition costs a thread — with a 250ms round trip injected,
ExecuteAsyncblocks the calling thread for 508ms with one condition versus 2ms with none, i.e. two roundtrips done synchronously before you are handed a task to await. That is thread-pool starvation shaped, and it
is the number the
WATCHwork has to beat.Not measured: the interpolated writer against the existing path end to end — that argument is still
architectural.
Since this was opened
Done:
CLIENT TRACKINGend to end; theIServercontext; the keyspace/server target split; three more commandgroups;
SetResultsplit into inspect-vs-parse (6 overrides vs 89), which movedNOSCRIPTretry into thepipeline and deleted eight
catchsites — and fixed a real bug on the way, since the frame path never noticed-NOSCRIPTand so could hold a stale script belief permanently;IRespHandler.Parse(ref RespReader)takingthe positioned reader rather than a span; the
Asyncsuffixes, per-call cancellation and the down-level shimsabove; and errors stay exceptions — the
redis.callvsredis.pcallquestion — because an errors-as-valuesopt-in is a post-verdict decision that the inspect/parse split has now made cheap, rather than a pervasive
second API.
Also done since: one projection per element type, shared by every aggregate form — seven element types had
the same projection written twice, up to 250 lines apart, and
long?had it three times. No divergence hadhappened, but nothing prevented one, and a pair that disagreed would be invisible: both forms succeed and
return the length you expected.
HIMPORTprobed, and it is not a third seam.GetMessagesis called fromWriteMessageInsideLockwiththe
PhysicalConnectionin hand, which is exactly the "inject once the connection is known" the bridge'shard-coded
HIMPORTinjection has — soFramePairMessage+IRespPreambleGatecovers it with no newmechanism, and the only difference from
EVALSHAis the gate's scope: connection rather than endpoint.What the probe did find is a second axis the interface does not name, when the belief is recorded. A script
confirms on the reply, because only the reply proves the server has it; a field-set cannot afford that, since
every import issued before the first
PREPAREreturns still reads "not prepared". Measured under a contendedburst of 8: confirm-on-reply injects 8 preambles, claim-on-write injects 1. Both are correct —
PREPAREis idempotent — so it is a cost difference invisible without counting. Two of the three probes are now done
and both landed on the same seam, so the count is two mechanisms rather than three.
Batches and transactions now offer the groups by name.
IDatabaseAsynccarriesIRespKeyspaceTarget, sotran.Strings.SetAsync(...)binds directly. That is a required-member break for anyone implementingIDatabaseAsync/IBatch/ITransaction, taken deliberately: extending this family has always been the onlyway to add functionality here, which is the problem this work exists to end — and after this one, every
addition is an extension member. The composed pair refuses inside a transaction rather than shifting
EXECpositions, and by structure rather than a sixth hand-written guard:
QueuedMessagedefault-refuses anythingwhose
CanWriteWithoutExpansionis false. That refusal is worth more than it looks — flipping the flag doesnot fail the test, it hangs it, because the pair's result box is on a message that is then written but
never enqueued for a reply.
Replies that are windows, not arrays.
RespReplyis a disposable base for replies whose contents pointinto the buffer they arrived in;
Streams.RangeAsyncis the first command on it, and the deferred shape is adeliberate trade rather than a free win. Measured against materialising the same thousand-entry read: ~5%
slower for ~2,100x less memory, with equal work on both sides. (An earlier pairing here claimed "18% faster";
it compared traversal against materialisation and was not a fair test.)
RespReplyallows externalconstruction, so NRedisStack-shaped consumers can derive rather than wrap.
RespKey: a borrowed key. For the ad-hoc/IRedisKeyquestion from #2578 and #2844 - a key that can be aspan, a
ReadOnlyMemory<byte>, a string or null, with no allocation. Null is a third state rather than anempty payload, which also fixed something nobody had noticed:
default(RespKey)now matchesdefault(RedisKey)in being null.$"{someString}"does not get key handling, by the way - it binds to theRedisValueoverload, so no prefix, no mark, no slot. That is why the key spelling is explicit.Cancellation reaches the surface. All 272 group methods take a
CancellationToken, threaded to the send.Free now, a binary break after
SER010comes off - which is the deadline that matters, not "later".A group is now its own files. Thirteen groups moved from
RespSurface.<Group>.cspartials of one class toGroups/<Group>.cs(group type + accessor) +.Methods.cs(+.Types.cswhere the group owns types).SORThas no group, so it isKeys.Sort.cs. The accessor cannot live on the group class -CS0542- so eachgroup contributes its own partial to
RespDatabaseExtensions, which means adding a group is one file and nocentral list can drift. The
RS0026suppression moves with it and becomes a per-group claim that can actuallybe checked.
Command text that carries a decision is written once. The lease/array/writable-lease siblings meant most
commands were composed two or three times; where that text holds an optional token, a derived operand or a
command chosen from an argument, the copies can disagree, and disagreeing changes the reply's shape.
Factored across seven groups - including the whole
FIELDS numfields field [field ...]family in hashes,eighteen sites behind one factory with the count derived from the span. Bare texts stay where they are sent:
there is nothing in
$"{RedisCommand.GET}{key}"to get out of step, and hiding it behind a factory costs thelegibility this surface exists for.
A silent wrong answer, found and fixed:
WithDatabasedid not change database. The database is not part ofa rendered frame - no
SELECTis written - so routing is the executor's, andWithDatabaseonly replaced aproperty:
db.WithDatabase(1).Strings.GetAsync(key)read database 0 and reported 1. Worse than usuallysilent, because the client-side cache keys on the executor's database too, so nothing was inconsistent with
anything; the answer was just wrong. Pinned against a real server, and the test fails without the fix. Found by
asking what a server-scoped
DBSIZEwould need.Naming, and a version.
RespFrame->RespRequestFrameandRespCommandHandler->RespRequestBuilder:XRANGEis a command,XRANGE 1 4 COUNT 10is a request.version.jsonsays 4.0..NET 11 runtime-native async: measured, and not adopted. It needs both
<Features>runtime-async=on</Features>and an assembly-level attribute that the RC1 ref pack does not ship, so the attribute is declared locally and
the whole target is behind
/p:IncludePreviewTargets=true, forced off while packing - a gate that had to bewritten where a global property cannot outvote it, because the obvious spelling silently put
net11.0in the.nupkg. On RC1 it helps the inline path ~8% and costs the suspending path ~22%, so: revisit at GA. (Theharness lied first:
Task.Yield()moved continuations to the thread pool and made suspending look fasterthan inline, which is impossible. Marc spotted it.)
The first server-scoped group exists:
server.Server.DatabaseSizeAsync(db), onIRespServerTarget-an interface that had been declared with nothing bound to it - reached through
RespServerExtensions, theserver-side twin of
RespDatabaseExtensions. Getting the name cost a rename: a public typeStackExchange.Redis.Serverand a namespaceStackExchange.Redis.Serverare both reachable asServerfrominside
StackExchange.Redis(CS0435), and the in-process test server had that namespace. Rather than callthe group
Serversor split the class and accessor names, the toy moved: assembly and package stillStackExchange.Redis.Server, code now inStackExchange.Redis.ManagedServer.DBSIZEtakes its databaseexplicitly - a server context has none of its own - and that is real routing, since the command takes no
operand and follows the
SELECTthe pipeline applies.Next, in likely order: the arity-2 walker that finishes
the row-parser collapse (
HashEntry,SortedSetEntry); wiring the realHashImportfield-set through theframe path so the bridge's
if (cmd is RedisCommand.HIMPORT)type test can retire; more command groups;ISubscriber's context; and dropping the.Interpolatednamespace, which is free whileSER010is on.Blocked, and honestly so:
WATCH/MULTI. The write loop pauses mid-transaction to decide betweenUNWATCH,EXECandDISCARD— and the fix is not a redesign of that pause, it is theMessage/write-looprefactor that removes the pulse the pause waits on. Designing around a mechanism already scheduled for
deletion would be wasted work, so this one waits.