Preparation work for smart-client-handoffs

.github/workflows/CI.yml

@@ -213,6 +213,19 @@ jobs:
           name: Tests Results - Windows Server 2025
           path: 'test-results/*.trx'
           reporter: dotnet-trx
+          # list only failures: listing every test rendered ~3MB into $GITHUB_STEP_SUMMARY, which has a
+          # 1MB limit, so the summary was dropped entirely and failures had to be read from annotations
+          list-suites: failed
+          list-tests: failed
+      - name: Upload test results
+        uses: actions/upload-artifact@v4
+        if: success() || failure()
+        with:
+          # the trx carries per-test output, which the failure-only report above deliberately omits; that is
+          # the difference between diagnosing a CI-only failure and guessing at it
+          name: test-results-windows
+          path: test-results/*.trx
+          retention-days: 14
       # Package and upload to MyGet only on pushes to main/v3, not on PRs
       - name: .NET Pack
         if: (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/v3')

.gitignore

@@ -31,3 +31,6 @@ launchSettings.json
 TestResults/
 BenchmarkDotNet.Artifacts/
 .codex
+
+# local planning/design notes (not for publication)
+planning/

src/StackExchange.Redis/Availability/FaultContext.cs

@@ -135,6 +135,7 @@ private static bool IsKnownNotApplied(RedisErrorKind kind, CommandStatus status)
             case RedisErrorKind.TryAgain: // slot mid-migration
             case RedisErrorKind.Moved: // wrong node; this one did not run it
             case RedisErrorKind.Ask: // ditto, mid-migration
+            case RedisErrorKind.UnknownRedirectTarget: // a redirect we could not follow, so still not run
             case RedisErrorKind.ReadOnly: // writes refused by a replica
             case RedisErrorKind.Misconfigured: // e.g. persistence failing, so writes are refused
             case RedisErrorKind.NoReplicas: // not enough replicas to accept the write

src/StackExchange.Redis/ClusterConfiguration.cs

@@ -287,10 +287,17 @@ internal ClusterNode(ClusterConfiguration configuration, string raw, EndPoint or
 
             var flags = parts[2].Split(StringSplits.Comma);
 
-            // redis 4 changes the format of "cluster nodes" - adds @... to the endpoint
+            // redis 4 changes the format of "cluster nodes" - adds @... to the endpoint, and the documented
+            // form is now ip:port@cport[,hostname[,aux-field=value]*]; keep all of it rather than just the
+            // address, since a hostname here is the node's second identity (see #2826)
             var ep = parts[1];
             int at = ep.IndexOf('@');
-            if (at >= 0) ep = ep.Substring(0, at);
+            if (at >= 0)
+            {
+                ParseBusPortAndIdentities(ep.Substring(at + 1));
+                ep = ep.Substring(0, at);
+            }
+            AuxFields ??= [];
 
             if (Format.TryParseEndPoint(ep, out var epResult))
             {
@@ -328,6 +335,59 @@ internal ClusterNode(ClusterConfiguration configuration, string raw, EndPoint or
             IsConnected = parts[7] == "connected"; // Can be "connected" or "disconnected"
         }
 
+        // "cport[,hostname[,aux-field=value]*]" - everything after the '@'. Parsed leniently on purpose:
+        // this is topology data, and per the note on the constructor an exception here does silent damage.
+        // Note the hostname slot can be empty while aux fields follow, i.e. "17000,,shard-id=abc"
+        private void ParseBusPortAndIdentities(string trailer)
+        {
+            var fields = trailer.Split(StringSplits.Comma);
+
+            if (Format.TryParseInt32(fields[0], out var busPort)) ClusterBusPort = busPort;
+
+            if (fields.Length > 1 && !string.IsNullOrWhiteSpace(fields[1])) Hostname = fields[1];
+
+            List<KeyValuePair<string, string>>? aux = null;
+            for (int i = 2; i < fields.Length; i++)
+            {
+                var field = fields[i];
+                int eq = field.IndexOf('=');
+
+                // parse by name and keep what we do not recognize; the set is documented as extensible
+                if (eq > 0)
+                {
+                    (aux ??= new List<KeyValuePair<string, string>>(fields.Length - i)).Add(
+                        new KeyValuePair<string, string>(field.Substring(0, eq), field.Substring(eq + 1)));
+                }
+            }
+            if (aux is null)
+            {
+                AuxFields = [];
+            }
+            else
+            {
+                AuxFields = aux.AsReadOnly();
+            }
+        }
+
+        /// <summary>
+        /// Gets the cluster bus port of the current node (the <c>cport</c> after the <c>@</c>), or
+        /// <c>null</c> for servers older than 4.0, which do not report one.
+        /// </summary>
+        public int? ClusterBusPort { get; private set; }
+
+        /// <summary>
+        /// Gets the hostname announced by the current node via <c>cluster-announce-hostname</c>, or
+        /// <c>null</c> if it announces none. This is an additional identity for the node rather than a
+        /// replacement for <see cref="EndPoint"/>, which remains the address the node reports.
+        /// </summary>
+        public string? Hostname { get; private set; }
+
+        /// <summary>
+        /// Gets any auxiliary fields the current node reports after its hostname, as declared. The set is
+        /// documented as extensible, so unrecognized fields are preserved rather than discarded.
+        /// </summary>
+        public IReadOnlyList<KeyValuePair<string, string>> AuxFields { get; private set; }
+
         /// <summary>
         /// Gets all child nodes of the current node.
         /// </summary>

src/StackExchange.Redis/ClusterSlotMetadataKey.cs

@@ -0,0 +1,36 @@
+using System;
+using RESPite;
+
+namespace StackExchange.Redis;
+
+/// <summary>
+/// Keys that can appear in the per-node metadata map of a <c>CLUSTER SLOTS</c> reply.
+/// </summary>
+/// <remarks>
+/// Matched case-insensitively on purpose: the documentation renders these as <c>IP</c>/<c>Hostname</c> in
+/// prose but lower-case in its examples, so the casing cannot be relied upon. The set is documented as
+/// extensible, hence <see cref="Unknown"/> - an unrecognized key is preserved rather than discarded.
+/// </remarks>
+internal enum ClusterSlotMetadataKey
+{
+    /// <summary>A key this library does not recognize.</summary>
+    [AsciiHash("")]
+    Unknown = 0,
+
+    /// <summary>The node's address, supplied when the reported endpoint is some other form.</summary>
+    [AsciiHash("ip")]
+    Ip,
+
+    /// <summary>The node's announced hostname, supplied when the reported endpoint is some other form.</summary>
+    [AsciiHash("hostname")]
+    Hostname,
+}
+
+/// <summary>
+/// Metadata and parsing methods for <see cref="ClusterSlotMetadataKey"/>.
+/// </summary>
+internal static partial class ClusterSlotMetadataKeyMetadata
+{
+    [AsciiHash(CaseSensitive = false)]
+    internal static partial bool TryParse(ReadOnlySpan<byte> value, out ClusterSlotMetadataKey key);
+}

src/StackExchange.Redis/ClusterSlots.ResultProcessor.cs

@@ -0,0 +1,182 @@
+using System;
+using System.Collections.Generic;
+using System.Net;
+using RESPite;
+using RESPite.Messages;
+
+namespace StackExchange.Redis;
+
+public sealed partial class ClusterSlotsResult
+{
+    internal static readonly ResultProcessor<ClusterSlotsResult?> Processor = new ClusterSlotsResultProcessor();
+
+    /// <summary>
+    /// As <see cref="Processor"/>, but also records the result against the server that answered - the
+    /// autoconfigure path needs the side effect, callers of <c>CLUSTER SLOTS</c> do not.
+    /// </summary>
+    internal static readonly ResultProcessor<ClusterSlotsResult?> AutoConfigureProcessor = new ClusterSlotsResultProcessor(autoConfigure: true);
+
+    private sealed class ClusterSlotsResultProcessor(bool autoConfigure = false) : ResultProcessor<ClusterSlotsResult?>
+    {
+        protected override bool SetResultCore(PhysicalConnection connection, Message message, ref RespReader reader)
+        {
+            if (reader.IsNull)
+            {
+                SetResult(message, null);
+                return true;
+            }
+            if (!reader.IsAggregate) return false;
+
+            // the reply is an array of [from, to, primary, replica...]; parse leniently, since bad topology
+            // data does silent damage - drop anything malformed rather than failing the whole reply
+            List<ClusterSlotAssignment>? assignments = null;
+            var ranges = reader.AggregateChildren();
+            while (ranges.MoveNext())
+            {
+                if (TryParseAssignment(ref ranges.Value, out var assignment))
+                {
+                    (assignments ??= new List<ClusterSlotAssignment>()).Add(assignment);
+                }
+            }
+
+            var result = new ClusterSlotsResult(
+                AsReadOnly(assignments));
+
+            if (autoConfigure)
+            {
+                connection.BridgeCouldBeNull?.ServerEndPoint?.SetClusterSlots(result);
+            }
+
+            SetResult(message, result);
+            return true;
+        }
+    }
+
+    private static bool TryParseAssignment(ref RespReader reader, out ClusterSlotAssignment assignment)
+    {
+        assignment = null!;
+        if (!reader.IsAggregate || reader.IsNull) return false;
+
+        var children = reader.AggregateChildren();
+        if (!children.MoveNext() || !children.Value.TryReadInt64(out var from)) return false;
+        if (!children.MoveNext() || !children.Value.TryReadInt64(out var to)) return false;
+        if (from < SlotRange.MinSlot || to > SlotRange.MaxSlot || to < from) return false;
+
+        ClusterSlotNode? primary = null;
+        List<ClusterSlotNode>? replicas = null;
+        while (children.MoveNext())
+        {
+            if (!TryParseNode(ref children.Value, out var node)) continue;
+            if (primary is null)
+            {
+                primary = node;
+            }
+            else
+            {
+                (replicas ??= new List<ClusterSlotNode>()).Add(node);
+            }
+        }
+
+        if (primary is null) return false; // a range with no primary tells us nothing
+
+        assignment = new ClusterSlotAssignment(
+            new SlotRange((int)from, (int)to),
+            primary,
+            AsReadOnly(replicas));
+        return true;
+    }
+
+    private static bool TryParseNode(ref RespReader reader, out ClusterSlotNode node)
+    {
+        node = null!;
+        if (!reader.IsAggregate || reader.IsNull) return false;
+
+        var children = reader.AggregateChildren();
+
+        // element 0 is positionally "the endpoint"; its content is whichever form the answering node
+        // prefers, so it may be a hostname, an empty string, "?", or the RESP null - never assume an address
+        if (!children.MoveNext()) return false;
+        string? announced = children.Value.IsNull ? null : children.Value.ReadString();
+
+        if (!children.MoveNext() || !children.Value.TryReadInt64(out var port)) return false;
+
+        // the node id arrived in 4.0 and the metadata map in 7.0, so both are absent on older servers
+        string? nodeId = children.MoveNext() ? children.Value.ReadString() : null;
+
+        IReadOnlyList<KeyValuePair<string, string?>> metadata = [];
+        string? ip = null, hostname = null;
+        if (children.MoveNext() && children.Value.IsAggregate && !children.Value.IsNull)
+        {
+            metadata = ReadMetadata(ref children.Value, out ip, out hostname);
+        }
+
+        node = new ClusterSlotNode(announced, (int)port, nodeId, ResolveEndPoint(announced, port), ip, hostname, metadata);
+        return true;
+    }
+
+    // walked pairwise rather than by declared length, since a map reports pairs while an array (RESP2)
+    // reports elements, and we do not need to care which we were given.
+    //
+    // Keys are matched against the known set over the raw bytes, so a recognized key costs no string at all -
+    // and since ranges repeat a node whenever its slot ownership is not contiguous, that is one allocation
+    // avoided per key per range rather than per node. Only unrecognized keys are materialized, into the
+    // collection that exists to preserve them
+    private static unsafe IReadOnlyList<KeyValuePair<string, string?>> ReadMetadata(
+        ref RespReader reader,
+        out string? ip,
+        out string? hostname)
+    {
+        ip = hostname = null;
+        List<KeyValuePair<string, string?>>? metadata = null;
+        var children = reader.AggregateChildren();
+        while (children.MoveNext())
+        {
+            if (!children.Value.TryParseScalar(&ClusterSlotMetadataKeyMetadata.TryParse, out ClusterSlotMetadataKey key))
+            {
+                key = ClusterSlotMetadataKey.Unknown;
+            }
+
+            // the key is still in the reader until we move on, so an unknown one can be read then
+            var unknownKey = key == ClusterSlotMetadataKey.Unknown ? children.Value.ReadString() : null;
+
+            if (!children.MoveNext()) break; // odd count: ignore the dangling key
+            var value = children.Value.IsNull ? null : children.Value.ReadString();
+
+            switch (key)
+            {
+                case ClusterSlotMetadataKey.Ip:
+                    ip = value;
+                    break;
+                case ClusterSlotMetadataKey.Hostname:
+                    hostname = value;
+                    break;
+                default:
+                    if (!string.IsNullOrEmpty(unknownKey))
+                    {
+                        (metadata ??= new List<KeyValuePair<string, string?>>()).Add(new(unknownKey!, value));
+                    }
+                    break;
+            }
+        }
+        return AsReadOnly(metadata);
+    }
+
+    // empty collection expressions to an interface target compile to Array.Empty<T>(), so the common case
+    // costs nothing; only a genuinely populated list pays for the wrapper
+    private static IReadOnlyList<T> AsReadOnly<T>(List<T>? values)
+    {
+        if (values is null) return [];
+        return values.AsReadOnly();
+    }
+
+    private static EndPoint? ResolveEndPoint(string? announced, long port)
+    {
+        // null and "" both mean "no endpoint reported"; "?" means an explicitly unknown node. None of the
+        // three can be turned into an endpoint here - substituting the connection's own address is a
+        // decision for the caller, and is outright wrong for "?"
+        if (announced is null or "" or "?") return null;
+        if (port is < 1 or > ushort.MaxValue) return null;
+
+        return Format.TryParseEndPoint(announced, port.ToString(), out var endpoint) ? endpoint : null;
+    }
+}

src/StackExchange.Redis/ClusterSlots.Server.cs

@@ -0,0 +1,12 @@
+using System.Threading.Tasks;
+
+namespace StackExchange.Redis;
+
+internal partial class RedisServer
+{
+    public ClusterSlotsResult? ClusterSlots(CommandFlags flags = CommandFlags.None)
+        => ExecuteSync(Message.Create(-1, flags, RedisCommand.CLUSTER, RedisLiterals.SLOTS), ClusterSlotsResult.Processor);
+
+    public Task<ClusterSlotsResult?> ClusterSlotsAsync(CommandFlags flags = CommandFlags.None)
+        => ExecuteAsync(Message.Create(-1, flags, RedisCommand.CLUSTER, RedisLiterals.SLOTS), ClusterSlotsResult.Processor);
+}