Skip to content
This repository was archived by the owner on Dec 24, 2022. It is now read-only.

Commit 47233b6

Browse files
mythzmythz
committed
Don't immediately kill connections of active clients after failover to give them a chance to dispose gracefully.
1 parent a1bfa1c commit 47233b6

9 files changed

Lines changed: 134 additions & 34 deletions

‎src/ServiceStack.Redis/PooledRedisClientManager.cs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,7 @@ public void FailoverTo(IEnumerable<string> readWriteHosts, IEnumerable<string> r
160160
{
161161
var redis = readClients[i];
162162
if (redis != null)
163-
{
164-
redis.DisposeConnection();
165-
}
163+
RedisState.DeactivateClient(redis);
166164

167165
readClients[i] = null;
168166
}
@@ -175,9 +173,7 @@ public void FailoverTo(IEnumerable<string> readWriteHosts, IEnumerable<string> r
175173
{
176174
var redis = writeClients[i];
177175
if (redis != null)
178-
{
179-
redis.DisposeConnection();
180-
}
176+
RedisState.DeactivateClient(redis);
181177

182178
writeClients[i] = null;
183179
}
@@ -233,6 +229,8 @@ public IRedisClient GetClient()
233229

234230
InitClient(inActiveClient);
235231

232+
RedisState.DisposeExpiredClients();
233+
236234
return inActiveClient;
237235
}
238236
}
@@ -250,7 +248,6 @@ private RedisClient GetInActiveWriteClient()
250248
for (int x = 0; x < readWriteTotal; x++)
251249
{
252250
var nextHostIndex = (desiredIndex + x) % readWriteTotal;
253-
RedisEndpoint nextHost = RedisResolver.GetReadWriteHost(nextHostIndex);
254251
for (var i = nextHostIndex; i < writeClients.Length; i += readWriteTotal)
255252
{
256253
if (writeClients[i] != null && !writeClients[i].Active && !writeClients[i].HadExceptions)
@@ -259,9 +256,10 @@ private RedisClient GetInActiveWriteClient()
259256
if (writeClients[i] == null || writeClients[i].HadExceptions)
260257
{
261258
if (writeClients[i] != null)
262-
writeClients[i].DisposeConnection();
259+
RedisState.DeactivateClient(writeClients[i]);
263260

264-
var client = InitNewClient(nextHost, readWrite:true);
261+
var nextHost = RedisResolver.GetReadWriteHost(nextHostIndex);
262+
var client = InitNewClient(nextHost, readWrite: true);
265263
writeClients[i] = client;
266264

267265
return client;
@@ -330,6 +328,8 @@ public virtual IRedisClient GetReadOnlyClient()
330328

331329
InitClient(inActiveClient);
332330

331+
RedisState.DisposeExpiredClients();
332+
333333
return inActiveClient;
334334
}
335335
}
@@ -347,7 +347,6 @@ private RedisClient GetInActiveReadClient()
347347
for (int x = 0; x < readOnlyTotal; x++)
348348
{
349349
var nextHostIndex = (desiredIndex + x) % readOnlyTotal;
350-
var nextHost = RedisResolver.GetReadOnlyHost(nextHostIndex);
351350
for (var i = nextHostIndex; i < readClients.Length; i += readOnlyTotal)
352351
{
353352
if (readClients[i] != null && !readClients[i].Active && !readClients[i].HadExceptions)
@@ -356,9 +355,10 @@ private RedisClient GetInActiveReadClient()
356355
if (readClients[i] == null || readClients[i].HadExceptions)
357356
{
358357
if (readClients[i] != null)
359-
readClients[i].DisposeConnection();
358+
RedisState.DeactivateClient(readClients[i]);
360359

361-
var client = InitNewClient(nextHost, readWrite:false);
360+
var nextHost = RedisResolver.GetReadOnlyHost(nextHostIndex);
361+
var client = InitNewClient(nextHost, readWrite: false);
362362
readClients[i] = client;
363363

364364
return client;
@@ -597,6 +597,8 @@ protected virtual void Dispose(bool disposing)
597597
{
598598
Log.Error("Error when trying to dispose of PooledRedisClientManager", ex);
599599
}
600+
601+
RedisState.DisposeAllDeactivatedClients();
600602
}
601603

602604
private int disposeAttempts = 0;

‎src/ServiceStack.Redis/RedisConfig.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,7 @@ public class RedisConfig
2222

2323
//Skip ServerVersion Checks by specifying Min Version number, e.g: 2.8.12 => 2812, 2.9.1 => 2910
2424
public static int? AssumeServerVersion;
25+
26+
public static TimeSpan DeactivatedClientsExpiry = TimeSpan.FromMinutes(1);
2527
}
2628
}

‎src/ServiceStack.Redis/RedisManagerPool.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,7 @@ public void FailoverTo(params string[] readWriteHosts)
8080
{
8181
var redis = clients[i];
8282
if (redis != null)
83-
{
84-
redis.DisposeConnection();
85-
}
83+
RedisState.DeactivateClient(redis);
8684

8785
clients[i] = null;
8886
}
@@ -135,6 +133,8 @@ public IRedisClient GetClient()
135133
poolIndex++;
136134
inActiveClient.Active = true;
137135

136+
RedisState.DisposeExpiredClients();
137+
138138
return inActiveClient;
139139
}
140140
}
@@ -157,7 +157,6 @@ private RedisClient GetInActiveClient()
157157
for (int x = 0; x < readWriteTotal; x++)
158158
{
159159
var nextHostIndex = (desiredIndex + x) % readWriteTotal;
160-
RedisEndpoint nextHost = RedisResolver.GetReadWriteHost(nextHostIndex);
161160
for (var i = nextHostIndex; i < clients.Length; i += readWriteTotal)
162161
{
163162
if (clients[i] != null && !clients[i].Active && !clients[i].HadExceptions)
@@ -166,9 +165,10 @@ private RedisClient GetInActiveClient()
166165
if (clients[i] == null || clients[i].HadExceptions)
167166
{
168167
if (clients[i] != null)
169-
clients[i].DisposeConnection();
168+
RedisState.DeactivateClient(clients[i]);
170169

171-
var client = InitNewClient(nextHost, readWrite:true);
170+
var nextHost = RedisResolver.GetReadWriteHost(nextHostIndex);
171+
var client = InitNewClient(nextHost, readWrite: true);
172172
clients[i] = client;
173173

174174
return client;
@@ -316,6 +316,8 @@ protected virtual void Dispose(bool disposing)
316316
{
317317
Log.Error("Error when trying to dispose of PooledRedisClientManager", ex);
318318
}
319+
320+
RedisState.DisposeAllDeactivatedClients();
319321
}
320322

321323
private int disposeAttempts = 0;

‎src/ServiceStack.Redis/RedisNativeClient.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
using System.Linq;
1818
using System.Net.Security;
1919
using System.Net.Sockets;
20-
using System.Text;
2120
using System.Threading;
2221
using ServiceStack.Logging;
2322
using ServiceStack.Redis.Pipeline;
@@ -50,7 +49,9 @@ public partial class RedisNativeClient
5049
private int clientPort;
5150
private string lastCommand;
5251
private SocketException lastSocketException;
53-
public bool HadExceptions { get; protected set; }
52+
53+
internal DateTime? DeactivatedAt;
54+
public bool HadExceptions { get { return DeactivatedAt != null; } }
5455

5556
protected Socket socket;
5657
protected BufferedStream Bstream;
@@ -117,7 +118,6 @@ internal IRedisTransactionBase Transaction
117118
}
118119
}
119120

