Skip to content

Commit a7e4fa6

Browse files
committed
- move the "ignore handshake nodes" logic deeper into the client, so external callers can still fetch the value
- add new IgnoreFromClient internal member for ^^^, and use - support "handshake" etc in the in-proc test server - support "only initially connect to default node" in the in-proc test server - add a unit test that handshake nodes are ignored
1 parent df0047f commit a7e4fa6

6 files changed

Lines changed: 79 additions & 17 deletions

File tree

src/StackExchange.Redis/ClusterConfiguration.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,6 @@ internal ClusterConfiguration(ServerSelectionStrategy serverSelectionStrategy, s
181181
if (node.IsNoAddr || node.IsFail || node.EndPoint == null)
182182
continue;
183183

184-
// Be resilient to "handshake" nodes, which are nodes that are in the process of joining the cluster and hence might not have all information available yet.
185-
// These nodes will be included in the configuration once they finish the handshake process and are fully part of the cluster, so we can safely ignore them for now.
186-
if (node.IsHandshake)
187-
continue;
188-
189184
// Override the origin value with the endpoint advertised with the target node to
190185
// make sure that things like clusterConfiguration[clusterConfiguration.Origin]
191186
// will work as expected.
@@ -431,6 +426,10 @@ public IList<ClusterNode> Children
431426
/// </summary>
432427
public IList<SlotRange> Slots { get; }
433428

429+
// Be resilient to "handshake" nodes, which are nodes that are in the process of joining the cluster and hence might not have all information available yet.
430+
// These nodes will be included in the configuration once they finish the handshake process and are fully part of the cluster, so we can safely ignore them for now.
431+
internal bool IgnoreFromClient => IsHandshake; // possibly also noaddr?
432+
434433
/// <summary>
435434
/// Compares the current instance with another object of the same type and returns an integer that indicates
436435
/// whether the current instance precedes, follows, or occurs in the same position in the sort order as the other object.

src/StackExchange.Redis/ConnectionMultiplexer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,7 +1760,7 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) =>
17601760
{
17611761
return null;
17621762
}
1763-
var clusterEndpoints = new EndPointCollection(clusterConfig.Nodes.Where(node => node.EndPoint is not null).Select(node => node.EndPoint!).ToList());
1763+
var clusterEndpoints = new EndPointCollection(clusterConfig.Nodes.Where(node => node.EndPoint is not null && !node.IgnoreFromClient).Select(node => node.EndPoint!).ToList());
17641764
// Loop through nodes in the cluster and update nodes relations to other nodes
17651765
ServerEndPoint? serverEndpoint = null;
17661766
foreach (EndPoint endpoint in clusterEndpoints)
@@ -1933,7 +1933,7 @@ internal void UpdateClusterRange(ClusterConfiguration configuration)
19331933
}
19341934
foreach (var node in configuration.Nodes)
19351935
{
1936-
if (node.IsReplica || node.Slots.Count == 0) continue;
1936+
if (node.IgnoreFromClient || node.IsReplica || node.Slots.Count == 0) continue;
19371937
foreach (var slot in node.Slots)
19381938
{
19391939
if (GetServerEndPoint(node.EndPoint) is ServerEndPoint server)

src/StackExchange.Redis/ServerEndPoint.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,8 @@ public void UpdateNodeRelations(ClusterConfiguration configuration)
324324
ServerEndPoint? primary = null;
325325
foreach (var node in configuration.Nodes)
326326
{
327+
if (node.IgnoreFromClient) continue;
328+
327329
if (node.NodeId == thisNode.ParentNodeId)
328330
{
329331
primary = Multiplexer.GetServerEndPoint(node.EndPoint);
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using System.Threading.Tasks;
2+
using Xunit;
3+
using Xunit.Sdk;
4+
5+
namespace StackExchange.Redis.Tests;
6+
7+
// context: https://github.com/StackExchange/StackExchange.Redis/pull/3043
8+
public class ClusterHandshakeNodesUnitTests(ITestOutputHelper log)
9+
{
10+
[Fact]
11+
public async Task ClusterHandshakeNodesAreIgnored()
12+
{
13+
using var server = new InProcessTestServer() { ServerType = ServerType.Cluster };
14+
var a = server.DefaultEndPoint;
15+
var b = server.AddEmptyNode();
16+
var c = server.AddEmptyNode(Server.RedisServer.NodeFlags.Handshake);
17+
using var conn = await server.ConnectAsync(defaultOnly: true); // defaultOnly: only connect to a initially
18+
19+
log.WriteLine($"a: {Format.ToString(a)}, b: {Format.ToString(b)}, c: {Format.ToString(c)}");
20+
var ep = conn.GetEndPoints();
21+
log.WriteLine("Endpoints:");
22+
foreach (var e in ep)
23+
{
24+
log.WriteLine(Format.ToString(e));
25+
}
26+
Assert.Equal(2, ep.Length);
27+
Assert.Contains(a, ep);
28+
Assert.Contains(b, ep);
29+
Assert.DoesNotContain(c, ep);
30+
}
31+
}

tests/StackExchange.Redis.Tests/InProcessTestServer.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ public InProcessTestServer(ITestOutputHelper? log = null, EndPoint? endpoint = n
2727
Tunnel = new InProcTunnel(this);
2828
}
2929

30-
public Task<ConnectionMultiplexer> ConnectAsync(bool withPubSub = true /*, WriteMode writeMode = WriteMode.Default */, TextWriter? log = null)
31-
=> ConnectionMultiplexer.ConnectAsync(GetClientConfig(withPubSub /*, writeMode */), log);
30+
public Task<ConnectionMultiplexer> ConnectAsync(bool withPubSub = true, bool defaultOnly = false /*, WriteMode writeMode = WriteMode.Default */, TextWriter? log = null)
31+
=> ConnectionMultiplexer.ConnectAsync(GetClientConfig(withPubSub, defaultOnly /*, writeMode */), log);
3232

3333
// view request/response highlights in the log
3434
public override TypedRedisValue Execute(RedisClient client, in RedisRequest request)
@@ -60,7 +60,7 @@ public override TypedRedisValue Execute(RedisClient client, in RedisRequest requ
6060
return result;
6161
}
6262

63-
public ConfigurationOptions GetClientConfig(bool withPubSub = true /*, WriteMode writeMode = WriteMode.Default */)
63+
public ConfigurationOptions GetClientConfig(bool withPubSub = true, bool defaultOnly = false /*, WriteMode writeMode = WriteMode.Default */)
6464
{
6565
var commands = GetCommands();
6666
if (!withPubSub)
@@ -106,9 +106,16 @@ public ConfigurationOptions GetClientConfig(bool withPubSub = true /*, WriteMode
106106
#endif
107107
*/
108108

109-
foreach (var endpoint in GetEndPoints())
109+
if (defaultOnly)
110110
{
111-
config.EndPoints.Add(endpoint);
111+
config.EndPoints.Add(DefaultEndPoint);
112+
}
113+
else
114+
{
115+
foreach (var endpoint in GetEndPoints())
116+
{
117+
config.EndPoints.Add(endpoint);
118+
}
112119
}
113120
return config;
114121
}

toys/StackExchange.Redis.Server/RedisServer.cs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,19 @@ public bool Migrate(int hashSlot, EndPoint to)
7878
public bool Migrate(Span<byte> key, EndPoint to) => Migrate(ServerSelectionStrategy.GetClusterSlot(key), to);
7979
public bool Migrate(in RedisKey key, EndPoint to) => Migrate(GetHashSlot(key), to);
8080

81-
public EndPoint AddEmptyNode()
81+
[Flags]
82+
public enum NodeFlags
83+
{
84+
None = 0, // note: implicitly primary, since no replica flag
85+
Replica = 1 << 0,
86+
Handshake = 1 << 1,
87+
Fail = 1 << 2,
88+
PFail = 1 << 3,
89+
NoAddress = 1 << 4,
90+
NoFailover = 1 << 5,
91+
}
92+
93+
public EndPoint AddEmptyNode(NodeFlags flags = NodeFlags.None)
8294
{
8395
EndPoint endpoint;
8496
Node node;
@@ -113,7 +125,7 @@ public EndPoint AddEmptyNode()
113125
break;
114126
}
115127

116-
node = new(this, endpoint);
128+
node = new(this, endpoint, flags);
117129
node.UpdateSlots([]); // explicit empty range (rather than implicit "all nodes")
118130
}
119131
// defensive loop for concurrency
@@ -124,7 +136,7 @@ public EndPoint AddEmptyNode()
124136
protected RedisServer(EndPoint endpoint = null, int databases = 16, TextWriter output = null) : base(output)
125137
{
126138
endpoint ??= new IPEndPoint(IPAddress.Loopback, 6379);
127-
_nodes.TryAdd(endpoint, new Node(this, endpoint));
139+
_nodes.TryAdd(endpoint, new Node(this, endpoint, NodeFlags.None));
128140
RedisVersion = s_DefaultServerVersion;
129141
if (databases < 1) throw new ArgumentOutOfRangeException(nameof(databases));
130142
Databases = databases;
@@ -487,7 +499,16 @@ protected virtual TypedRedisValue ClusterNodes(RedisClient client, in RedisReque
487499
{
488500
sb.Append("myself,");
489501
}
490-
sb.Append("master - 0 0 1 connected");
502+
sb.Append((node.Flags & NodeFlags.Replica) == 0 ? "master" : "slave");
503+
if ((node.Flags & NodeFlags.Handshake) != 0)
504+
{
505+
sb.Append(",handshake");
506+
}
507+
if ((node.Flags & NodeFlags.Fail) != 0) sb.Append(",fail");
508+
if ((node.Flags & NodeFlags.PFail) != 0) sb.Append(",fail?");
509+
if ((node.Flags & NodeFlags.NoAddress) != 0) sb.Append(",noaddr");
510+
if ((node.Flags & NodeFlags.NoFailover) != 0) sb.Append(",nofailover");
511+
sb.Append(" - 0 0 1 connected");
491512
foreach (var range in node.Slots)
492513
{
493514
sb.Append(" ").Append(range.ToString());
@@ -603,11 +624,13 @@ public override string ToString()
603624

604625
private readonly RedisServer _server;
605626
public RedisServer Server => _server;
606-
public Node(RedisServer server, EndPoint endpoint)
627+
public NodeFlags Flags { get; }
628+
public Node(RedisServer server, EndPoint endpoint, NodeFlags flags)
607629
{
608630
Host = GetHost(endpoint, out var port);
609631
Port = port;
610632
_server = server;
633+
Flags = flags;
611634
}
612635

613636
public void UpdateSlots(SlotRange[] slots) => _slots = slots;

0 commit comments

Comments
 (0)