Skip to content

Commit e43dd9b

Browse files
authored
Resolve critical failure in high-integrity-mode with -MOVED response (#3049)
* failing test for #3048 * Critical fix for high-integrity-mode re -MOVED * release notes
1 parent 9281ca6 commit e43dd9b

File tree

3 files changed

+65
-6
lines changed

3 files changed

+65
-6
lines changed

docs/ReleaseNotes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Current package versions:
88

99
## Unreleased
1010

11-
- none
11+
- Critical fix for high-integrity mode in cluster scenarios ([#3049 by @mgravell](https://github.com/StackExchange/StackExchange.Redis/issues/3049))
1212

1313
## 2.12.8
1414

src/StackExchange.Redis/PhysicalConnection.cs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,19 +1914,25 @@ private void MatchResult(in RawResult result)
19141914

19151915
Trace("Response to: " + msg);
19161916
_readStatus = ReadStatus.ComputeResult;
1917-
if (msg.ComputeResult(this, result))
1917+
1918+
// need to capture HIT promptly, as -MOVED could cause a resend with a new high-integrity token
1919+
// (a lazy approach would be to not rotate, but: we'd rather avoid that; the -MOVED case is rare)
1920+
var highIntegrityToken = msg.HighIntegrityToken;
1921+
1922+
bool computed = msg.ComputeResult(this, result);
1923+
if (computed)
19181924
{
19191925
_readStatus = msg.ResultBoxIsAsync ? ReadStatus.CompletePendingMessageAsync : ReadStatus.CompletePendingMessageSync;
1920-
if (!msg.IsHighIntegrity)
1926+
if (highIntegrityToken == 0)
19211927
{
19221928
// can't complete yet if needs checksum
19231929
msg.Complete();
19241930
}
19251931
}
1926-
if (msg.IsHighIntegrity)
1932+
if (highIntegrityToken != 0)
19271933
{
1928-
// stash this for the next non-OOB response
1929-
Volatile.Write(ref _awaitingToken, msg);
1934+
// stash this for the next non-OOB response, retaining the old HIT iff we had a -MOVED etc
1935+
Volatile.Write(ref _awaitingToken, computed ? msg : new DummyHighIntegrityMessage(msg, highIntegrityToken));
19301936
}
19311937

19321938
_readStatus = ReadStatus.MatchResultComplete;
@@ -2455,4 +2461,17 @@ internal bool HasPendingCallerFacingItems()
24552461
}
24562462
}
24572463
}
2464+
2465+
internal sealed class DummyHighIntegrityMessage : Message
2466+
{
2467+
// note: we don't create this message very often - only when a HIT gets a -MOVED or similar
2468+
public DummyHighIntegrityMessage(Message msg, uint highIntegrityToken) : base(msg.Db, msg.Flags, msg.Command)
2469+
{
2470+
WithHighIntegrity(highIntegrityToken);
2471+
}
2472+
2473+
public override int ArgCount => 0;
2474+
protected override void WriteImpl(PhysicalConnection physical)
2475+
=> throw new NotSupportedException("This message cannot be written; it is a place-holder for high-integrity scenarios.");
2476+
}
24582477
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
using System.Threading.Tasks;
2+
using Xunit;
3+
4+
namespace StackExchange.Redis.Tests;
5+
6+
public class HighIntegrityMovedUnitTests(ITestOutputHelper log)
7+
{
8+
[Theory]
9+
[InlineData(false)]
10+
[InlineData(true)]
11+
public async Task HighIntegritySurvivesMovedResponse(bool highIntegrity)
12+
{
13+
using var server = new InProcessTestServer(log) { ServerType = ServerType.Cluster };
14+
var secondary = server.AddEmptyNode();
15+
16+
var config = server.GetClientConfig();
17+
config.HighIntegrity = highIntegrity;
18+
using var client = await ConnectionMultiplexer.ConnectAsync(config);
19+
20+
RedisKey a = "a", b = "b"; // known to be in different slots
21+
Assert.NotEqual(ServerSelectionStrategy.GetClusterSlot(a), ServerSelectionStrategy.GetClusterSlot(b));
22+
23+
var db = client.GetDatabase();
24+
var x = db.StringIncrementAsync(a);
25+
var y = db.StringIncrementAsync(b);
26+
await x;
27+
await y;
28+
Assert.Equal(1, await db.StringGetAsync(a));
29+
Assert.Equal(1, await db.StringGetAsync(b));
30+
31+
// now force a -MOVED response
32+
server.Migrate(a, secondary);
33+
x = db.StringIncrementAsync(a);
34+
y = db.StringIncrementAsync(b);
35+
await x;
36+
await y;
37+
Assert.Equal(2, await db.StringGetAsync(a));
38+
Assert.Equal(2, await db.StringGetAsync(b));
39+
}
40+
}

0 commit comments

Comments
 (0)