Skip to content

Commit d643c53

Browse files
imperugoimperugo
authored
Merge pull request #667 from LeaFrock/issue666
Refactor server iteration to `ServerSource` (reduce allocations, improve clarity)
2 parents d6bf654 + 95ca076 commit d643c53

5 files changed

Lines changed: 197 additions & 51 deletions

File tree

‎src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,11 +505,12 @@ public Task<long> SetCombineAndStoreAsync(SetOperation operation, string destina
505505
/// <inheritdoc/>
506506
public async Task<IEnumerable<string>> SearchKeysAsync(string pattern)
507507
{
508-
pattern = $"{keyPrefix}{pattern}";
508+
pattern = keyPrefix + pattern;
509509
var keys = new HashSet<string>();
510510
var hasPrefix = !string.IsNullOrEmpty(keyPrefix);
511511

512-
foreach (var server in ServerIteratorFactory.GetServers(connectionPoolManager.GetConnection(), serverEnumerationStrategy))
512+
var serverSource = new ServerSource(connectionPoolManager.GetConnection());
513+
foreach (var server in serverSource.GetServers(serverEnumerationStrategy))
513514
{
514515
// The prefix is stripped while filling: a lazy Select would re-allocate every substring on each enumeration.
515516
await foreach (var key in server.KeysAsync(dbNumber, pattern, 1000).ConfigureAwait(false))

‎src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerEnumerable.cs‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Ugo Lattanzi. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
22

3+
using System;
34
using System.Collections;
45
using System.Collections.Generic;
56

@@ -10,6 +11,11 @@ namespace StackExchange.Redis.Extensions.Core.ServerIteration;
1011
/// <summary>
1112
/// The class that allows you to enumerate all the redis servers.
1213
/// </summary>
14+
#if NET5_0_OR_GREATER
15+
[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.", DiagnosticId = "SRE0002")]
16+
#else
17+
[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.")]
18+
#endif
1319
public class ServerEnumerable : IEnumerable<IServer>
1420
{
1521
private readonly IConnectionMultiplexer multiplexer;
@@ -36,25 +42,13 @@ public ServerEnumerable(
3642
/// Return the enumerator of the Redis servers
3743
/// </summary>
3844
public IEnumerator<IServer> GetEnumerator()
39-
{
40-
foreach (var endPoint in multiplexer.GetEndPoints())
41-
{
42-
var server = multiplexer.GetServer(endPoint);
43-
if (targetRole == ServerEnumerationStrategy.TargetRoleOptions.PreferSlave)
44-
{
45-
if (!server.IsReplica)
46-
continue;
47-
}
48-
49-
if (unreachableServerAction == ServerEnumerationStrategy.UnreachableServerActionOptions.IgnoreIfOtherAvailable)
50-
{
51-
if (!server.IsConnected || !server.Features.Scan)
52-
continue;
53-
}
54-
55-
yield return server;
56-
}
57-
}
45+
=> new ServerSource(multiplexer).GetServers(new()
46+
{
47+
Mode = ServerEnumerationStrategy.ModeOptions.All,
48+
TargetRole = targetRole,
49+
UnreachableServerAction = unreachableServerAction
50+
})
51+
.GetEnumerator();
5852

5953
IEnumerator IEnumerable.GetEnumerator()
6054
{

‎src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ namespace StackExchange.Redis.Extensions.Core.ServerIteration;
1010
/// <summary>
1111
/// The factory that allows you to enumerate all Redis servers.
1212
/// </summary>
13+
#if NET5_0_OR_GREATER
14+
[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.", DiagnosticId = "SRE0002")]
15+
#else
16+
[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.")]
17+
#endif
1318
public static class ServerIteratorFactory
1419
{
1520
/// <summary>
@@ -21,34 +26,5 @@ public static class ServerIteratorFactory
2126
public static IEnumerable<IServer> GetServers(
2227
IConnectionMultiplexer multiplexer,
2328
ServerEnumerationStrategy serverEnumerationStrategy)
24-
{
25-
switch (serverEnumerationStrategy.Mode)
26-
{
27-
case ServerEnumerationStrategy.ModeOptions.All:
28-
return new ServerEnumerable(
29-
multiplexer,
30-
serverEnumerationStrategy.TargetRole,
31-
serverEnumerationStrategy.UnreachableServerAction);
32-
33-
case ServerEnumerationStrategy.ModeOptions.Single:
34-
var serversSingle = new ServerEnumerable(
35-
multiplexer,
36-
serverEnumerationStrategy.TargetRole,
37-
serverEnumerationStrategy.UnreachableServerAction);
38-
39-
return TakeFirst(serversSingle);
40-
41-
default:
42-
throw new NotImplementedException();
43-
}
44-
}
45-
46-
private static IEnumerable<IServer> TakeFirst(ServerEnumerable servers)
47-
{
48-
foreach (var server in servers)
49-
{
50-
yield return server;
51-
yield break;
52-
}
53-
}
29+
=> new ServerSource(multiplexer).GetServers(serverEnumerationStrategy);
5430
}

‎src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerSource.cs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright (c) Ugo Lattanzi. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System.Collections.Generic;
4+
using System.Linq;
5+
using System.Net;
6+
7+
using StackExchange.Redis.Extensions.Core.Configuration;
8+
9+
using static StackExchange.Redis.Extensions.Core.Configuration.ServerEnumerationStrategy;
10+
11+
namespace StackExchange.Redis.Extensions.Core.ServerIteration;
12+
13+
/// <summary>
14+
/// Represents a source of servers from a connection multiplexer.
15+
/// </summary>
16+
/// <param name="multiplexer">The connection multiplexer to retrieve servers from.</param>
17+
/// <remarks>
18+
/// DO NOT implement <see cref="IEnumerable{IServer}"/> to avoid boxing struct enumerators and to keep allocation semantics explicit.
19+
/// </remarks>
20+
internal readonly struct ServerSource(IConnectionMultiplexer multiplexer)
21+
{
22+
private readonly EndPoint[] endPoints = multiplexer.GetEndPoints();
23+
24+
public IEnumerable<IServer> GetServers(ServerEnumerationStrategy strategy)
25+
=> strategy.Mode == ModeOptions.Single
26+
? EnumerateCore(strategy).Take(1)
27+
: EnumerateCore(strategy);
28+
29+
private IEnumerable<IServer> EnumerateCore(ServerEnumerationStrategy strategy)
30+
{
31+
foreach (var endPoint in endPoints)
32+
{
33+
var server = multiplexer.GetServer(endPoint);
34+
35+
if (strategy.TargetRole == TargetRoleOptions.PreferSlave)
36+
{
37+
if (!server.IsReplica)
38+
continue;
39+
}
40+
41+
if (strategy.UnreachableServerAction == UnreachableServerActionOptions.IgnoreIfOtherAvailable)
42+
{
43+
if (!server.IsConnected || !server.Features.Scan)
44+
continue;
45+
}
46+
47+
yield return server;
48+
}
49+
}
50+
}

‎tests/StackExchange.Redis.Extensions.Core.Tests/ServerSourceTests.cs‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// Copyright (c) Ugo Lattanzi. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using System.Net;
7+
8+
using NSubstitute;
9+
10+
using StackExchange.Redis.Extensions.Core.Configuration;
11+
using StackExchange.Redis.Extensions.Core.ServerIteration;
12+
13+
using Xunit;
14+
15+
namespace StackExchange.Redis.Extensions.Core.Tests;
16+
17+
public class ServerSourceTests
18+
{
19+
[Fact]
20+
public void GetServers_SingleMode_ReturnsOnlyFirstServer()
21+
{
22+
var (multiplexer, servers) = BuildMultiplexer(3);
23+
var strategy = new ServerEnumerationStrategy { Mode = ServerEnumerationStrategy.ModeOptions.Single };
24+
25+
var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
26+
27+
Assert.Single(result);
28+
Assert.Same(servers[0], result[0]);
29+
}
30+
31+
[Fact]
32+
public void GetServers_AllMode_ReturnsEveryServer()
33+
{
34+
var (multiplexer, servers) = BuildMultiplexer(3);
35+
var strategy = new ServerEnumerationStrategy { Mode = ServerEnumerationStrategy.ModeOptions.All };
36+
37+
var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
38+
39+
Assert.Equal(3, result.Count);
40+
Assert.Equal(servers, result);
41+
}
42+
43+
[Fact]
44+
public void GetServers_PreferSlave_SkipsPrimaries()
45+
{
46+
var (multiplexer, servers) = BuildMultiplexer(3);
47+
servers[1].IsReplica.Returns(true);
48+
49+
var strategy = new ServerEnumerationStrategy
50+
{
51+
Mode = ServerEnumerationStrategy.ModeOptions.All,
52+
TargetRole = ServerEnumerationStrategy.TargetRoleOptions.PreferSlave
53+
};
54+
55+
var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
56+
57+
Assert.Single(result);
58+
Assert.Same(servers[1], result[0]);
59+
}
60+
61+
[Fact]
62+
public void GetServers_IgnoreIfOtherAvailable_SkipsDisconnectedServers()
63+
{
64+
var (multiplexer, servers) = BuildMultiplexer(3);
65+
servers[0].IsConnected.Returns(false);
66+
67+
var strategy = new ServerEnumerationStrategy
68+
{
69+
Mode = ServerEnumerationStrategy.ModeOptions.Single,
70+
UnreachableServerAction = ServerEnumerationStrategy.UnreachableServerActionOptions.IgnoreIfOtherAvailable
71+
};
72+
73+
var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
74+
75+
Assert.Single(result);
76+
Assert.Same(servers[1], result[0]);
77+
}
78+
79+
// Guards the deferred-execution contract: GetServers must not touch the multiplexer until the
80+
// result is enumerated. An earlier revision evaluated the Single branch eagerly, which this catches.
81+
[Theory]
82+
[InlineData(ServerEnumerationStrategy.ModeOptions.Single)]
83+
[InlineData(ServerEnumerationStrategy.ModeOptions.All)]
84+
public void GetServers_IsLazy_DoesNotResolveServersUntilEnumerated(ServerEnumerationStrategy.ModeOptions mode)
85+
{
86+
var (multiplexer, _) = BuildMultiplexer(3);
87+
88+
var deferred = new ServerSource(multiplexer).GetServers(new() { Mode = mode });
89+
90+
multiplexer.DidNotReceive().GetServer(Arg.Any<EndPoint>());
91+
92+
_ = deferred.ToList();
93+
94+
multiplexer.Received().GetServer(Arg.Any<EndPoint>());
95+
}
96+
97+
private static (IConnectionMultiplexer Multiplexer, List<IServer> Servers) BuildMultiplexer(int count)
98+
{
99+
var multiplexer = Substitute.For<IConnectionMultiplexer>();
100+
var endPoints = new EndPoint[count];
101+
var servers = new List<IServer>(count);
102+
103+
for (var i = 0; i < count; i++)
104+
{
105+
var endPoint = new IPEndPoint(IPAddress.Loopback, 6379 + i);
106+
endPoints[i] = endPoint;
107+
108+
var server = Substitute.For<IServer>();
109+
server.IsReplica.Returns(false);
110+
server.IsConnected.Returns(true);
111+
112+
// SCAN landed in Redis 2.8; without a version the mock reports Features.Scan == false
113+
// and the IgnoreIfOtherAvailable filter would discard every server.
114+
server.Features.Returns(new RedisFeatures(new Version(2, 8)));
115+
116+
servers.Add(server);
117+
118+
multiplexer.GetServer(endPoint).Returns(server);
119+
}
120+
121+
multiplexer.GetEndPoints().Returns(endPoints);
122+
123+
return (multiplexer, servers);
124+
}
125+
}

0 commit comments

Comments
 (0)