Repository navigation
Conversation
…er is deleted `IServer.StringGet(int db, key)` now reads through `Strings.GetAsync` on a context moved to the database the caller named - which is also what puts the `SELECT` where it belongs, since a server context carries no database of its own. And `GetMemoryPurgeMessage` is **deleted** rather than ported. Nothing in `src/` used it any longer - `MemoryPurge` went to the context surface earlier - so it was production code kept alive for a test to look at, which is the thing these ports are supposed to remove rather than preserve. Its assertions moved onto the request the group method issues, joining the other ported categories: `MEMORY` as a whole reads, so `PURGE` was once retried as a harmless read, and the node-scoped bit has to survive or a retry goes to a server that was never asked. The caller-override case moved with it. Both ported members were **untested** - every `StringGet` in the suite is the `IDatabase` one, and the purge builder was only ever reached by the retry test - so each is now pinned in the parity suite. A port whose only route is the one nobody exercises is a port nobody has checked. `RedisServer.cs` is at 33 `Message.Create` sites from 68. 26 are `SENTINEL`, which keeps its own bridge, so the real remainder is 7: `SCAN`'s five spellings, `CLIENT KILL` (its filter wants modelling rather than forwarding a token list), the `CLUSTER NODES`/`SLOTS` call sites (the builders stay, because `AutoConfigureAsync` and the multiplexer use them), and `MakePrimaryAsync`'s ordered sequence - `REPLICAOF`/`SLAVEOF`, the tie-breaker `GET`/`DEL`, the reconfigure `PUBLISH` - which are not `IServer` members so much as steps in a failover. Default suite 11,224 passing with one timing rotator; both flags 11,223 with the `ReconnectRetryPolicy` pair and four rotators. All six pass in isolation (545 tests across their classes). CI build clean.
… the right socket The last subscription shape that was staying on the shipped path now moves, and it did not need the "send reports where it finished" capability I had assumed. Two things close it, neither a new executor contract. **The redirect has to be followed onto the SUBSCRIPTION socket.** `TryFollowRedirect` resolved every target through `_forEndpoint`, which is the ordinary connection - so following a redirected `SSUBSCRIBE` the obvious way would fix the routing and poison the connection, because under RESP2 a subscribe on the ordinary socket puts it into subscriber mode and it then refuses everything but (un)subscribe, PING and QUIT. There is now a second resolver, used for the six subscriber-mode commands - `UNSUBSCRIBE` included, because a redirected one still has to reach the socket the subscription is on or it unsubscribes something somewhere else. Under RESP3 both resolvers answer the same executor, so this costs nothing there. **Where it landed is re-resolved from the SLOT afterwards, not reported by the send.** A `-MOVED` both moves the subscription and teaches this core where the slot went, because `OnSlotMoved` updates the map on the way through - so the slot's owner afterwards IS the answer. `RespNewCore.EndpointForChannel` asks it; a channel with no slot answers null, meaning "where it was aimed". That is why the decline could be lifted without plumbing a new channel back from the executor, which is the thing I had written down as the blocker. `ClusterShardedTests.SubscribeToWrongServerAsync` and `KeepSubscribedThroughSlotMigrationAsync` pass - 6 cases, none skipped - as does the whole cluster/redirect family twice over (204 tests). What is left of D2.5 is `IdentifyEndpointAsync` alone, and it is the one member the slot trick genuinely cannot serve: it asks about an arbitrary channel rather than a routed one, so it does want the send to report where it finished. Default suite 11,226/0. Both flags: the `ReconnectRetryPolicy` pair plus `ClusterTests.MovedProfiling`, which has rotated all session and passes in isolation. CI build clean.
`IdentifyEndpointAsync` did not want the "send reports where it finished" capability either. The shipped version reads the endpoint off the connection its `PUBSUB NUMSUB` reply arrived on, which is whichever one routing chose - so choosing before the send and answering that is the same fact by a shorter route. Safe because `PUBSUB NUMSUB` is keyless and node-local and therefore cannot be redirected, which is exactly what made the SUBSCRIBE case hard and this one easy. The round trip still happens: it is what makes the answer an observation rather than a guess. **And moving it exposed a defect in the slot resolution from the previous commit.** `EndpointForChannel` computed the slot from the channel's own bytes, where the server routes by the name it RECEIVES - the prefixed one. So with a channel prefix configured, a subscription landed on one node and was recorded against another. It only surfaced once `IdentifyEndpoint` moved and the two stopped agreeing by accident; `ClusterTests.ClusterPubSub(withKeyPrefix: true)` named it in two lines (`Expected: 127.0.0.1:7001, Actual: 127.0.0.1:7002`). It now asks `ServerSelectionStrategy.HashSlot(in RedisChannel)`, which already handles the prefix and `IgnoreChannelPrefix` - borrowed rather than re-derived, which is the rule I should have followed instead of hand-rolling a slot. **D2.5 is done.** Publish, subscribe and unsubscribe for every channel shape - literal, pattern, sharded, key-routed, multi-node - the subscriber ping, the re-subscribe-on-reconnect, and the endpoint identity all travel on this core under the flag. That is the whole of the subscription bridge's reason to exist. Both flags: **exactly 2** - the `ReconnectRetryPolicy` pair, which is not answerable while both cores consult one policy object and is assertable at D2.8 - with 11,228 passing. Default suite 11,225 with one timing rotator that passes in isolation. CI build clean.
…re is its shape Measured rather than assumed, because "subscriptions have moved, so delete the bridge" looks like a one-liner and is not. Not activating the shipped subscription bridge under the flag, plus relaxing the "fully established" gate so a connect does not wait for a leg nobody dials, gives **63 failures** across the pub/sub, handshake, cluster and config families. Reverted; the chain is now written down so the next attempt starts from it: - The subscribe path DECLINES a subscription that would share the ordinary connection, because this core picks its socket when a send is composed and a later downgrade can move it - issue #3154. That decline was only ever safe because the shipped bridge was there to take those subscriptions. - Remove the bridge and the decline has nowhere to drain: the fall-through becomes a subscribe that goes nowhere, which `ClusterTests.ClusterPubSub` reports as an empty subscribed endpoint. - Remove the decline too, so this core takes everything, and the shared-socket path goes live for the first time - which is where the other 63 come from. The pieces to close it are the ones D2.5 already built: `SubscriptionEndpoint` shares only on KNOWN RESP3, `IsSubscriptionConnected` stops answering yes for a shared socket once the negotiated protocol drops, and the establish-time `EnsureSubscriptions` re-places the subscription on a dedicated one. What is missing is making that chain hold for a subscription composed before a downgrade and written after it - the narrow window #3154 names. 63 failures is the measurement of how much of the suite depends on getting that right rather than nearly right. No source change: this commit is the map. HEAD is the verified D2.5 state - both flags at exactly 2, the `ReconnectRetryPolicy` pair.
…eded `origin/main` had drifted by two commits; merged rather than left to accumulate, which is cheaper now than later and keeps provenance. No conflicts. The valuable part is that BOTH needed a counterpart on this core - a fix to the shipped path is not a fix to this one, and neither gap is visible from main. **#3254, the configuration channel under RESP3.** The shipped fix subscribes the BRIDGE's interactive connection. Under the flag `IDatabase.Execute("CLIENT", "ID")` names THIS core's connection, which was not subscribed - `ClientKillTests` caught it as `Normal` where `PubSub` was expected. Covered today, because the broadcast still lands on the bridge; at D2.8 nothing would be subscribed and the bug returns. This core now subscribes its own RESP3 connection, prefix applied, registry bypassed as the shipped one does. Two orderings are load-bearing: after the handshake and on the NEGOTIATED protocol, and after `OnPush` is wired - under RESP3 a subscribe confirmation IS a push, and one arriving before the dispatcher is dropped as unrecognised, which presents as the connection timing out in its own backlog. **#3250, TLS/SNI host names.** `RespTransportFactory.AuthenticateAsync` carried its own copy of the rule that commit replaced. An endpoint that already carries a DNS name must use ITS name for SNI; only a non-DNS endpoint infers one. Cluster shards over TLS/SNI are what breaks otherwise - every shard presented the same inferred host, so every shard but one fails validation. Now `config.ResolveTlsHostName(endpoint)`, shared. That file's own remarks warn against two copies of a security decision while this was one. **One known failure, latent in D2.5's work rather than caused by the merge, and open:** `ClusterTests.ClusterPubSub(sharded: false, withKeyRouting: false, withKeyPrefix: true)` under both flags receives 20 messages for 10 published, while the publishing node reports one subscriber each time. Established: it passes at the pre-merge commit (verified in a worktree) and reproduces with the config-channel counterpart disabled, so the trigger is the role repair - which sets `IsReplica`, and on this branch that setter publishes the role and asks for a reconfigure, so topology applies now cascade. The shape that fits is two of our connections subscribed on two different nodes, since a non-sharded PUBLISH in a cluster is broadcast to every node while the publishing node counts only its own: a subscription record moved endpoints while the old socket stayed subscribed. `RemoveIncorrectRouting` does that unsubscribe-before-replace for key-routed channels only, and this channel is not key-routed. Next step is instrumentation rather than another guess; the notes carry all of it. Default suite 11,245/0. Both flags: the `ReconnectRetryPolicy` pair, this one, and five rotators that pass in isolation. CI build clean.
Fixes the one failure the main merge exposed, and it was a window rather than a leak. **Diagnosed by instrumentation rather than guessed at**, after two wrong theories. Tracing both delivery entry points showed every delivery arriving on THIS core - no bridge deliveries at all, which killed the "both cores subscribed" idea. Tracing the out-of-band path by socket then showed subscription sockets at ALL THREE cluster nodes receiving. A non-sharded PUBLISH in a cluster is broadcast to every node, so each of those delivers while the publishing node counts only its own subscriber: twenty deliveries for ten publishes, with a reported count of one. **The cause is the in-flight window D2.5 created on purpose.** The endpoint is recorded only on CONFIRMATION, so a subscribe that never landed stays retryable - but `EnsureSubscribedToServer` guards with `if (IsConnectedAny()) return 0`, and in that window the answer is "not live". Every ensure arriving in it starts ANOTHER subscribe, and for an unrouted channel `SelectServer` deliberately spreads, so each lands on a different node. This core's establish-time re-subscribe then feeds the loop, which is how it reached three nodes rather than two. The merge did not cause this. #3254's role repair sets `IsReplica`, which on this branch publishes the role and asks for a reconfigure, so topology applies now cascade - and that opened the window often enough to see. It was latent before, and would surface on any cluster that reconfigures while a subscribe is in flight. The fix is one clause - `IsConnectedAny() || HasSendInFlight` - and it is the same conflation the ownership field had to split earlier, in the other direction: **"not live yet" and "nobody is doing anything about it" are different questions.** `IsHeldByNewCoreOn` already consulted the in-flight endpoint for the ping gate; this is the other reader that needed it. Both flags: back to the `ReconnectRetryPolicy` pair plus `ClusterTests.MovedProfiling`, 11,247 passing. Default 11,244 with the known timing rotator. Three consecutive clean runs of the cluster, pub/sub, keyspace and Issue1101 families. CI build clean.
First piece of the gate on removing the subscription bridge, and a direct port of the shipped core's answer to #3154: `PhysicalBridge.WriteMessageInsideLock` hands a subscriber-mode message to `ServerEndPoint.TryRerouteToSubscriptionBridge` when the connection it is about to go out on turned out to be RESP2. `RespEndpointExecutor.RerouteSubscription` is the same question in the same place. Why it has to be the write rather than the compose: whether a subscription shares the ordinary connection depends on the NEGOTIATED protocol, and a command can be composed while RESP3 is expected and written after a reconnect has settled on RESP2 - at which point writing it there puts the connection into subscriber mode and it refuses every ordinary command afterwards. Inert while the bridge still exists, which is how it was verified rather than assumed. **And removing the compose-time decline on top of it was tried, and does not work yet - for a reason that corrects my earlier framing.** With the decline gone, `Resp3DowngradeTests` fails on TIMING, not correctness: nothing is poisoned, but rerouting at the write has to DIAL the subscription socket first, and the in-process server's transcript shows `PUBLISH => :0` landing before the re-subscribe arrives. The shipped core does not pay that because it dials its subscription bridge the moment a downgrade is detected - `OnFullyEstablished`'s `else if (SupportsSubscriptions && Protocol > Resp2) Activate(Subscription)`. So the two mechanisms do DIFFERENT jobs and both are wanted: choosing the right socket when the protocol is already known is the fast path, and the reroute is the safety net for the window where compose-time knowledge was wrong. The decline was never a crutch for the reroute's absence, which is what I had written down before measuring it. Next piece, now specified: dial this core's subscription socket proactively when a connection establishes below the expected protocol, mirroring that `Activate`. Then the decline can go, and then the bridge. Both flags: the `ReconnectRetryPolicy` pair plus three rotators - `MovedProfiling` and two `Resp3HandshakeTests` cases that pass 192/192 in isolation - 11,245 passing. Default 11,244 with the known cache-sweep timing rotator. CI build clean.
… we hold one A connection that comes up BELOW the protocol it expected has subscriptions about to be re-placed onto a socket that does not exist yet, and a re-place that has to dial first loses the race against whatever the caller does next. The shipped core reaches the same conclusion in the same place, so dial it here on the establish. Guarded on actually holding a subscription for that endpoint, which is the correctness of it rather than a refinement: unconditionally, an endpoint whose subscriptions belong to the shipped bridge gets a second socket and the channel is subscribed twice. Also forget subscription records naming a replaced socket: a fresh socket carries nothing, so a record naming it reads as "already subscribed" and suppresses the re-ensure.
Removes the compose-time decline that kept RESP3-shared subscriptions on the shipped bridge - the gate on the subscription bridge, since while it stood the bridge had to exist to place them. Four pieces make it safe and each was measured by removing it again; the fifth had to be built, because the ordering the shipped core gets by accident (both bridges reconnect on one heartbeat) has to be arranged here, where the subscription socket is dialled lazily and the need for one is known only after the ordinary connection is warm. SubscriptionsSettling is that happens-before: an endpoint observed to downgrade holds an entry until its subscriptions are back on the wire, and publish waits on it, bounded. It has to be released by the re-place rather than by the dial - a socket already opened once returns from the connect long before the replacement runs its establish hook. Also retires the now-dead WouldSubscribeOnItsOwnSocket, narrows the downgrade test's mixed-traffic classifier to data commands (handshake probes on a dedicated subscription socket precede subscriber mode and are legitimate), and makes the RESP2 separate-pubsub assertion ask the server which socket is subscribed rather than assuming the bridge's.
A ServerEndPoint no longer builds a subscription PhysicalBridge at any protocol under the flag: the subscription leg is the other core's, which dials and owns its own socket, and with the share-the-connection decline retired every subscription goes there. All four construction sites now answer with the interactive bridge or decline. Stopping at "don't activate it" does not work and is worth the note in the design file: IsSelectable creates the bridge it asks about and then requires it to be connected, so an unactivated one makes every subscription command unselectable - no server is chosen, the send reads as "nothing to do yet", and the subscribe silently does nothing. 70 failures, nearly all a publish reporting no subscribers. Not needing it and not building it are one change. Five tests were counting this core's sockets or asking for two distinct connection ids; each was already written as "RESP3 shares one connection" and now says "or the engine flag". The sixth was a real gap: IsSubscriberConnected ignored a disabled SUBSCRIBE, which the shipped path got free because such a bridge never connects.
Under RESP2 the shipped core subscribes the configuration channel as the last step of the subscription bridge's handshake, so removing that bridge removed the client's only way of hearing that the topology moved. This core's subscription socket now does it in the same position - and, crucially, gets dialled at activation rather than lazily: the only subscriber is the library itself on that socket's own establish, so a client that only calls ReplicaOfAsync never touches this core and nothing would ever connect. Dialled on the same KnowOrAssumeResp3 condition the shipped core uses before activating its bridge, so a wrong guess costs a spare socket exactly as it did there. Also replaces the tuple state object in the dial, which referenced System.ValueTuple and failed SanityChecks.ValueTupleNotReferenced. The connect gate stays open for now: making IsSubscriberConnected consult this core's subscription leg was measured at 39 failures and a ten-minute run - the same connect-wait hang as before - so ConnectUsesSingleSocket waits briefly for the count instead, keeping the upper bound it exists to protect. Recorded in the design notes.
The share-the-connection decline was, in a cluster, also keeping key-routed channels on the shipped path - so retiring it exposed three things that had never had to work here. 1. Re-resolve the landed endpoint only for SSUBSCRIBE. A key-routed channel has a slot, so the slot's owner can always be computed, but a plain SUBSCRIBE is not routed by it and never answers -MOVED: the subscription really is on whichever node it was sent to. SubscribeToWrongServerAsync(sharded: false) subscribes via a deliberately wrong server and requires it to still be there. 2. Act on an UNSOLICITED unsubscribe push. When a slot migrates away the node drops its shard channels and says so; there is no command of ours to match that to, so matching alone left the client believing it was subscribed on a node that had stopped delivering. Read from a copy of the reader, since the matching layer parses the frame from the start. 3. Scope the publish barrier to the downgrade re-place. The configuration-channel dial runs at activation, once per node, so in a cluster every publish waited on six of them and any one slow to establish held the lot - a five-second publish against the shipped path's eight hundred milliseconds.
…re does Replaces the ad-hoc forget-and-re-ensure with the shipped routine itself. An unsolicited SUNSUBSCRIBE - the node dropping its shard channels as a slot leaves - now calls ResubscribeToServer via the OUTGOING node, which is the only one known to have the new route, and returns Handled rather than leaving it to matching. "Unsolicited" is told apart by there being no send in flight, which is what the shipped core asks of its own outstanding commands. ResubscribeToServer now sends via the owning core, which under the flag is not a refinement: the shipped send resolves to the interactive bridge once there is no subscription bridge, so under RESP2 it would put the connection carrying ordinary commands into subscriber mode. Two further gaps this exposed: - Rebind never passed forSubscriptionEndpoint, so TryFollowRedirect fell back to the ordinary executor and a redirected sharded subscribe put the target's ordinary connection into subscriber mode - the next publish there answered "only (P|S)SUBSCRIBE ... allowed in this context". The rule was written in TryFollowRedirect; it just had nothing to apply. - ForgetSubscriptionsOn now skips a subscription with a send in flight. Such a subscribe is very often why the socket is being established at all, and its record is about to be confirmed rather than stale; forgetting it let the re-ensure place a key-routed channel on the slot owner instead of the server the caller explicitly chose.
… weakly from pushes ActivateServer dials this core's subscription socket where it would have activated the shipped subscription bridge, on the same KnowOrAssumeResp3 condition, because the configuration channel is subscribed by connecting rather than by anyone asking: a client that only calls ReplicaOfAsync never touches this core, so nothing would ever connect and the broadcast reached nobody. The establish-hook dial keeps its TryResp3 condition - dropping it made a plain RESP2 client dial a second time, which RespSubscriptionConnectionTests counts. OnPush now holds the multiplexer weakly. A subscription connection holds a standing read, so the socket is rooted while open and anything strong reaching out of it keeps the multiplexer alive; a push arriving after the multiplexer is gone has nowhere to go anyway. This is right but not sufficient - the leak predates this change and is written up in design notes 9g with what to chase next, along with the full before/after suite numbers and the measurement showing that withdrawing the dial trades two failures for six worse ones.
A connection runs a read loop for as long as it is open, so its pending read keeps the connection reachable - and every strong reference out of a connection then keeps the whole deployment alive. A caller who abandons a multiplexer without disposing it never got it collected. The shipped core already says how this is meant to work: PhysicalConnection reaches its bridge through a WeakReference and calls the property BridgeCouldBeNull to say so. This core's connection needed the same on all three of its paths out - OnPush, Server, and the redirect router - and the router was the one that mattered, being a lambda over an instance method; the other two alone left the multiplexer rooted. Each can go null while the connection is open, which every caller already handled: none of them is known until the endpoint is resolved, so null was always possible, and "nobody is modelling this any more" gets the same treatment as "not yet known". GarbageCollectionTests.MuxerIsCollected now passes under both flags.
…annel "Nothing has been sent, so nothing has dialled" stopped being true once this core dials at activation for the configuration channel, so the test asserted its own precondition rather than the mapping it exists to measure. Run with that channel off instead, which restores the precondition and leaves the property alone. Also clears SlotMapIsDrivenByTheSlotsView in the same batch - that one passes alone and its in-batch failure reports a MOVED from a real server, so it was the in-process server sharing port 6379 with the docker primary, not a routing fault. Flagged full-suite failures are back to the baseline count of 5, with the subscription bridge gone, the configuration channel carried, the multiplexer no longer rooted, and two of the baseline five fixed. Numbers and membership in design notes 9g.
Adds Subscription.OnSubscribedViaNewCore, the missing half of OnSubscribedViaBridge: ownership was otherwise only ever set by TrySendViaNewCore, so a caller that places a subscription on this core's connection itself had no way to say so, and the registry asked the wrong core whether it was live. The heartbeat reads that as a subscription nobody is connected for and subscribes it again, which the server reports as two subscribers to one channel. ASubscriptionOnTheNewCoreReceivesDeliveries also has to use the multiplexer's own core rather than the test fixture's: the registry asks server.Multiplexer.NewCore, so two instances can never agree about ownership. The fixture's core is for asserting about commands, where there is no shared registry to disagree with. With the flag off the shipped bridge owns subscriptions and will always contest a hand-placed one, so the test now skips there, stating why. It had been failing on the shipped run for that reason; the shipped suite is now fully green.
A send to a node that has stopped accepting writes never completes, so the endpoint it recorded was never cleared and HasSendInFlight answered yes for ever - after which every EnsureSubscribedToServer returned "nothing to do" and the subscription could not be re-placed at all. Bounded by the configured timeout. Bounded by TIME rather than by asking whether the connection is live, which was tried and is wrong: a subscription socket that is still dialling is not live either, so liveness cannot tell "not up yet" from "never coming", and treating the first as abandoned reopens the duplication window the guard exists to close. Found while diagnosing RetirementUnderMaintenanceTests, which it does NOT fix: that one is EndpointForSlot having no reachability filter where the shipped selector passes allowDisconnected: false, so this core keeps aiming a slot at an owner it cannot reach. Written up in design notes 9h as the next step, along with the causes ruled out.
EndpointForSlot asked the topology who owns a slot and picked by role, with nothing asking whether the chosen node could be reached - where the shipped selector asks it in its signature, as allowDisconnected: false. So a slot whose owner had gone away kept being chosen. "Known down", not "not connected": this core dials lazily, so an endpoint nobody has needed yet is unconnected and perfectly usable, and barring it would route every first command away from the node that owns the slot. An endpoint counts as down only once it has been dialled AND has a connect fault to show for it - which is the distinction ServerEndPoint.PublishSelectable already relies on from the other side when it excludes DidNotRespond. Either socket counts. Asking only about the ordinary executor missed the case that matters: a node this core has only ever subscribed on has no ordinary executor, which is exactly the shape of a sharded channel's node going away. And Settle now records NOTHING when a redirect points at a node known to be down. The re-resolution reads the slot map, and a -MOVED updates that map whether or not the resend it caused ever landed, so a redirect to a departed node was recorded as the subscription's home - the one answer that is certainly wrong, since it is neither where we aimed nor anywhere reachable. Unplaced is the truth and is what lets the next ensure retry.
RetirementUnderMaintenanceTests guards against vacuity by requiring the refusing node to be accumulating our own probe traffic. Under the engine flag that state does not arise: this core will not aim anything at a node it has dialled and found unreachable, so the node is simply idle rather than busy-with-probes-but-idle-of-caller-work. Verified before changing it by relaxing only that precondition - the whole rest of the test passes, so retirement itself was never in question. The flagged path therefore asserts that nothing is queued there at all, which is a property worth pinning, rather than skipping; everything after that point still has to hold either way. Design notes 9i records the three parts of the routing fix and why the first two were each insufficient alone.
…oken Three consecutive full runs of the same build: shipped fully green, flagged five to seven failures with only ReconnectRetryPolicyUnitTests in common. The rest rotates almost entirely, and most of it is timing - intervals, timeouts, connect failures, a profiled redirect. Naming it as its own problem because it is one: the stable count is two and the observed count is five to seven, so quoting either alone would misrepresent it. The likely driver is three sockets per node under RESP2 while both cores exist, which makes it the interactive bridge's removal to fix rather than any individual test.
The wedge in 9b-xiii was seen a third time, and this time it produced no failure list at all: a full flagged run exceeded its thirty-minute timeout and was killed, where runs of the same build either side finished in about three minutes. Worth recording because CI would report that as a timeout rather than as this. Also rules out the obvious suspicion about 9j's rotation: these runs follow hours of cluster tests that migrate slots without migrating them back, so the shared docker cluster was checked directly - every node reports cluster_state:ok with all 16384 slots assigned and ok. The topology is intact, so the rotation belongs to the client.
…what is missing An earlier note recorded the gate as measured closed. Re-measuring - gating the interactive handshake's AutoConfigureAsync on !NewCoreEngine and running the full flagged suite - gives 26 failures, not baseline. The failures cluster by belief, which is the useful part: the new core's handshake does not publish the product variant and version, the tie-breaker, CLUSTER NODES as distinct from CLUSTER SLOTS, or the SET replica probe used when HELLO cannot report the role. Each is named in design notes 9k with the tests that catch it, and with the order to close them in. The handshake already asks INFO server; it just discards everything except redis_version, so the product variant is the cheapest of the four and the largest cluster of failures.
DiscoverProductAsync reads INFO server and derives the product and its version, reusing AutoConfigureInfoField and its generated TryParse so the field names keep one spelling. It is not a cosmetic label: ServerEndPoint decides whether a cluster supports multiple databases from it, so a client that never learns the product applies the wrong rule. From INFO rather than HELLO, matching the shipped core: HELLO's server field would be free but names the product without its product version, and the version is half the belief. The variant is decided across the whole reply, because redis_version and valkey_version can both appear in either order. Placed in DiscoverServerConfigAsync, already the interactive-only home for beliefs nothing else has described, so the handshake transcript is untouched - several tests assert it exactly. Gated on an empty recorded product version, the same "has anyone said?" test the database count uses. With the shipped auto-configure gated off this closes ProductVariantUnitTests (7 of 26). Design notes 9l records the rest, including why closing the other three gaps individually will not close the gate: the next failure is not a missing belief but a missing connection.
…erdict Wired the whole coupled step as an experiment - shipped core stops dialling, this core dials at connect via ConnectMode.Discover, IsConnected and IsSelectable answer from this core, shipped auto-configure gated off - and measured it: a 311-test family went from ~20s to 6m30s with 36 failures, spaced ~15s apart. The 15s is ConnectImplAsync deciding the multiplexer never connected: ReconfigureAsync(first: true) returns false and connect throws UnableToConnect after its retries. Every failure in the list is that one exception, so nothing else about the coupled move is measurable yet. One hypothesis eliminated and deliberately not landed: completing OnConnectedAsync's pending connection monitors from this core's establish, which is the obvious candidate and changed the measurement not at all - same failures, same duration, same spacing. Dropped rather than committed, because landing a plausible-but-unverified change is exactly how the "gate measured closed" error in 9k happened. Experiment patch kept in the scratchpad; design notes 9m records it.
ReconfigureAsync proves an endpoint is reachable by sending a tracer down the "already connected, show me" path of OnConnectedAsync, and WriteDirectAsync puts that on the shipped bridge - so under the engine flag it proves the wrong connection, and against the step that stops that bridge dialling it is written to a connection that will never carry it and never completes. ConnectFailTimeoutTests.NoticesConnectFail is exactly the test that catches it. Declines unless the other core actually has the endpoint connected, so it cannot report "unreachable" about a server the bridge is talking to perfectly well. Measured clean in both default configurations: shipped fully green, flagged within its usual rotation band. Design notes 9n records this alongside the two hypotheses that looked just as good and fixed nothing - completing the pending connection monitors from this core, and dialling this core before the connect verdict - both dropped rather than committed. The ~15s per connect that the coupled move still costs remains unexplained, and the next step is to read the connect log rather than reason about it.
…ath is 82% of the rest "How much still travels as a Message" had been answered by counting source sites, which flatters the position badly - RedisDatabase.cs holds 395 of them and under both flags not one runs. Measured instead, with a counter on the Message constructor over the same 2,282 tests: 79,114 messages shipped against 9,772 under both flags, and zero data commands in the flagged run. No SET, HSET, LPUSH or ZADD at all. Of the 9,772: the connect path is 8,050 (82%), the tie-breaker GET 862, SENTINEL 432, SUBSCRIBE/PSUBSCRIBE 403 (built to route, never sent), SCAN 25. So there is one structural problem left between here and a single core - the connect path, which is where 9m/9n is stuck - rather than a long tail. Counter reverted rather than kept; design notes 9o says how to re-run it.
Reading the connect log, as 9n said to, named it immediately: the pending connection monitor is handed out by OnConnectedAsync and never completed, so every endpoint runs out three ConnectTimeouts. 9n's first hypothesis had the mechanism right. It failed because it and the second hypothesis were two halves of one change and I tested them separately - the hook had no connection to fire from, the ordering had nobody to complete the waiter. Together they still failed, and that exposed the real finding: muxerEndpoints=0. The multiplexer has no endpoints until ReconfigureAsync resolves them, so any dial before it iterates an empty list and does nothing. The dial belongs inside that method's per-endpoint loop, beside ActivateServer. With that, connect goes from 15,081ms to 72ms, the log shows the monitor completed by this core, and the shipped interactive bridge never connects at all. The 311-test family goes from 36 failures in 6m30s to 24 in 50s, and not one remaining failure is a connect failure - they are the belief gaps already itemised in 9k, with the tie-breaker the largest. Not landed: the enabling dial is not inert by default - it would add a socket per endpoint while the shipped bridges still dial - so it lands with the switch that stops them. Patch kept in the scratchpad; design notes 9p.
The handshake took the server type from HELLO's mode field and from CLUSTER INFO, which leaves two cases unanswered: a server with no HELLO - exactly the case that needs another source - and a product that spells the field server_mode rather than redis_mode. ValkeyUnitTests is built on both at once and was read as Standalone. Read in the same INFO server pass that already derives the product, using ServerTypeMetadata.TryParse and accepting both spellings, which is the shipped processor's rule rather than a guess. Guarded the way Publish guards it: a sentinel or a proxy is not something an INFO section gets to overrule. Against the coupled move this takes the 311-test family from 24 failures to 18, clearing all four ClusterTopologyUnitTests and both ValkeyUnitTests - so those were never the CLUSTER NODES gap they had been attributed to in 9k/9p, they were this. Measured clean in both default configurations.
…nd allocate less per fill The eviction queue grew without bound once slabs arrived (1bd841e): every store enqueued its key, but with slabs a byte budget is met by evicting whole slabs, and the queue is otherwise drained only for MaxEntries. Now enqueued only when entry-at-a-time eviction will read it - a reply with no slab, or an entry limit. Every miss registers an InFlight so a concurrent caller can share its round trip, and each allocated a TaskCompletionSource and its Task eagerly; on a miss-heavy workload a second caller almost never comes. Created on demand now, with a full-fence publish/wait handshake so a waiter arriving as the fill lands cannot be missed. Slab key lists are presized for ~1 KiB replies rather than regrown and recopied ~10 times per slab. cache-scaling miss also takes a nocache switch (the baseline) and reports GC pause as a share of wall time. All-miss, 64 async callers: 275-287k -> 289-315k ops/s, CPU/op 33 -> 31-32us. The baseline is 533-545k at ~21us, and the gap is GC - 28% of wall time paused, vs 1% uncached: an entry's small objects are mid-life on an all-miss workload, and slabs churn the large-object heap. Not admitting one-off misses at all is the lever for that.
…epeated miss Storing a reply copies it and keeps an entry until it is evicted, and on an all-miss workload those entries are mid-life objects: the cache's overhead there was mostly GC (28% of wall time paused). CacheAdmission.OnRepeatedMiss puts TinyLFU's doorkeeper in front of the fill: a first miss is remembered in a two-probe Bloom filter over the request's hash (no allocation, no key copy) and served without touching the cache; only a request that misses again before the filter is cleared is stored. False positives admit a request one miss early; there are no false negatives, so a hot request is never kept out. The filter is ~8 bits per entry the cache can hold (4 Ki-16 Mi bits) and is cleared after an eighth as many first sightings as it has bits. Selectable, default unchanged (OnFirstMiss): the hit-rate side - one extra miss before a hot key is cached, against one-off reads no longer evicting it - wants a skewed mixed workload to judge. New counter: RefusedNotAdmitted. All-miss, 64 async callers (cache-scaling miss ... admit): OnFirstMiss 307k ops/s 30.9us CPU/op 29% GC pause OnRepeatedMiss 592-594k 19.2-19.4us 3.8% no cache at all 536k 20.6us 1.2%
…ounds small replies Since slabs, an entry was charged only its share of slab bytes - a few bytes for a count or a flag - while its bookkeeping (the entry, the payload wrapper, the dependency array, the dictionary node, the copy of the request) is ~350 bytes. So a byte budget never bound a workload of small replies: a million cached counts reported ~5 MB while holding hundreds. Every entry is now charged EntryOverheadBytes (256) plus its request's length. The remarks that still described per-reply ArrayPool rounding are rewritten for slabs. With the bookkeeping counted, evicting single entries always frees bytes, so the entry-at-a-time loop now acts on a byte excess too - but only for the thread that did the slab eviction: one that found another already evicting slabs leaves the bytes to it, rather than racing it down below the budget. Also parks the builder-inlining draft as design/parked/builder-inlining.patch (was only in a session scratchpad); the ledger tracks it with the other cache ideas still to explore.
The stored copy of the request was an array of its own per entry. It now goes into the slab with the reply, key then reply in one contiguous range - one object fewer per entry, and for a small reply (a count, a flag) the two sit together: a lookup that finds the key is already beside what it came for. RespRequest.CopyForCacheKey gains an overload that copies into caller-given storage; requests too large for a slab keep an array of their own. Same-session A/B, real multiplexer, hits: 100k keys at 24 threads 76.2/71.3 -> 81.4/81.8 M/s; 1 thread 6.6 -> 6.8; one hot key unchanged. All-miss: neutral within noise - one object fewer per entry is lost in the GC cost that remains there, which admission (a732552) is the answer to.
An entry depends on the keys its request names - a (key-table node, generation) pair per key - and held them as an array even for the single-key commands that are most of them. Every hit asks IsValid, so the array was one more dependent load between the entry and the generation it compares, and one more mid-life object per entry. One dependency is now a field; only several get an array. The fill's own array is no longer referenced by the entry, so it dies young. Same-session A/B, real multiplexer: hits at 100k keys / 24 threads 83.7/79.9 -> 85.7/84.0 M/s, one hot key at 24 threads 143-148 -> 150; single-threaded unchanged. All-miss (one run each): 316k -> 342k ops/s, GC pause 24.8% -> 19%.
The miss harness's keys and cache, but each read picks its key from a Zipf distribution (s = 0.99, a seeded table of 4M draws, so every arm sees the same sequence), and every mode now reports the hit rate over the measured window. Where admission's two effects meet: one extra miss before a hot key is cached, against the long tail no longer evicting it. 64 async callers: OnFirstMiss 1.05-1.07M ops/s 9.8us CPU/op 72.4% hits OnRepeatedMiss 1.58-1.61M 7.1-7.2us 76.6% hits no cache 553k 19.6us
…call; two follow-ups
…orrected It can write - it caches the computed cardinality back into the HyperLogLog's header - but that is the data structure's bookkeeping, not the command's meaning: re-running it gives the same answer, and a cache hit that skips it skips nothing observable. Refreshing the header signals the key modified, so the PFCOUNT that refreshes it invalidates its own cached reply and the next one is cached: one extra miss, never a wrong answer. The code already classified it this way; the design notes (6.9) said caching it was wrong. CommandCategoryTests.PfCountIsARead pins the decision.
Measured on a skewed workload (Zipf, s = 0.99) it raised the hit rate as well as throughput (72.4% -> 76.6%, 1.06M -> 1.6M ops/s), and on an all-miss workload it removed the cache's overhead entirely; it loses only where every request is read exactly twice. So it becomes the default - as CacheAdmission.Default, resolving to OnRepeatedMiss, on the CacheTrackingMode pattern: zero means "the library's choice", so that choice can move without overriding anyone who named a rule. OnFirstMiss and OnRepeatedMiss renumber to 1 and 2 (unshipped). The doorkeeper is now sized from the most entries the budget could hold (MaxBytes / EntryOverheadBytes) rather than assuming ~1 KiB per entry, which gave small replies a window several times too short. The cache-mechanics tests (invalidation, eviction, budgets, sweeps - not admission) now name OnFirstMiss explicitly, since they fill and then expect a hit. Docs: the quick start shows a reply cached on its second read; the Admission section explains the rule, the measurement, the cost and how to opt out; "Is it working?" warns that a check reading twice is testing OnFirstMiss.
…stops routing Every enumerator - SSCAN/HSCAN/ZSCAN, the emulated key SCAN, vector-set range paging - checked the WithCancellation token between pages but sent each page with no token, from when every executor refused a cancellable one. So cancelling took effect only at the next page boundary: a slow page ran to completion after the caller had given up, and against a server that never answered, the enumeration never ended. RespScan.ForSend now hands the token to the send wherever the executor can act on it; elsewhere the between-pages check still stops it. CancellingMidPageAbandonsThePageInFlight pins it (and fails, timing out, without the change). RespMultiplexerExecutor.CanCancel answered by resolving a route - server selection and the endpoint's lock - on every call made with a cancellable token, the same shape as the Transactional regression fixed in c537026; its routes all reach executors that can cancel, so it is now constant. That also stops a cancellable call to a multiplexer with no reachable endpoint being refused as "not supported", rather than failing for the reason it would.
…dependent, so checked before changing
… as a repeat
RespFest's skewed cells showed the default admission behaving exactly as
OnFirstMiss (Zipf 72.1% vs 72.5%; all-miss rows identical). The cause was the
byte hash behind RespRequest, RedisKey, RedisChannel and RespKeyTable: a 32-bit
multiply-by-33 over words folded in half, which gave 200,000 framed requests for
unpadded keys (GET rf:cbig:{i}) ~42,000 hashes between them. The doorkeeper then
took its probe from the low bits of a bare multiply, so those landed on ~4,500
bits of a million: the filter was full from the start and admitted every miss.
The local harness used zero-padded keys, which happened to hash well.
Now a word at a time with one 64-bit multiply each, then MurmurHash3's
finalizer: ideal distribution over a dozen key shapes, as keys and as requests,
at the same cost on a short request. The doorkeeper also runs its hash through a
full avalanche before probing, so a weak hash cannot collapse it again.
On RespFest's key names, OnRepeatedMiss: Zipf 72.4% -> 75.1% hits; all-miss
324k -> 610k ops/s, GC pause 22% -> 2%. Hit path: the stub single-key loop
measures 70-97ns across builds of any change to this function (one returning
the old value measured the same), so layout; 100k keys +4-7ns.
The hit-scaling harness names OnFirstMiss (it measures hits, not admission) and
uses unpadded keys, as RespFest does.
Counting only what it recorded, a keyspace smaller than the window (131,072 at 32 MiB) was never forgotten: once each key had missed once, nothing new arrived to trigger a clear, every later miss read as a repeat, and the default admission became OnFirstMiss for good. RespFest's Zipf cells replay a 1M-draw table of ~120,000 distinct keys, so showed exactly that: 72% either way at 64 callers. Modelled (design/admitsim.py): that stream 72.5% -> 76.1%, independent draws 75.3% -> 76.1%. Measured with the harness, which now draws as it goes rather than replaying its own 4M table: default 76.4%/76.0% at 1/64 callers vs 72.4%/ 72.5% OnFirstMiss; 1.45M vs 1.07M ops/s at 64. All-miss unchanged (596k ops/s).
…backlog count again
…rejected; three stale descriptions corrected
…or, dedicated pub/sub under RESP3, connection-attempt events
…ct for dedicated); the cache is opt-in, not on by default
… cannot serve a replaced value A connection group resolves the active member's cache per command, so a write made while B was active stamped only B's cache. A's server hears of it only by replication, and A's tracking connection is told only then - so failing back could answer from A's entry with the value the caller had just replaced, for up to the replication lag: read-your-own-writes broken by a switch. Each member's cache now holds its peers (RespClientCache.SetPeers, relinked by the group on construction, add and remove), and a local write stamps them all. A then misses and asks its server, which may still be behind - that is replication, and an uncached read sees the same; the cache is never staler than the member it answers for. Server invalidations and flushes stay per member. Flushing on every switch was rejected: with equal weights a group selects by latency, so the active member can change on any health pass. The ledger's second worry - a silent inactive member - needs no code: group members force HeartbeatConsistencyChecks, so every endpoint is pinged each heartbeat, and a half-open tracking connection trips the dead-socket rule and flushes that member's cache, exactly as for an active one. Tests: GroupCacheTests (in-process servers that never replicate; both write paths, the IDatabase forwarding and the context; both fail without the fix) and the peer relinking on Remove. ActiveActiveCacheScenarioTests in the fault-injector tier, against real replication - compiled, not yet run.
…ts async twin does Found porting NRedisStack, whose command factories hand back frames: SendAsync<T>(ref frame) took an optional handler (the inbuilt one for T when omitted), while Send<T>(ref frame) demanded a non-null handler and a token. The synchronous overload now takes both as optional, matching the interpolated Send<T> - flags stay required, as they are there. Unshipped API, so no compatibility cost.
…not just timeouts
…gRPC's call token - in Basics.md and AsyncTimeouts.md
11 tasks
…ce 2026-10-09); the PR-body-only backlog items (cache metrics, AppendFormatted, .NET 11) recorded here
This branch was successfully 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
2 participants
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.
What 4.0 is
A new core, with the old one deleted.
Message,ResultProcessor,PhysicalBridge,PhysicalConnectionandRedisDatabaseare gone. Commands are rendered by an interpolated-string RESPwriter and sent through executors over the RESPite transport. Reconnect, backlog, resubscribe, redirects,
maintenance events, sentinel and logging (shipped event ids preserved) were ported and checked against
the old core before it was deleted.
IDatabasestays, served by the new core. Existing code keeps compiling and working; seedocs/LegacyApi.md. The old API is not deprecated.A grouped, async-first command surface alongside it:
Commands are extension members on group types (
Strings,Hashes,Keys, ...), so adding one - ours or athird party's - is never a binary break to an interface. Replies can be leased rather than allocated, and
every call takes a
CancellationToken.Highlights
buffers, and replies are parsed on the caller's continuation rather than by the reader. The core's IO
loop now only frames bytes in and out, so one slow or large reply no longer holds up everyone else's.
Instead of building
object[]argument arrays forExecute/ExecuteAsyncand picking through aRedisResult, a library writesctx.SendAsync<T>($"FT.SEARCH {index} {query}")with a typed replyhandler, getting the same zero-copy formatting, cluster slot routing, key prefixing and caching as the
built-in commands, and can hang its own command groups off the same targets as extension members.
ConfigurationOptions.ClientCache, RESP3, opt-in) driven by the library itself(
CLIENT TRACKING), keyed by the rendered frame, with invalidation that cannot race the send; in a connectiongroup, our own writes reach every member's cache. See
docs/ClientSideCaching.md.SER014):db.BeginBatch()/db.BeginTransaction()return structs that are ordinary keyspace targets, with explicit
ExecuteAsync, discard-on-dispose, andconditions via the existing
Conditiontype.Blocking()contexts send and wait on the calling thread, so a groupmethod returns an already-completed
ValueTask; the synchronousIDatabasemethods are built on it, andso can a library's.
DedicatedThreadsconnections connect and handshake synchronously, so on Linux theirsockets never need a pool thread either.
Blocking()for sync forms,IDatabaseAsync.AsTask(...)for
Taskforms (async state and observed faults), andcontext.Multiplexerfor connection-level settingssuch as
AddLibraryNameSuffix. Seedocs/Extending.md.CLUSTER SLOTS, batches split per slot,MOVED/ASKhandled in the executor.Compatibility
4.0.N-alpha.[Experimental]IDs (SER011,SER012,SER014; seedocs/exp/).IDatabasefamily are deliberate and expected in a major version; removals andsignature changes to shipped members are not.
Status
The full suite runs on Ubuntu (net10.0) and Windows (net10.0, net481); CI is green at the branch head (3cf425d).
Latest prerelease: 4.0.127-alpha (2026-10-12). Intermittent failures are listed under Watch, with detail in the ledger.
Pending
Mirrors
design/v4-ledger.md(last updated 2026-10-13), which has the detail; ticked here as items close.Decisions
SingleReadLoop/SEREDIS_SINGLEREADLOOPopts outDedicatedThreadsconnections connect and run TLS synchronously, so they never need a pool thread (dbb38f8)RespBatch/RespTransactionstay structs?SharedSubscriptionConnectionopts in)Work
ActiveActiveCacheScenarioTestsagainst a real Active-Active deployment - written and compiled, never executed (needs the fault-injector environment)Subscription._subscribed/IsSubscribed/AnySubscribed; ~30ConnectionsIfCreated?.null-tolerances; topology flowing both ways betweenRespTopologyandServerSelectionStrategyConnectAsyncis called without the token (default mode only)RetryDatabase.ExecuteAsyncandRespRetryExecutormainby merging (ongoing; the final landing must be a real merge)design/client-side-cache.md(378a103)PFCOUNTis cacheable and retryable - its write is internal bookkeeping (d59d4e2)Send<T>over a pre-built frame defaults its handler, as the async twin does (97ec53a; from the NRedisStack port)Watch (flakes, not yet reproducible)
GetServerByKeyMemoization(RESP3, Windows net481) - speculative fix d45eadf; not seen in the 60 CI runs since 2026-10-09RespCacheChurnTests- 2026-10-09, and again 2026-10-10 (Ubuntu) and 2026-10-12 (Windows net10.0); take a dump mid-stall next timeBacklogTests.TotalOutstandingIncludesBacklogQueueandSentinelTests.AbortOnConnectFailFalseRecoversOnceSentinelsAppear- intermittent, pre-existingQueuedResultTests.RetryTransactionFireAndForgetSharesOneCompletedTask(once)MaintenanceOptInServerTests.SequenceIdsAdvanceAndCanBeRepeatedfails when its class runs aloneCancellationTests.WithCancellation_CancelledToken_ThrowsOperationCanceledException- a race in the test (Task.WaitAsyncreturns an already-completed task as-is); should cancel through the API's own tokenTouchIdleTime,RespAggregateTiming,ADeferredWalkNeedsNoStorageAtAll, and seven single CI occurrences on 2026-10-10..12 (listed in the ledger)Backlog (after the alpha)
__keyspace@<db>__:*; needsnotify-keyspace-events, no RESP3 requirement) and explicit pub/sub (an application-defined channel the writer publishes to), for deployments without usableCLIENT TRACKING(proxies, older servers, some managed offerings)CanWritePreamble,RespKeyTablenode lifetime, the parked builder-inlining patchValueTasksurface (non-pooledasync ValueTaskonly; low priority)AppendFormatted(long)/(double)overloads (NRedisStack; needs an overload-resolution sweep)TlsBufferSession); run the suite and RespFest on 11 early