Repository navigation
Read HELLO "server" before trusting "version" as the Redis API version - #3279
Merged
Merged
Conversation
#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.
Parse it as a Version (no string) once we know "server" is redis; only the vendor path, which stores the string anyway, materializes it.
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.
…d 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.
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.
FloatingMilkshake
pushed a commit
to FloatingMilkshake/MechanicalMilkshake
that referenced
this pull request
Oct 24, 2026
![Renovate [bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [StackExchange.Redis](https://seredis.dev/) ([source](https://github.com/StackExchange/StackExchange.Redis)) | nuget | patch | `3.4.0` → `3.4.20` | --- ### Release Notes <details> <summary>StackExchange/StackExchange.Redis (StackExchange.Redis)</summary> ### [`v3.4.20`](https://github.com/StackExchange/StackExchange.Redis/releases/tag/3.4.20) [Compare Source](StackExchange/StackExchange.Redis@3.4.5...3.4.20) #### Stability improvements - Do not abandon a connection attempt before ConnectTimeout has elapsed by [@​mgravell](https://github.com/mgravell) in [#​3277](StackExchange/StackExchange.Redis#3277) - Read HELLO "server" before trusting "version" as the Redis API version by [@​mgravell](https://github.com/mgravell) in [#​3279](StackExchange/StackExchange.Redis#3279) - Sentinel soft-fail follow-ups: discovery failure as RedisConnectionException, late sentinel init, clear LastException by [@​mgravell](https://github.com/mgravell) in [#​3280](StackExchange/StackExchange.Redis#3280) - Sentinel: re-resolve the primary when it is demoted on a healthy connection by [@​rebaseandpanic](https://github.com/rebaseandpanic) in [#​3281](StackExchange/StackExchange.Redis#3281) - Route as a cluster only with a slot map; take HELLO mode/role from the interactive connection only by [@​mgravell](https://github.com/mgravell) in [#​3285](StackExchange/StackExchange.Redis#3285) - Fail connection-dropped commands with RedisConnectionException, never a raw transport exception or a cancellation by [@​mgravell](https://github.com/mgravell) in [#​3287](StackExchange/StackExchange.Redis#3287) - Connection groups: a member answering LOADING triggers an immediate re-probe and re-select by [@​mgravell](https://github.com/mgravell) in [#​3289](StackExchange/StackExchange.Redis#3289) - IBatch: complete commands refused by a poisoned connection; divert a batch's tail on reconnect by [@​mgravell](https://github.com/mgravell) in [#​3286](StackExchange/StackExchange.Redis#3286) - Give multi-message sub-commands and injected prefix commands a write time by [@​mgravell](https://github.com/mgravell) in [#​3293](StackExchange/StackExchange.Redis#3293) - Send primary-only commands to the primary when the caller prefers a replica by [@​AsadIqbal-Dev](https://github.com/AsadIqbal-Dev) in [#​3291](StackExchange/StackExchange.Redis#3291) #### Maintenance - Tests: stabilise two recurring Windows CI flakes (hash-field expiry clock skew, backlog counter quiescence) by [@​mgravell](https://github.com/mgravell) in [#​3288](StackExchange/StackExchange.Redis#3288) - Tests: don't race the connect's own reconfigures in AllClusterCommandsRefusedIsQuiet by [@​mgravell](https://github.com/mgravell) in [#​3290](StackExchange/StackExchange.Redis#3290) - Refresh package pins: 10.0.13 servicing family and tooling by [@​mgravell](https://github.com/mgravell) in [#​3294](StackExchange/StackExchange.Redis#3294) - xunit.v3 4.0 on Microsoft.Testing.Platform by [@​mgravell](https://github.com/mgravell) in [#​3295](StackExchange/StackExchange.Redis#3295) - StyleCop.Analyzers 1.3.151-beta, and the tidy-up it asks for by [@​mgravell](https://github.com/mgravell) in [#​3296](StackExchange/StackExchange.Redis#3296) #### New Contributors - [@​AsadIqbal-Dev](https://github.com/AsadIqbal-Dev) made their first contribution in [#​3291](StackExchange/StackExchange.Redis#3291) **Full Changelog**: <StackExchange/StackExchange.Redis@3.4.5...3.4.20> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xODEuMCIsInVwZGF0ZWRJblZlciI6IjQ0LjE4MS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.milkshake.services/FloatingMilkshake/MechanicalMilkshake/pulls/10 Reviewed-by: Milkshake <me@floatingmilkshake.com>
FloatingMilkshake
pushed a commit
to FloatingMilkshake/Azusa
that referenced
this pull request
Oct 24, 2026
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [StackExchange.Redis](https://seredis.dev/) ([source](https://github.com/StackExchange/StackExchange.Redis)) | nuget | patch | `3.4.0` → `3.4.20` | --- ### Release Notes <details> <summary>StackExchange/StackExchange.Redis (StackExchange.Redis)</summary> ### [`v3.4.20`](https://github.com/StackExchange/StackExchange.Redis/releases/tag/3.4.20) [Compare Source](StackExchange/StackExchange.Redis@3.4.5...3.4.20) #### Stability improvements - Do not abandon a connection attempt before ConnectTimeout has elapsed by [@​mgravell](https://github.com/mgravell) in [#​3277](StackExchange/StackExchange.Redis#3277) - Read HELLO "server" before trusting "version" as the Redis API version by [@​mgravell](https://github.com/mgravell) in [#​3279](StackExchange/StackExchange.Redis#3279) - Sentinel soft-fail follow-ups: discovery failure as RedisConnectionException, late sentinel init, clear LastException by [@​mgravell](https://github.com/mgravell) in [#​3280](StackExchange/StackExchange.Redis#3280) - Sentinel: re-resolve the primary when it is demoted on a healthy connection by [@​rebaseandpanic](https://github.com/rebaseandpanic) in [#​3281](StackExchange/StackExchange.Redis#3281) - Route as a cluster only with a slot map; take HELLO mode/role from the interactive connection only by [@​mgravell](https://github.com/mgravell) in [#​3285](StackExchange/StackExchange.Redis#3285) - Fail connection-dropped commands with RedisConnectionException, never a raw transport exception or a cancellation by [@​mgravell](https://github.com/mgravell) in [#​3287](StackExchange/StackExchange.Redis#3287) - Connection groups: a member answering LOADING triggers an immediate re-probe and re-select by [@​mgravell](https://github.com/mgravell) in [#​3289](StackExchange/StackExchange.Redis#3289) - IBatch: complete commands refused by a poisoned connection; divert a batch's tail on reconnect by [@​mgravell](https://github.com/mgravell) in [#​3286](StackExchange/StackExchange.Redis#3286) - Give multi-message sub-commands and injected prefix commands a write time by [@​mgravell](https://github.com/mgravell) in [#​3293](StackExchange/StackExchange.Redis#3293) - Send primary-only commands to the primary when the caller prefers a replica by [@​AsadIqbal-Dev](https://github.com/AsadIqbal-Dev) in [#​3291](StackExchange/StackExchange.Redis#3291) #### Maintenance - Tests: stabilise two recurring Windows CI flakes (hash-field expiry clock skew, backlog counter quiescence) by [@​mgravell](https://github.com/mgravell) in [#​3288](StackExchange/StackExchange.Redis#3288) - Tests: don't race the connect's own reconfigures in AllClusterCommandsRefusedIsQuiet by [@​mgravell](https://github.com/mgravell) in [#​3290](StackExchange/StackExchange.Redis#3290) - Refresh package pins: 10.0.13 servicing family and tooling by [@​mgravell](https://github.com/mgravell) in [#​3294](StackExchange/StackExchange.Redis#3294) - xunit.v3 4.0 on Microsoft.Testing.Platform by [@​mgravell](https://github.com/mgravell) in [#​3295](StackExchange/StackExchange.Redis#3295) - StyleCop.Analyzers 1.3.151-beta, and the tidy-up it asks for by [@​mgravell](https://github.com/mgravell) in [#​3296](StackExchange/StackExchange.Redis#3296) #### New Contributors - [@​AsadIqbal-Dev](https://github.com/AsadIqbal-Dev) made their first contribution in [#​3291](StackExchange/StackExchange.Redis#3291) **Full Changelog**: <StackExchange/StackExchange.Redis@3.4.5...3.4.20> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xODEuMCIsInVwZGF0ZWRJblZlciI6IjQ0LjE4MS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.milkshake.services/FloatingMilkshake/Azusa/pulls/10 Reviewed-by: Milkshake <me@floatingmilkshake.com>
mgravell
added a commit
that referenced
this pull request
Oct 24, 2026
… 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 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.
Fixes #3278.
Problem
Since 3.4.0 the
HELLOreply has been a source of the server version, and both the interactive and subscription connections issueHELLO. The handler assignedversionstraight to the Redis API version, but the two conventions in the wild differ (measured against real servers):serverversionredis7.4.3(Redis API version)garnet_version => 2.2.2valkey9.1.2(Valkey's own version)redis_version:7.2.4On Valkey the detected version therefore depended on whether the last reply processed was
HELLO(9.1.2) orINFO(7.2.4), so version-gated commands (e.g.DELEXinLockRelease) were sometimes sent to a server that does not have them.Rule
Decided once the whole
HELLOmap has been read:server => redis(or absent):versionis the Redis API version; a<vendor>_versionkey, if present, sets the product variant (Garnet shape).serveris a recognised non-Redis name (valkey,redict, ...):versionis the vendor version and only feeds the product variant; the API version is left toINFO'sredis_version.serveris unrecognised:versionis ignored (logged). Conservative by design: ifINFOis unavailable too, the version falls back toDefaultVersionrather than guessing.HELLOnever touches the product variant, so it cannot clobberINFO's KeyDB detection (which comes fromexecutable, not a*_versionkey).Verification
HelloServerName,HelloVersionString,HelloExtraFields).ProductVariantUnitTestsandValkeyUnitTestsrun per protocol and assert the API version as well as the variant.DetectProductVariantFromHelloAlonedisablesINFOsoHELLOis the only source; it fails deterministically on RESP2 and RESP3 with the fix reverted. The test vendor version is now9.1.2rather than1.2.3-preview4, because an unparseable version was hiding the bug.Build.csprojRelease build withCI=trueis green.