Repository navigation
Conversation
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.
…shake AUTH was awaited with no catch, so a server that refused it failed the whole handshake - where the shipped handshake writes AUTH fire-and-forget, cannot read the reply at all, and therefore continues while recording the suspicion via SetAuthSuspect. Commands then fail individually with the server's own words, which is a far better diagnostic than a connection that never exists. The case that made it matter is not a wrong password but ONE configuration spanning servers with different requirements: a config carrying the secure server's password also reaches the plain one, which answers "Client sent AUTH, but no password is set". Awaiting that reply took a perfectly good endpoint out of the deployment. Also reads the tie-breaker, which is the deciding vote in the primary election - without it every endpoint logs "had no tiebreaker set" and then "No primaries detected". Re-read on every handshake rather than cached, because a tie-breaker is mutable state and changing it is how an operator moves the election. Database 0 named explicitly: a tie-breaker is a key, the asking context may name no database, and GET is refused before it reaches a socket - which is what RespNewCoreDiscoveryTests caught, in both configurations. Against the coupled move these take the 311-test family from 18 failures in 50s to 6 in 11s, clearing all ten MultiPrimaryTests.TestMultiWithTiebreak and both ConfigTests.MutableOptions - so MutableOptions' "could not connect" was this, not a separate structural problem.
A role is not a preference, it is a routing fact - the client refuses writes to a replica on the strength of it - and unlearned, every server looks like a primary. MultiPrimaryTests.CannotFlushReplica connects to the replica, looks for the server where IsReplica is true, and finds none. From INFO replication, which is the only source always available. ROLE is better where it applies, since a primary lists its replicas and one reply describes both sides, but it is asked only when the topology says roles could change a decision - which with a single configured endpoint they cannot, and that is exactly this case. HELLO carries a role only under RESP3. master_host and master_port come from the same section, so PrimaryEndPoint is read with it. Re-read per handshake like the tie-breaker rather than cached like the product: a failover changes this. Takes the coupled family from 6 failures to 5; design notes 9q records the whole 24 → 5 run and corrects two wrong attributions - CLUSTER NODES turns out not to be needed by anything measured, so the 9k itemisation over-counted by a row.
Four pieces, each needed by the coupled move and each correct on its own terms. KEEP-ALIVE. The heartbeat reached this core's executors but only ever expired backlog and pending - it sent nothing. An idle connection is a connection being timed out: servers close one after their own 'timeout' (the very setting this core reads during discovery) and the network between does it sooner. Two schedules, as the shipped bridge has: every heartbeat when consistency checks are on, since noticing a dropped stream promptly is their whole purpose, and otherwise once the connection has been quiet for WriteEverySeconds. Fire-and-forget, because a heartbeat that waits for a server that has stopped answering is a heartbeat that has stopped beating for everything else. OPERATION COUNT. ServerEndPoint.OperationCount summed bridges, so it read zero for a server this core had been talking to all along - which is also how a test asks whether the heartbeat is doing anything at all. CONNECTION ID. The handshake now asks CLIENT ID, and GetConnectionId answers from whichever core holds the connection. It is the only handle on a connection from outside - CLIENT LIST names it, CLIENT KILL takes it - and it was null for the connection actually carrying the commands. The subscription leg falls back to the ordinary connection when it IS the ordinary connection, which under RESP3 it is; answering null there would describe a connection that does not exist rather than the one carrying the deliveries. Together with the earlier handshake work this takes the coupled 311-test family from 36 failures in 6m30s to 0 in 4s. Measured clean in both default configurations.
The 311-test family is fully green under the coupled move - 36 failures in 6m30s to 0 in 4s - but it was not representative, and the full suite says 126. Recording that plainly, because optimising against a slice made the position look much better than it is. Grouped, it is one piece rather than a scatter: maintenance notifications are 70 of the 126 across six test classes, and they are a coherent feature - the opt-in is sent only by the shipped handshake, and the pushes, relaxation windows, retention and replay all hang off a connection that has opted in. This core neither opts in nor would recognise the pushes. That reframes the census in 9o: the connect path was 82% of remaining Message traffic by volume, but by test weight maintenance is the largest thing still on the shipped bridge. One run of this build was killed at the 25-minute timeout where neighbours took 3.5 minutes, so 126 is from a run that completed rather than a stable average.
The opt-in is the feature's gate: the windows, retention, replay and migration handling all hang off a connection that has asked, and one that has not asked is never told. With this core carrying the commands and only the shipped bridge opting in, the client was being warned on a connection it no longer used. Sends CLIENT MAINT_NOTIFICATIONS ON [moving-endpoint-type <t>] and reads the reply, which is why it is asked from here rather than written blind - "agreed" and "declined, because X" are different facts. Both decisions are borrowed rather than restated: the shipped ShouldRequestMaintenanceNotifications and MaintenanceMovingEndpointTypeLiteral gained overloads this core can call, and the shipped callers delegate to them. moving-endpoint-type needed real plumbing rather than an approximation. For Auto the type is derived from the address actually reached and whether the connection is encrypted - not the endpoint dialled, which is usually a name - and only whoever built the transport knows either, with encryption sometimes the tunnel's decision. RespTransportFactory.ConnectAsync now reports both. Guessing internal where it is external sends a client to an address it cannot reach when a shard moves. Fixes ~11 maintenance failures; 58 remain and all need the push path. Design notes 9s records its shape: lift PhysicalConnection.OnMaintenanceNotification's ~270 lines of deliberately tolerant parsing out so both dispatchers call it, rather than writing a second copy of rules that exist for go-redis compatibility.
9s said the 58 remaining failures wanted the push path and that the move was to lift the parse rather than copy it. It was a smaller refactor than it looked: eight of the eleven members were already static, and the three that touched 'this' wanted exactly four facts - the server, whether the connection is established, the address it reached, and a trace sink. So OnMaintenanceNotification becomes ReadMaintenanceNotification taking those, the instance method is a four-line wrapper, and RespPushDispatch calls the same routine. The ~270 lines of deliberately tolerant parsing are untouched: no second spelling of the optional sequence id, the movable time, SMIGRATED's nested triplets or the explicit nulls. It returns void rather than OutOfBandResult, which is private to PhysicalConnection and a different enum from the new core's: a maintenance notification is never a reply, so the frame is consumed either way and there is nothing to signal. OnMovingAnnounced took a PhysicalConnection and used it for one thing - the address the announcing connection reached - so it now takes the address. That is the third time this session that 'the address we actually reached' turned out to be the real parameter hiding behind a connection. Maintenance failures 58 to 17. Shipped suite fully green.
ReconcileMaintenanceNotifications decides whether an unavailable-but-Enabled feature should fail the connection. The rule and its wording are now an internal overload returning the failure message; the shipped caller turns it into RecordConnectionFailed and this core throws out of its handshake, which is the same thing in its terms - a handshake that throws fails the connect, and "Enabled means required: no notifications, no connection" is exactly that. And a correction. This core had been asking for the opt-in only when RESP3 was actually negotiated, rather than on intent as the shipped core does - strictly fewer round trips, same end state, apparently better. It is not better: MaintenanceOptInClientTests.AutoIsOffWhenTheServerDowngradesToResp2 requires the server to have SEEN the opt-in and the client to disbelieve the acceptance anyway. That sequence is observable, deliberate and covered, so skipping it is an unflagged behaviour change dressed up as an optimisation. Now asks on intent, with the reasoning written where the temptation is. Also: a group member never asks but must warn that it is not asking, because a caller who wrote maintNotifications=Enabled asked for a guarantee - silently suppressing it is the gap the warning exists to close. And the suppressed path still reconciles, since Enabled over RESP2 never gets as far as asking and failing it is the point. Maintenance 70 to 8; the flagged suite is at 3, shipped fully green.
…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
…d drop the Logging.Abstractions reference where it is in the shared framework (NU1510) (#3276)
…r hops - DedicatedThreads recovers it
…lt now resolves to it (Broadcast when Prefixes are given, since PREFIX needs BCAST)
…y flag, the entry queue spares flagged entries once, and flagged entries are copied forward out of an evicted slab Measured in-test against exact LRU and FIFO at the same capacity (Zipf s=0.99: LRU 66%, FIFO 62%, now 67%; hot set + scan: LRU 74.5%, FIFO 56%, now 74.5%), and in RespFest's cache league before/after (zipf hit rate 76% -> 79%; hit, churn and sad-path cells within spread). The hit path gains one conditional field write per entry per eviction cycle.
…te(RedisKey|span) and InvalidateAll(), acting as local writes (peers stamped, no grace period), plus the cache's counters
…OptOut, TrackingOptIn, TrackingByPrefix) (#3297) * client cache: opt-in/opt-out by mode and by flag - CommandFlags.CacheOptIn/CacheOptOut (two bits, both-set reserved) replace NoClientCache; Tracking negotiates OPTOUT, new TrackingOptIn negotiates OPTIN, new TrackingByPrefix (OPTIN, Prefixes applied client-side, flags win) is what Prefixes now select by default; CLIENT CACHING YES|NO travels as a per-request prefix frame written as one unit with the request (RESPite IRespMessage.Prefix + discard sink); a transaction that reads is sent behind CACHING NO under OPTOUT * tests: netfx expectations for the CacheOptIn/CacheOptOut formatting cases (the second zero-valued name, PreferMaster, is appended to every non-zero value on .NET Framework)
… ported onto the new core, and StyleCop 1.3 (#3298) * Fix StringSet(Expiration, ValueCondition) doc link: set, not delex (#3272) The overload (and StringSetAsync via inheritdoc) pointed at the DELEX command page; it was copied from StringDelete. Also add the missing <returns> to match the sibling overloads. Fixes #3269 * Tolerate refused CLUSTER SLOTS/NODES during topology updates (#3275) * Tolerate refused CLUSTER SLOTS/NODES during topology updates (#3274) Proxied deployments (Azure Managed Redis / Redis Enterprise) can identify as a cluster while refusing CLUSTER SLOTS ("ERR command is not allowed"; Enterprise exposes SLOTS only under the OSS cluster API). That refusal surfaced as a logged *error* from GetEndpointsFromClusterNodes at connect and on every reconfigure, including the periodic topology refresh. A refusal is a posture, not a fault: catch RedisServerException per task - SLOTS refused falls back to the NODES-only path (as the method's own comment always intended), NODES refused means no topology view at all, so return null; both are logged at Information (EventIds 128/129). Tests: ClusterCommandsRefusedUnitTests drives the in-process test server with SLOTS (and SLOTS+NODES) refused, asserting discovery still works and no error is logged; CapturingLogger promoted from ConnectFailureRefreshTests to a shared test helper. * Observe both topology tasks, so an abandoned one never faults unobserved SLOTS and NODES are issued as a pair; if one await throws anything other than a server refusal, the other task is abandoned mid-flight - and both ride the same connection, so the orphan is likely to fault too. Attach ObserveErrors at creation so it can never surface as an UnobservedTaskException in the host. (Pre-existing gap: the bare sequential awaits had the same shape.) * Report a transport connection's failure the way a stream connection's is reported (#3273) * Lag-aware failback checks for geo-redundant failover (#3242) * Investigate Redis Enterprise lag-aware availability checks Confirms the premise: this is the client-side geographic failover feature family, and Lettuce, Jedis and redis-py have all shipped it. It feeds exactly the health checks our Availability/ namespace already has behind SER007. The abstractions line up almost one-to-one with redis-py's - probe, policy (all/any/majority), circuit breaker, per-member config - and two details make it land cleanly: HealthCheckProbe has no internal abstract members, so it is already subclassable from outside the assembly, and ConnectionGroupMember already carries a per-member HealthCheck override, which is what a per-cluster REST endpoint needs. Where we are better placed: HealthCheckResult has Inconclusive, which Lettuce's two-state HealthStatus lacks. Lettuce maps a failed REST call to UNHEALTHY, so a management-plane outage is indistinguishable from the data plane being down and can fail over a database that is serving fine. We can say "could not determine" instead. The awkward part is that src/ has no HTTP client and no JSON parser. JSON turns out to be avoidable if the bdb uid is configured rather than discovered, since the availability call is a status-code check. Also records the defaults disagreeing across clients: 100ms everywhere - server, docs, redis-py - but 5000ms in Lettuce. * Record what a Tunnel API for the management plane should look like mgravell offered to add one; the shape is what decides whether it helps. A member returning HttpMessageHandler would drag System.Net.Http into StackExchange.Redis's public API for the sake of one commercial deployment target - which is the thing keeping this in a separate package was meant to avoid, decided the expensive way. A stream-shaped member instead composes into SocketsHttpHandler.ConnectCallback, which takes exactly a ValueTask<Stream>, so the Enterprise side gets a complete HTTP hijack and core gains no HTTP types at all. Tunnel already speaks that language via BeforeAuthenticateAsync and ConnectTransportAsync, and adding a virtual is binary-safe for subclasses. Two functional arguments for Tunnel over a separate knob, beyond testing: a CONNECT proxy configured for Redis almost certainly applies to the Enterprise REST API too, so co-locating makes that correct by construction; and the test harness already assigns Tunnel, so one assignment would spoof both planes. Caveats recorded: ConnectCallback is net5+, the existing members are EndPoint/ConnectionType-shaped where HTTP wants a Uri, and widening Tunnel's remit means HttpProxyTunnel and LoggingTunnel each need an opinion about whether they apply to the management plane. Also retires open question 5: RawConfig and ConfigurationOptions.Tunnel are both already public, so a probe reaches the tunnel from the context it already gets - no HealthCheckContext change needed. * Real clusters and the fault injector, and a correction I had written that inducing "reachable but stale" on demand is not something anyone can do in CI, so the fakes were the only route to the scenario that matters. That is wrong. Redis ships a fault injection service that the client suites drive over HTTP - POST /action with a type and parameters, poll the returned action_id - and Lettuce's ActiveActiveFailoverScenarioTest injects network_latency against a bdb_id for a bounded duration, which is exactly the mechanism for pushing replication lag past the tolerance and watching the lag-aware check flip while the plain check stays green. So the authoritative end-to-end test is real cluster plus fault injector, on a road Lettuce has already built. The fakes are demoted rather than removed: they still cover machines with no Enterprise cluster, they are far faster than an injector action that budgets minutes plus a stabilisation wait, and they cover what the injector does not obviously reach - specific error_code bodies, 401/403 credential expiry, slow-but-successful responses for probe timeouts. Also records what the real clusters are uniquely good for: everything in section 1 is read off documentation, and a short session settles whether extend_check=lag is actually supported, what failure bodies really contain, whether the Host header is required, what the bdbs listing really returns - which decides whether the JSON-avoidance argument holds - and what lag real geo links show, which is the empirical way to settle 100ms versus 5000ms. * We already have a fault-injector tier; it answers several of the open questions It is not merged - tests/StackExchange.Redis.FaultInjector.Tests lives on marc/maint-optin-server, open as #3191 (107 files, +13160) - which is presumably why I did not find it when I grepped a branch taken fresh from main. Two pieces of it bear directly on this. FaultInjectorClient wraps the same POST /action + GET /action/{id} contract go-redis, redis-py and node-redis converged on, and adds GetValidTriggersAsync, so we can ask a deployment which lag-inducing effects it supports instead of guessing at an action name. And ClusterRestClient is already a Redis Enterprise REST client on 9443 - Basic auth, System.Text.Json, GET /v1/bdbs?fields=uid,name - so the availability call is a method on an existing class, not a new project. It also settles by observation what I had listed as open and sourced only from documentation: the management certificate is self-signed per environment and pinned to a CA from the config directory, so the probe needs CA configuration and cannot rely on the ambient trust store; auth really is Basic; and the bdbs listing returns the requested fields, so discovery is easy - which weakens the avoid-JSON argument to "it keeps JSON out of src/", not "discovery is hard". Sequencing consequence recorded: an end-to-end test is downstream of #3191. And #3191's own summary is the best argument for validating before building - same vendor, same class of API: the specifications are prose, the payloads were never published, and nearly every starting assumption was wrong. * Add the plan, blocked on #3191 Step 0 is to wait for #3191 to merge and build from there, rather than writing a second Redis Enterprise REST client against the same cluster and reconciling the two later - its ClusterRestClient already does Basic auth, CA pinning and GET /v1/bdbs, and its FaultInjectorClient can be asked which lag-inducing effects a deployment supports. The one thing that can start earlier is the API validation, because it produces notes rather than code. Step 1 is that validation, before any implementation: a checklist of things currently taken from documentation, including the measurement that settles 100ms versus 5000ms, with captured bodies saved as fixtures so the stubbed tests replay real payloads. Step 2 deliberately does not decide where the code lives, but records that the dependency argument is weaker than it looked - System.Net.Http needs no package reference on any target we ship - so the question is judgement, not constraint. Also records that this branch becomes the feature branch when step 0 clears, and that the way to take the merge is to rebase onto main rather than merge main in, since #3191 squash-merges. * Step 0 is cleared; note that step 1 needs the machine with cluster access #3191 merged as dc915bb, so this branch is rebased onto it - which is what step 0 asked for, and rebased rather than merged because #3191 squash-merged and a merge would have moved the merge base without bringing its ancestry. The fault-injector tier is present here now. Also recorded where the work can happen. Step 1 is validation against a real deployment plus the fault injector, and both live on the machine holding the environment directory; this box has no ~/aws and no env_output.json anywhere, so nothing in step 1 could be run or verified here. Worth writing down rather than rediscovering, since the environment being absent looks identical to the environment being offline. Steps 2 onward are ordinary code and are not tied to a machine. Noted too that the directory is the AWS multi-cluster template, so env_output.json nests its outputs under .clusters.value[0] - a shape FaultInjectorEnvironment already handles, so it needs no work, just no surprise. * Pin the cluster REST certificate per host rather than chaining to the config CA The 9443 certificate is self-signed by the cluster itself; the CA in the config directory only signs the RESP proxy certificates, so the chain build could never pass. Pin the leaf on first use, keyed by host so multi-cluster templates work. * Step 1 measured: the lag check is Active-Active only, and does what it says there Records the real-deployment results (findings section 10): off Active-Active the lag check fails unconditionally; on it, the stale member flips within seconds of sync pausing and the tolerance is honoured on a running link. Also the two integration problems found in Availability/, which mean src/ needs a change. * Correct the client defaults: Lettuce, Jedis and redis-py all default to 5000 ms Read from source: redis-py's 5000 contradicts its own docstring's 100. None of the three checks for Active-Active, and none has a third result state. * Measure what a probe can discover: not Active-Active over RESP, db_viewer suffices Active-Active membership has no RESP signal, so it comes from REST (fields=crdt) or configuration. db_viewer can call everything a probe needs, but unfiltered bdb/crdb reads expose database passwords even to it, so only filtered requests. The REST endpoint derives from the configured database hostname. * TLS reuse, the cross-client survey, and the local form routed by database host TrustIssuer(leaf) via TlsOptions validates the 9443 call unchanged. No other client activates lag checks automatically or checks crdt; go-redis routes the local form through each database host, which makes it truthful, and scopes the lag verdict to failback only. * The local form flips under paused sync exactly as the database form does Same verdict, same timing, stale member only, tolerance ignored while paused. Its failure code is bdb_endpoint_unavailable, shared with the wrong-node case, so the choice between forms is about call shape, not information. * Sketch the lag-aware API for review A failback-only health check per member rather than a new result value, a member-scoped probe, and a LagAware factory whose only required input is the management credential. Compared against Lettuce, Jedis, redis-py and go-redis. * API sketch: System.Text.Json decided; open questions compared with other clients Keep LagAware naming and failback (not fallback) terminology; rotating NetworkCredential callback plus a separate client-certificate option; and keep the liveness fallback, which no client has, because a dead peer probably makes the survivor's lag check fail. * Partition test: the lag check is blind for a minute, then fails on both sides Partitioning the clusters (tagged iptables rules, self-healing) showed no lag for ~60 s while sync had stopped, then tolerance-governed failure on both members; a tolerance of 0 is honoured. Supports the liveness fallback, and adds a caveat: failback in the first minute after a link breaks is not protected. * AllSuccess: a run of only inconclusive probes is not a pass The default policy reported Healthy when no probe failed, even if none succeeded. Harmless for liveness, where both count as connected, but wrong for any caller that distinguishes them. * Failback health checks: ask whether a member has caught up before moving to it (SER011) ConnectionGroupMember.FailbackHealthCheck (and a group default) is consulted only for members that are not active, and only Healthy makes them eligible. It never evicts the active member, and when no live member is eligible, selection falls back to liveness, since a partition leaves every region behind at once. HealthCheckProbe.Scope lets a probe run once per member rather than per endpoint. * Notes: core change implemented behind SER011 (SER007 is retired) * HealthCheck.LagAware: ask Redis Enterprise whether a member is available and caught up (SER011) A member-scoped probe of the database availability API. Only the management credentials are required: the REST endpoint derives from the configured host, the database is discovered with filtered requests only, the lag question is asked only of Active-Active databases, and no tolerance is sent unless set. Could-not-ask is Inconclusive, never Unhealthy. System.Text.Json on the down-level targets; custom certificate validation is unavailable on net461. * Notes: probe implemented; RawConfig is not public after all * LagAware: ask the cluster name, and never follow redirects Found against live clusters: non-master nodes answer the management routes (discovery, the crdt lookup) with a 307 to the master's internal address, so deriving the REST endpoint from the database host timed out for three of four members. Derive the cluster FQDN (strip redis-{port}.), which resolves to the master, and report a redirect as Inconclusive rather than following it. * Notes: management routes redirect to the master's internal address * Fault-injector scenario: refuse to fail back onto a stale Active-Active member Pauses sync into the preferred member through its cluster's REST API while writes go to the other, and asserts the group stays off it for 15s although it is alive, then fails back once it has caught up. The tier now reads every cluster's credentials, and shares its certificate pin with the probe. * Notes: tier-3 scenario test passing * LagAware: client certificates for the REST call LagAwareOptions.ClientCertificate, with SetUserPfxCertificate and (net6.0+) SetUserPemCertificate mirroring ConfigurationOptions and sharing its loaders. Wired into SocketsHttpHandler on .NET and HttpClientHandler down-level; not supported on net461. Every other client offers mTLS for this call. * Failback: a member chosen by the liveness fallback is provisional Other clients never select a member that fails its lag check; the nearest equivalent that still serves when every member is behind is to leave such a member for an eligible one as soon as one is available, whatever the weights, and to keep checking it so it becomes an ordinary active member once caught up. * Notes: fallback choice is provisional, as decided by client parity * Move the lag-aware log message to a free event id main now uses 124 and 125 (#3257); the guard test from #3266 rejects duplicates. * AnySuccess: a run of only inconclusive probes is not a failure AnySuccess reported Unhealthy whenever nothing succeeded, even when nothing failed either - so a probe that cannot tell (a key-write probe on a replica, say) marked its member unhealthy. Now Inconclusive, matching AllSuccess and MajoritySuccess. * Move the lag-aware log message to a free event id (again) main has since taken 128 as well. * Failback: a zero FailbackDelay clears a failure in the same clock tick; no ValueTuple UpdateState compared the last failure strictly before the cutoff. With FailbackDelay zero the cutoff is now, and on .NET Framework's ~15.6ms clock a failure and the passing check that clears it often share a tick, holding the member out for another interval. Compare inclusively. Also replace a named tuple in the failback-check pass: the library must not reference System.ValueTuple. * Ship lag-aware failback without an experiment Remove SER011: the attributes, the constant and NoWarn entry, the [SER011] API prefixes and docs/exp/SER011.md. It never shipped, so it is not a reserved id. Also restores main's using directives and encoding in the files that already existed there, which earlier edits had disturbed. * Notes: no experiment * Notes: drop the SER011 history * Do not abandon a connection attempt before ConnectTimeout has elapsed (#2894) (#3277) The heartbeat's Connecting branch aborted an in-flight attempt as soon as the retry policy said a retry was due. With the default ExponentialRetry at half the connect timeout that is ~2.5s, so any attempt whose name resolution or TLS handshake took longer than that was killed inside its own ConnectTimeout budget and reported as a connect timeout. A primary DNS server that does not answer costs ~5s per lookup on glibc, so on Linux every reconnect attempt died during resolution and the connection never left Connecting. Require ConnectTimeout to have elapsed before the retry policy may abort an attempt. The socket connect already carries its own ConnectTimeout cancellation; this check bounds TLS and the handshake after it. * Read HELLO "server" before trusting "version" as the Redis API version (#3279) * Read HELLO "server" before trusting "version" as the Redis API version (#3278) Valkey reports its own version as HELLO "version" ("server" => "valkey", "version" => "9.1.2") while INFO still says redis_version:7.2.4; Garnet keeps "version" as the Redis API version ("server" => "redis") and adds "garnet_version". Both the interactive and subscription connections issue HELLO, so the detected version depended on whether the last reply processed was HELLO or INFO, and version-gated commands (DELEX for LockRelease) were sent to servers that don't have them. Now "version" only sets the Redis API version when "server" is redis; for a recognised non-Redis server it feeds the product variant instead, and for an unrecognised server it is ignored. A "<vendor>_version" HELLO field sets the product variant in the Garnet shape. The in-process test server can emulate both measured shapes, and a HELLO-only test (INFO disabled) pins the rule down on RESP2 and RESP3. * Snapshot the HELLO "version" reader instead of allocating a string Parse it as a Version (no string) once we know "server" is redis; only the vendor path, which stores the string anyway, materializes it. * Parse the HELLO "version" straight from UTF-8 Adds Format.TryParseVersion(ReadOnlySpan<byte>): net10.0 has a built-in UTF-8 Version.TryParse; elsewhere a Utf8Parser loop over the dotted components, so no char transcoding and, downlevel, no string. The HELLO handler uses it via the function-pointer TryParseScalar overload; a parity test keeps it in step with the char/string overloads. * Use IsScalar on the snapshot reader instead of a separate flag * Match the HELLO "server" name case-insensitively * Pin the raw Version parts (unspecified = -1) for the UTF-8 parser, and the gate normalisation System.Version treats "7.4" as (7, 4, -1, -1) and "7.4.0" as (7, 4, 0, -1); the UTF-8 overload must produce the same raw parts as the char/string overloads, and IsAtLeast/IsEqual must then treat the unspecified parts as zero. * UTF-8 TryParseVersion: defer to the char overload for exact parity The goal is identical results to the pre-existing char path for identical text, whatever System.Version does with it (-1 for unspecified parts, whitespace and sign tolerance per part), not a tidier parser. net10.0 fast-paths the built-in UTF-8 Version.TryParse; anything it rejects, and every other TFM, transcodes and calls the char overload. On .NET Framework that still allocates a string, which is acceptable. The parity test now covers the whitespace/sign cases. * Sentinel soft-fail follow-ups: discovery failure as RedisConnectionException, late sentinel init, clear LastException (#3280) Follow-ups to #3187 (AbortOnConnectFail=false for sentinel), prompted by the investigation of #3271: - GetSentinelMasterConnection with a config that has no endpoints of its own and nothing discovered threw AggregateException(InvalidOperationException "No nodes to consider") from the seed connect. It now throws the discovery RedisConnectionException, as it did before #3187. - A sentinel connection whose initial connect soft-failed never ran InitializeSentinel, because only the connect paths called it; such a connection never subscribed to +switch-master and tracked failovers only through the fallback polling. InitializeSentinel is now idempotent and is also reached from a non-first reconfigure that detects a sentinel. - The discovery failure recorded as LastException at connect time stayed there after recovery and was folded into every later "no connection available" message. It is cleared once the managed connection is restored. - Document what abortConnect means for sentinel connections. * Sentinel: re-resolve the primary when it is demoted on a healthy connection (#3281) * fix(sentinel): re-resolve the primary when it is demoted on a healthy connection A sentinel-managed connection notices an in-place demotion of its primary (INFO replication reports role:slave) but only re-elects among the nodes it already knows. When sentinel promoted a node at an address the client has never seen and the +switch-master announcement was missed, there is no primary among them and nothing asks sentinel again: writes fail with "No connection (requires writable - not eligible for replica)" until restart. The sentinel connection's heartbeat now checks, from local state only, whether each managed connection's sentinel-reported primary is connected and reporting itself as a replica (or was never added). If so, a recovery attempt runs off the heartbeat thread, at most one per managed connection: - it asks sentinel first; no answer: retry on a later heartbeat; - sentinel names a node held as a connected replica (sentinel has not failed over yet, or our view of that node's role is stale): re-read that node's role with one INFO replication, rather than reconfiguring every node once per heartbeat for as long as sentinel lags (where INFO cannot be sent, fall through to the switch rather than ask sentinel forever); - otherwise switch, as +switch-master would have. An attempt is skipped while the managed connection is already reconfiguring, or once either multiplexer is disposed; disconnected primaries are left to the existing reconnect timer. The heartbeat scan also forgets disposed children, as the +switch-master handler does. Tests: an in-process sentinel (InProcessSentinelServer) and a standalone SetReplicaOf on the toy server. The demotion-to-unknown-address test fails without the fix (no primary at all) and passes with it; a second test holds sentinel on the demoted node and checks that sentinel is still polled but the connection is not reconfigured on every heartbeat, then follows sentinel once it catches up. Refs #1891, #2487, #1670 * Review follow-ups: make SwitchPrimary claim the reconfigure gate before reshaping the connection - SwitchPrimary now claims the reconfigure gate (activeConfigCause) BEFORE rewriting the endpoint list or retiring servers, via new TryClaimReconfigure/ReleaseReconfigureClaim and a 'claimed' handover parameter on ReconfigureAsync. Previously a reconfigure that started between RecoverDemotedPrimary's pre-checks and ReconfigureAsync's own CAS would refuse the switch's reconfigure AFTER the connection had been reshaped, leaving retired servers and no primary for at least a heartbeat; concurrent switches could also interleave their Clear/TryAdd endpoint rewrites. A lost race now touches nothing and the heartbeat retries. - RecoverDemotedPrimary hands sentinel's already-validated answer to SwitchPrimary (reportedPrimary), so the switch acts on the answer the guards evaluated instead of re-querying and possibly acting on a different one while sentinel flaps - also removing redundant blocking sentinel round-trips from the claimed section. - A successful demotion recovery clears connection.LastException (as the restore handler does) once the reported primary is connected as a primary, so a stale soft-connect discovery failure stops polluting timeout messages after the client is healthy again. - currentSentinelPrimaryEndPoint is now volatile: it is written by SwitchPrimary on worker/handler threads and read by the heartbeat demotion check. - The disposed-child cleanup shared by the +switch-master handler and the heartbeat sweep is extracted into ForgetDisposedChild. - DemotedPrimaryDoesNotReconfigureWhileSentinelStillNamesIt no longer uses wall-clock quiet detection plus a fixed observation window with an exact event count: it waits for a stretch of three sentinel polls with zero configuration changes, re-baselining on stragglers - deterministic on loaded machines, and faster. - docs/Configuration.md documents how a sentinel-managed connection tracks failovers, including demotion-in-place re-resolution. --------- Co-authored-by: mgravell <marc.gravell@gmail.com> * Route as a cluster only with a slot map; take HELLO mode/role from the interactive connection only (#3285) * Route as a cluster only with a slot map; take HELLO mode/role from the interactive connection (#3284) A proxied deployment can answer HELLO with "mode: cluster" while INFO says redis_mode:standalone and every CLUSTER subcommand is refused (Azure Managed Redis with the Enterprise clustering policy). Under RESP3 pub/sub now has its own connection (#3264), whose handshake is HELLO alone: when that leg alone reconnected, its HELLO retyped the server as a cluster with no INFO to follow, the resulting reconfigure put the multiplexer into cluster mode with no slot map, and cross-slot commands were refused client-side. - Cluster *routing* now requires a slot map: servers that identify as a cluster but give us no map are routed as a single standalone server (new event id 133). The map is never cleared once populated, so an established cluster whose refresh is refused (#3275) keeps routing as one. When a map does arrive, stale redundant-primary flags from an earlier election are cleared. - AnyServer accepts a cluster-typed server while routing as standalone (ServerType.IsRoutableAs), so such a server stays a valid target. - HELLO "mode" and "role" are applied only from the interactive connection; the subscription connection does no discovery and must not overwrite server-level facts. Tests: HelloClusterModeUnitTests reproduces the subscription-only reconnect under both protocols (fails under RESP3 before this change); ClusterCommandsRefusedUnitTests asserts standalone routing and a working cross-slot MGET when CLUSTER is refused. The in-process server gains an overridable HelloModeValue so HELLO and INFO can disagree. * Derive the slot-map check from allocation; reject inverted or out-of-range slot ranges The routing decision in ReconfigureAsync needs only "is there any slot map", not a count. Add ServerSelectionStrategy.IsSlotMapEmpty, derived from whether the map has been allocated: the map is allocated only immediately before a write, writers never store null, and nothing clears an entry, so a non-null map always covers at least one slot. CountCoveredSlots remains for the log line in the cluster branch, where it has always been. That derivation is only sound if nothing can allocate without writing, so UpdateClusterRange now ignores an inverted or out-of-range range instead of allocating and then writing nothing (or indexing out of bounds). SlotRange does not validate ordering and the reply-driven paths build ranges straight from what the server sent. SlotMapUnitTests covers the rejected ranges leaving the map unallocated. * Fail connection-dropped commands with RedisConnectionException, never raw or cancelled (#3283) (#3287) When a connection failed, most queued commands failed with RedisConnectionException but a few received the raw transport exception (IOException, or the RESPite writer's InvalidOperationException) or a TaskCanceledException with no token in play. Three sources, all confirmed with the reporter's repro: - RecordConnectionFailed builds its detailed RedisConnectionException only for the bridge's current connection and only on the first report; a repeat report (the write path and the read loop both reporting), or one against a connection the bridge has already replaced, drained the queue with the raw inner exception - or with null, which the result box turns into a cancellation (every EOF path in the read loop reports with no exception). - The write-lock catch fails the message properly and then reports the connection, which drained the same still-queued message again with the raw exception; last write wins. - FailDueToNoConnection, TimedOutBeforeWrite and the fail-fast bin completed the message via Cancel(); ExecuteAsyncImpl then races to fault the task with the real exception, and the box's thread-pool continuation sometimes wins. A batch has no caller translating the result at all. Fix: the drain always uses a RedisConnectionException (wrapping whatever it was given, with the failure type), and the bridge's refusals set the exception the caller path would have built instead of cancelling. Message.Cancel itself is unchanged; transactions rely on it for aborted conditions. ConnectionFailureExceptionTests covers the drain (null and IOException inner, on a bridge-less connection, i.e. the non-current path) and the no-connection refusal observed at the message's own task. Reporter's repro, 90 s: 884 TaskCanceledException + 415 InvalidOperationException before; none after. Fixes #3283 * Tests: tolerate clock skew on absolute hash-field expiries; wait for quiescence before asserting backlog counters (#3288) Two of the recurring Windows CI flakes. HashFieldTests' DateTime-based expiries are computed on the client clock and read back on the server's (in WSL), and the TTL came back as 7200001ms against a hard 7200000 ceiling: five tests in one run. BacklogTests.TotalOutstandingIncludesBacklogQueue asserted TotalOutstanding == 0 the instant the last backlogged ping completed, while the reconnect's handshake traffic was still in flight (seen as 7 and 1; six failures in the last twelve days). * Connection groups: a member answering LOADING triggers an immediate re-probe and re-select, like a breaker trip (#3289) The group's fast-path only reacted to ConnectionFailureType.CircuitBreaker. A server that is loading fails every reconnect during the handshake, so commands routed to it die in the backlog without a reply, and the breaker only observes replies - it can never be what moves traffic away. This is the shape of the recurring Windows CI failure of WithRetry_FailsOverBetweenGroupsOnLoading (six attempts in twelve days, always 20 x 5000ms backlog timeouts): when the heartbeat rather than the test's command met the first LOADING, nothing ever failed over. LOADING now joins CircuitBreaker in the fast-path; the health check still decides where traffic goes, so a member whose probe passes is kept (WithRetry_WhenFailoverNeverArrives_KeepsRetryingSameMember). New LoadingRerouteTests fails without the change. * IBatch: complete commands refused by a poisoned connection; divert a batch's tail on reconnect (#3286) * Complete a message refused by a poisoned connection; divert a batch's tail on reconnect (#3282) Since #3205 a connection whose write faulted is poisoned and refuses further writes, so that no reply can be matched to the wrong caller. That refusal returned WriteFailure without completing the message. Single commands were unaffected, as ExecuteAsyncImpl/ExecuteSyncImpl translate a non-success result into an exception for the caller; but RedisBatch.Execute writes a list via PhysicalBridge.TryEnqueue, which does not act on the result, so once one message of a batch faulted the connection, the rest of that batch was neither queued nor completed and its tasks were awaited forever (3.2.0 to 3.4.5). - WriteMessageToServerInsideWriteLock: the IsWriteFaulted refusal now fails and completes the message, as every other refusal to write already does (FailDueToNoConnection, TimedOutBeforeWrite, QueueOrFailMessage, the write-lock catch, the transaction sub-command loop). - TryEnqueue(list): once NeedsReconnect is set part-way through the batch, the remaining messages take the same path a single command would at the top of TryWriteSync - backlog or fail, per policy - instead of being written to a connection known to be dead. BatchWriteFailureTests injects a write failure mid-batch under both backlog policies and asserts that every task completes and that any successful result is real. The reporter's repro (400 batches in flight, CLIENT KILL in a loop) hung within 13 s on main and ran 6.6 M batches in 120 s with this change. Fixes #3282 * Batch: route the tail to the backlog when the held connection is poisoned, not just while the reconnect flag is set The reconnect flag is cleared as soon as TryConnect starts a replacement connection, on another thread; a batch still holding the previous PhysicalConnection could then write its remaining messages to it and have them refused with WriteFailure. Seen on the Windows CI runner (2 of 3 attempts); reproduces locally 12/25 pinned to two cores. Also make the test's fail-fast variant wait for the reconnect before its verification read, which is the other way the two-core run failed. * Tests: don't race the connect's own reconfigures in AllClusterCommandsRefusedIsQuiet (#3290) A freshly connected multiplexer fires fire-and-forget reconfigures of its own (connection-restored per physical connection, autoconfigure field changes), and ReconfigureAsync returns false while one is in flight. The test asserted on that return value directly, so it lost the race on the Windows runner (main @ 74c52bf, net10.0/RESP3). Hoist the retry loop HelloClusterModeUnitTests already used for this into Poll.ReconfigureAsync and use it in both tests; dump the captured log after the assertions so a future failure shows what was actually logged. * Send primary-only commands to the primary when the caller prefers a replica (#3291) * Send primary-only commands to the primary when the caller prefers a replica KeyDelete, KeyExpire (milliseconds), LockExtend and LockRelease pick a server up front to check its features. With PreferReplica that picked a replica, and the primary-only message was then rejected with 'Command cannot be issued to a replica'. GetFeatures now upgrades the flags the same way Message does. Fixes #2821 * Mirror Message.SetPrimaryOnly exactly; explain the PEXPIRE expiry in the test --------- Co-authored-by: Marc Gravell <marc.gravell@gmail.com> * Give multi-message sub-commands and injected prefix commands a write time (#3292) (#3293) Sub-commands of an IMultiMessage (WATCH, condition reads, MULTI, the queued operations; also SCRIPT LOAD and PTTL) are written inside the lock without passing through SetEnqueued, as are the SELECT/READONLY/ASKING/PREPARE prefixes injected ahead of a command. With a zero write time the heartbeat sees them as overdue by the process uptime; since #2965 such no-box messages count as sync timeouts, and together with an idle connection that satisfies the dead-socket heuristic while the reply is still in flight. Sub-commands now inherit the parent's write time (sharing its budget, including backlog time); prefix commands are stamped when written, via a single helper replacing four identical blocks. * Refresh package pins: 10.0.13 servicing family and tooling (#3294) Shipped BCL dependencies move 10.0.5 -> 10.0.13 (System.IO.Hashing 10.0.12 -> 10.0.13); test/tooling packages to current. Removes the unreferenced PerformanceCounter pin and documents that the StackExchange.Redis 2.13.17 pin is the baseline projects' previous-major comparison, not a lagging version. Analyzer.Testing 1.1.4 brings NuGet.Packaging 7.0.3 -> System.Security.Cryptography.Pkcs 9.0.6, which NuGet audit rejects; a direct Pkcs 10.0.13 reference in Build.Tests lifts it. Held back for separate PRs: xunit.v3 4.x (needs Microsoft.Testing.Platform 2.5+, which drops the VSTest bridge under the .NET 10 SDK, plus discoverer/attribute changes) and StyleCop 1.3-beta (92 new rule hits in src). * xunit.v3 4.0 on Microsoft.Testing.Platform (#3295) * xunit.v3 4.0 on Microsoft.Testing.Platform xunit.v3 4.x depends on xunit.v3.mtp-v2, which needs Microsoft.Testing.Platform 2.5+, and that drops the VSTest bridge under the .NET 10 SDK. So: global.json selects the MTP runner for dotnet test, the four test projects set UseMicrosoftTestingPlatformRunner and stop referencing the VSTest adapters (Microsoft.NET.Test.Sdk, xunit.runner.visualstudio, xunit.v3.runner.console), and CI.yml/build.ps1 use the MTP dotnet test shape (--project / --solution, --report-xunit-trx; the GitHub Actions reporter self-registers as an extension). xunit 4 fallout: the per-protocol TheoryDiscoverer overrides the new 6-parameter CreateTestCasesForDataRow; the fault-injector tests use [Parallelization(Mode = None)]; xUnit1069 is suppressed for the round-trip tests, whose Timeout is a hang guard; net481 regains System.Reflection.Metadata and Microsoft.CSharp, previously transitive via Test.Sdk. * CI: the Ubuntu composite action also moves to the MTP dotnet test shape; enable the GitHub reporter --report-github is off by default under Microsoft.Testing.Platform (the VSTest logger was on by name), so both jobs ask for it explicitly, with the summary limited to failures as before. * Follow-ups for the MTP runner: analyzer tests join the solution, build.ps1 wording and framework, docs Build.Tests was never in StackExchange.Redis.slnx, so dotnet test --solution skipped it. The -NetCoreOnlyTests path moves to net10.0: two test projects have no net8.0, and the runner fails a project lacking the requested framework instead of skipping it. AGENTS.md, the skills and the fault-injector README use the --project / --filter-class forms. * Use the platform TRX reporter; observe the test-side tasks the new runner reports as unobserved xunit 4.0.2's --report-xunit-trx wrote a truncated file for the 6.8k-test run (ends mid-element; dorny/test-reporter: 'Invalid XML'), locally as well as in CI. Microsoft.Testing.Extensions.TrxReport's --report-trx produces a complete report (validated: parses, 6828 UnitTestResult elements). Microsoft.Testing.Platform also prints every UnobservedTaskException at the end of the run, which VSTest never showed. All of the ones seen are in test code, not the library: the in-process server's background work logging to xunit after its test ended (xunit v3 throws 'no currently active test'); BatchWriteFailureTests never reading its Task.WhenAll's exception; TcpForwarder's losing copy direction. Each now observes or swallows its own failure. * VectorSetInfo: vset-uid is a zero-based counter, so do not assert it is non-zero Measured on a fresh test server: the first vector set created after startup reports vset-uid 0, the next 1, and so on. CI starts a fresh server every run, and the xunit 4 discovery order makes VectorSetInfo(Binary) under RESP2 the first test to create one, so the old assertion failed there and nowhere else. Check the parsed value against the raw VINFO reply instead. * StyleCop.Analyzers 1.2.0-beta.556 -> 1.3.151-beta, and the tidy-up it asks for (#3296) New rule hits, all fixed rather than suppressed: SA1407 (parenthesise comparisons combined with the non-short-circuit & and |), SA1143 (multi-line conditionals use the expanded layout), SA1504 (an accessor pair is all single-line or all multi-line), SA1315 (override/implementation parameter names match the member they implement), SA1653 (<content> on secondary partial declarations), plus single SA1117/SA1413/SA1500 sites. 46 sites in RESPite, 175 in StackExchange.Redis, 3 in RESPite.Benchmark. SA1315 found a real bug: KeyPrefixedDatabase.SortedSetRangeAndStore (the sync, key-prefixed overload) declared its first two parameters as (destination, source) against the interface's (source, destination) and forwarded them by name, so it read from the destination and wrote into the source. Regression test: WithKeyPrefixTests.SortedSetRangeAndStoreUsesTheRightKeys. Public API: AzureOptionsProvider/AzureManagedRedisOptionsProvider.AfterConnectAsync rename 'muxer' to the base's 'multiplexer' (recorded in PublicAPI.Unshipped); every other rename is on an internal type or an explicit interface implementation. * Port main's fixes onto the new core by intent (merge follow-up): #3291, #3279, #3285, #3275, #3281, #3289, #3287, #3286 - #3291 (#2821) primary-only + PreferReplica: RespConnectionManager.EndpointForAny rewrites PreferReplica to DemandMaster for a primary-only command before the selector sees the flags, as Message.SetPrimaryOnly did; DemandReplica still fails. KeyTests.PrimaryOnlyCommandsWithPreferReplicaRunOnThePrimary passes. - #3279 (#3278) HELLO "server" before "version": HelloHandler reads the fields through HelloFieldMetadata; "version" is the Redis API version only when "server" is redis (absent counts as redis), a "{vendor}_version" names the product, a known non-Redis server's "version" is its own and the API version comes from INFO or the configured default, an unknown "server" is logged; the handshake applies the product variant from HELLO. ProductVariantUnitTests and ValkeyUnitTests pass. - #3285 (#3284) route as a cluster only with a slot map: RespTopology.RoutesBySlot now also wants a map, so a server that announces "cluster" while refusing CLUSTER subcommands routes as standalone (cross-slot commands go to the proxy) until a map arrives from the handshake, a refresh or a -MOVED; the refresh sweep asks SLOTS whenever the deployment may be a cluster (NeedsSlots). HELLO mode and role are published only by the interactive connection (or a reply that carried slots), so a subscription reconnect's HELLO alone cannot flip the type. RespMultiplexerExecutorTests give their topology a map where they declare a cluster; HelloClusterModeUnitTests subscribes first, since this core opens the subscription connection on first use. - #3275 follow-ups: a refused CLUSTER SLOTS is logged by the handshake, and GetEndpointsFromClusterNodes asks for the SLOTS view when only NODES is cached (ReadSlotsViewAsync), so a server that keeps refusing says so on every pass; ClusterViews is a struct, not a tuple (SanityChecks.ValueTupleNotReferenced). - #3281 sentinel demotion on a healthy connection: the heartbeat re-reads each established interactive server's role (ROLE via RespConnectionManager.TryRefreshRole), paced by ConfigCheckSeconds with the existing exponential ramp (ServerEndPoint.RoleCheckDue/OnRoleChecked), which is what the shipped bridge's CheckInfoReplication did and what RecoverDemotedPrimary needs. Also found: the new core validated commands against RawConfig.CommandMap rather than the multiplexer's effective map, so a sentinel multiplexer (CommandMap.Sentinel) refused SENTINEL when the caller's map lacked it; every RawConfig.CommandMap read in the core now uses the effective map. SentinelPrimaryDemotionUnitTests pass; SentinelTests against the real sentinels still pass. - #3289 LOADING steers a connection group away: the endpoint executor's outcome observer reports the first LOADING reply of an episode through ConnectionFailed(Loading), re-armed by a later success. Deliberately WITHOUT dropping the connection, which the shipped core did: a loading server answers everything with LOADING, its handshake included, so the drop only bought a reconnect loop, and a retrying caller then found the member mid-dial and was moved off it. LoadingRerouteTests and RetryEndToEndTests pass. - #3287 (#3283) and #3286: this core already fails connection-dropped commands with RedisConnectionException and completes a poisoned batch; ConnectionDroppedExceptionTests pins both from the public surface, replacing the two tests dropped in the merge because they drove v3 internals. - #3277 (#2894), #3273, #3293: their tests (ConnectionAttemptUnitTests, TransportCloseTests, MultiMessageWriteTimeTests) pass on this core as merged; nothing to port. Main suite net10.0: 7893 passed, 0 failed, 167 skipped. * StyleCop.Analyzers 1.3.151-beta on v4 (main #3296 reached the new core): the 79 sites the shared tidy could not see Held at 1.2.0-beta.556 through the main merge so that the merge and the intent ports could be judged on their own; this takes main's pin and answers what 1.3 says about v4-only code. All 79 in src/StackExchange.Redis: - SA1653 (54): partial types documented in every part. The primary file keeps its <summary>; the other parts (RedisDatabase.*.cs, the Groups *.Methods/*.Types/*.Sort parts, RespClientCache.Slabs.cs) use <content>, as main's tidy did for its own partials. - SA1143 (11): ternaries laid out with ? and : each starting a line; two multi-line conditions (PFCOUNT's replica safety, XREADGROUP's read-only category) moved into a named local first. - SA1504 (8): a single-line getter beside a multi-line setter/init made block-bodied. - SA1315 (5): IInternalConnectionMultiplexer implementations take the interface's parameter names (endPoint, type). - SA1407 (1): parenthesised the comparison beside a non-short-circuit &. No behavioural change. Main suite net10.0: 7893 passed, 0 failed, 167 skipped. * Merge fix-up: restore v4's CI.yml (the --theirs resolution had replaced the whole file), and the report-github reporter for the two small test projects - CI.yml: the merge took main's file wholesale for a one-hunk conflict, dropping v4's Windows server setup - the diskless replica load and the wait for every replica to sync and be listed by CLUSTER SLOTS - as well as the v4 branch triggers and the 30-minute test step timeout. Without the replica wait, no replica on the Windows job ever syncs, CLUSTER SLOTS lists none, and the replica-routing tests fail ("only 0 slots carried replicas"). This is v4's file with main's one change applied: the Microsoft.Testing.Platform `dotnet test` invocation. VSTest's --blame-hang has no platform equivalent without the HangDump extension; the step timeout stays as the backstop. - RESPite.Tests and Build.Tests: the CI action passes --report-github to every test project, which is the GitHubActionsTestLogger package's platform extension; the merge had dropped that reference from these two, so their steps exited 5 (invalid arguments, zero tests ran). Verified locally with the action's exact flags: 1960 and 188 pass. --------- Co-authored-by: Diogo Martins <165835485+MDA2AV@users.noreply.github.com> Co-authored-by: rebaseandpanic <rebaseandpanic@proton.me> Co-authored-by: AsadIqbal-Software Engineer <asadiqbaldst2023@gmail.com>
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
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