120-
121121
internal IRedisPipelineShared Pipeline
122122
{
123123
get

‎src/ServiceStack.Redis/RedisNativeClient_Utils.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private void Connect()
8989
{
9090
socket.Close();
9191
socket = null;
92-
HadExceptions = true;
92+
DeactivatedAt = DateTime.UtcNow;
9393
return;
9494
}
9595

@@ -183,7 +183,7 @@ private void Connect()
183183
socket.Close();
184184
socket = null;
185185

186-
HadExceptions = true;
186+
DeactivatedAt = DateTime.UtcNow;
187187
var throwEx = new RedisException("could not connect to redis Instance at " + Host + ":" + Port, ex);
188188
log.Error(throwEx.Message, ex);
189189
throw throwEx;
@@ -255,7 +255,7 @@ private bool Reconnect()
255255

256256
private bool HandleSocketException(SocketException ex)
257257
{
258-
HadExceptions = true;
258+
DeactivatedAt = DateTime.UtcNow;
259259
log.Error("SocketException: ", ex);
260260

261261
lastSocketException = ex;
@@ -269,7 +269,7 @@ private bool HandleSocketException(SocketException ex)
269269

270270
private RedisResponseException CreateResponseError(string error)
271271
{
272-
HadExceptions = true;
272+
DeactivatedAt = DateTime.UtcNow;
273273
string safeLastCommand = string.IsNullOrEmpty(Password) ? lastCommand : (lastCommand ?? "").Replace(Password, "");
274274

275275
var throwEx = new RedisResponseException(
@@ -281,7 +281,7 @@ private RedisResponseException CreateResponseError(string error)
281281

282282
private RedisException CreateConnectionError()
283283
{
284-
HadExceptions = true;
284+
DeactivatedAt = DateTime.UtcNow;
285285
var throwEx = new RedisException(
286286
string.Format("Unable to Connect: sPort: {0}",
287287
clientPort), lastSocketException);
@@ -456,7 +456,7 @@ private int SafeReadByte()
456456
}
457457
catch (Exception)
458458
{
459-
HadExceptions = true;
459+
DeactivatedAt = DateTime.UtcNow;
460460
throw;
461461
}
462462
}

‎src/ServiceStack.Redis/RedisState.cs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
using System;
2+
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
4+
using ServiceStack.Logging;
5+
6+
namespace ServiceStack.Redis
7+
{
8+
/// <summary>
9+
/// Don't immediately kill connections of active clients after failover to give them a chance to dispose gracefully.
10+
/// Deactivating clients are automatically cleared from the pool.
11+
/// </summary>
12+
internal static class RedisState
13+
{
14+
private static ILog log = LogManager.GetLogger(typeof(RedisState));
15+
16+
static readonly ConcurrentDictionary<RedisClient, DateTime> DeactivatedClients = new ConcurrentDictionary<RedisClient, DateTime>();
17+
18+
internal static void DeactivateClient(RedisClient client)
19+
{
20+
if (RedisConfig.DeactivatedClientsExpiry == TimeSpan.Zero)
21+
{
22+
client.DisposeConnection();
23+
return;
24+
}
25+
26+
var deactivatedAt = client.DeactivatedAt ?? DateTime.UtcNow;
27+
client.DeactivatedAt = deactivatedAt;
28+
29+
if (!DeactivatedClients.TryAdd(client, deactivatedAt))
30+
client.DisposeConnection();
31+
}
32+
33+
internal static void DisposeExpiredClients()
34+
{
35+
if (RedisConfig.DeactivatedClientsExpiry == TimeSpan.Zero || DeactivatedClients.Count == 0)
36+
return;
37+
38+
var now = DateTime.UtcNow;
39+
var removeDisposed = new List<RedisClient>();
40+
41+
foreach (var entry in DeactivatedClients)
42+
{
43+
try
44+
{
45+
if (now - entry.Value <= RedisConfig.DeactivatedClientsExpiry)
46+
continue;
47+
48+
if (log.IsDebugEnabled)
49+
log.Debug("Disposed Deactivated Client: {0}".Fmt(entry.Key.GetHostString()));
50+
51+
entry.Key.DisposeConnection();
52+
removeDisposed.Add(entry.Key);
53+
}
54+
catch
55+
{
56+
removeDisposed.Add(entry.Key);
57+
}
58+
}
59+
60+
if (removeDisposed.Count == 0)
61+
return;
62+
63+
var dict = ((IDictionary<RedisClient, DateTime>)DeactivatedClients);
64+
foreach (var client in removeDisposed)
65+
{
66+
dict.Remove(client);
67+
}
68+
}
69+
70+
internal static void DisposeAllDeactivatedClients()
71+
{
72+
if (RedisConfig.DeactivatedClientsExpiry == TimeSpan.Zero)
73+
return;
74+
75+
var allClients = DeactivatedClients.Keys.ToArray();
76+
DeactivatedClients.Clear();
77+
foreach (var client in allClients)
78+
{
79+
if (log.IsDebugEnabled)
80+
log.Debug("Disposed Deactivated Client (All): {0}".Fmt(client.GetHostString()));
81+
82+
client.DisposeConnection();
83+
}
84+
}
85+
}
86+
}

‎src/ServiceStack.Redis/ServiceStack.Redis.Signed.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@
183183
<Compile Include="RedisSentinel.cs" />
184184
<Compile Include="RedisSentinelResolver.cs" />
185185
<Compile Include="RedisSentinelWorker.cs" />
186+
<Compile Include="RedisState.cs" />
186187
<Compile Include="ScanResult.cs" />
187188
<Compile Include="ShardedConnectionPool.cs" />
188189
<Compile Include="ShardedRedisClientManager.cs" />

‎src/ServiceStack.Redis/ServiceStack.Redis.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
<Compile Include="RedisSentinel.cs" />
178178
<Compile Include="RedisSentinelResolver.cs" />
179179
<Compile Include="RedisSentinelWorker.cs" />
180+
<Compile Include="RedisState.cs" />
180181
<Compile Include="ScanResult.cs" />
181182
<Compile Include="ShardedConnectionPool.cs" />
182183
<Compile Include="ShardedRedisClientManager.cs" />

‎tests/ServiceStack.Redis.Tests/RedisManagerPoolTests.cs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
using System.Collections.Generic;
33
using System.Diagnostics;
44
using System.Threading;
5-
using Moq;
65
using NUnit.Framework;
76
using ServiceStack.Text;
87

@@ -22,21 +21,28 @@ public class RedisManagerPoolTests
2221
private string firstReadWriteHost;
2322
private string firstReadOnlyHost;
2423

24+
[TestFixtureSetUp]
25+
public void TestFixtureSetUp()
26+
{
27+
RedisConfig.VerifyMasterConnections = false;
28+
}
29+
30+
[TestFixtureTearDown]
31+
public void TestFixtureTearDown()
32+
{
33+
RedisConfig.VerifyMasterConnections = true;
34+
}
35+
2536
[SetUp]
2637
public void OnBeforeEachTest()
2738
{
2839
firstReadWriteHost = hosts[0];
2940
firstReadOnlyHost = testReadOnlyHosts[0];
3041
}
3142

32-
public RedisManagerPool CreateManager(params string[] hosts)
33-
{
34-
return CreateManager(hosts);
35-
}
36-
3743
public RedisManagerPool CreateManager()
3844
{
39-
return CreateManager(hosts);
45+
return new RedisManagerPool(hosts);
4046
}
4147

4248
[Test]

0 commit comments

Comments
 (0)