From ce418774dead689f6fb965f2cdbb657bd9c05218 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 30 Oct 2026 16:45:52 +0000 Subject: [PATCH 1/3] Parenthesize a Debug-only assertion that fails SA1407 (same as #3314 on main) --- src/RESPite/Buffers/CycleBuffer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RESPite/Buffers/CycleBuffer.cs b/src/RESPite/Buffers/CycleBuffer.cs index a56c0a4b2..695b2e922 100644 --- a/src/RESPite/Buffers/CycleBuffer.cs +++ b/src/RESPite/Buffers/CycleBuffer.cs @@ -260,7 +260,7 @@ private void DebugAssertValid() if (startSegment is null) { Debug.Assert( - endSegmentLength == 0 & endSegmentCommitted == 0, + (endSegmentLength == 0) & (endSegmentCommitted == 0), "un-init state should be zero"); return; } From 8a1a2e6390ca4a031577c06b4bec9669572d9f0d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 30 Oct 2026 16:45:52 +0000 Subject: [PATCH 2/3] Client cache: replace a TimeToLive-expired entry on the next fill The fill path replaced a resident entry only when it was a refresh or the entry had been invalidated. IsValid tracks invalidation generations only, so an entry that had aged past TimeToLive still read as valid: TryAdd failed against it, the fill was written off as redundant, and the key could not be re-cached until the sweep (10s by default) removed the corpse. A steadily read key therefore missed on every read from expiry until the sweep; with a 5s TimeToLive that is half of every ten seconds at a 0% hit rate. An expired entry is now treated as dead in the same way as an invalidated one. Regression test runs with sweeping disabled so only the fill path can make it pass. Fixes #3318 --- .../Caching/RespClientCache.cs | 8 ++++- .../RespCacheLifetimeTests.cs | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/StackExchange.Redis/Caching/RespClientCache.cs b/src/StackExchange.Redis/Caching/RespClientCache.cs index 38f1642c3..cff8aeb3f 100644 --- a/src/StackExchange.Redis/Caching/RespClientCache.cs +++ b/src/StackExchange.Redis/Caching/RespClientCache.cs @@ -1144,8 +1144,14 @@ private bool TryCompleteCore(in RespFill fill, RespPayload response) // cache never recovers: it decays to a 0% hit rate and then costs pure overhead. Found by the // RespFest cache league, where this cache served 0.0% of reads under churn against 58-82% for // every other client, and the cached build was slower than the same build with caching off. + // + // An EXPIRED resident entry is dead in the same way, and was missed above: IsValid tracks + // invalidation generations only, so an entry that aged past TimeToLive still reads as valid, + // the fill was written off against it, and a steadily-read key missed on every read from the + // moment it expired until the sweep took the corpse away (#3318, found by the caching demo: + // with a 5s TimeToLive and the default 10s sweep, half of every ten seconds at a 0% hit rate). if (_entries.TryGetValue(entryKey, out var previous) - && (fill.Replaces || !previous.IsValid) + && (fill.Replaces || !previous.IsValid || CachePolicy.IsOlderThan(previous.FilledAt, Policy.TimeToLiveTicks)) && _entries.TryUpdate(entryKey, entry, previous)) { Interlocked.Add(ref _bytes, entry.Bytes - previous.Bytes); diff --git a/tests/StackExchange.Redis.Tests/RespCacheLifetimeTests.cs b/tests/StackExchange.Redis.Tests/RespCacheLifetimeTests.cs index 9e79ff25c..5735d0949 100644 --- a/tests/StackExchange.Redis.Tests/RespCacheLifetimeTests.cs +++ b/tests/StackExchange.Redis.Tests/RespCacheLifetimeTests.cs @@ -56,6 +56,38 @@ public async Task AnExpiredEntryIsNotServed() Assert.True(cache.Expired > 0); } + [Fact] + public async Task AnExpiredEntryIsReplacedByTheNextFill() + { + // an expired entry is DEAD for every purpose except the dictionary slot it still occupies; the fill + // that follows the miss has to take that slot over, not be written off as redundant against it. + // Otherwise the key cannot be re-cached until the sweep (ten seconds by default) removes the corpse, + // and a steadily-read key misses on every read for the whole of that gap (#3318). Sweeping is + // turned off here so that nothing but the fill path can make this pass. + using var cache = new RespClientCache(new CacheOptions + { + Admission = CacheAdmission.OnFirstMiss, + SweepInterval = TimeSpan.Zero, + DefaultPolicy = new CachePolicy { TimeToLive = TimeSpan.FromMilliseconds(100) }, + }); + var executor = new FakeExecutor("$1\r\na\r\n", "$1\r\nb\r\n", "$1\r\nc\r\n"); + var context = new RespDatabaseContext(new RespContext().WithExecutor(executor).WithCache(cache)); + + Assert.Equal("a", await Get(context)); + Assert.Equal(1, cache.Stored); + + await Task.Delay(200); + + Assert.Equal("b", await Get(context)); // expired; fetched again... + Assert.Equal(2, executor.Sends); + Assert.True(cache.Expired > 0); + Assert.Equal(2, cache.Stored); // ...and the fresh reply took over the slot + Assert.Equal(1, cache.Count); + + Assert.Equal("b", await Get(context)); // so the next read is a hit again + Assert.Equal(2, executor.Sends); + } + [Fact] public async Task AContextCanDemandSomethingFresherThanThePolicy() { From 150889b8d84c95a90a5dc4c286d4a146d01ff556 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 30 Oct 2026 16:51:35 +0000 Subject: [PATCH 3/3] v4 Debug build: remove the string-to-RedisValue conversions the Debug-only self-check flags All writes, so .AsRedisValue() where the value is computed (AUTH, HELLO, CLIENT SETINFO/SETNAME, EVALSHA hash, CONFIG GET pattern) and RedisLiterals where it is a literal (HOTKEYS START arguments; CPU, DURATION, METRICS, NET, SAMPLE and START added to the table). CoreBench gets the same NoWarn as its baseline. Release builds were unaffected; local Debug builds of v4 have been red on this since early October. --- .../Contexts/RespConnectionManager.cs | 2 +- src/StackExchange.Redis/Contexts/RespHandshake.cs | 10 +++++----- .../Database/RedisDatabase.Scripts.cs | 2 +- src/StackExchange.Redis/HotKeys.Server.cs | 14 +++++++------- src/StackExchange.Redis/RedisLiterals.cs | 6 ++++++ src/StackExchange.Redis/RedisServer.cs | 2 +- toys/CoreBench/CoreBench.csproj | 4 +++- 7 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/StackExchange.Redis/Contexts/RespConnectionManager.cs b/src/StackExchange.Redis/Contexts/RespConnectionManager.cs index ffd170630..e1dcf4983 100644 --- a/src/StackExchange.Redis/Contexts/RespConnectionManager.cs +++ b/src/StackExchange.Redis/Contexts/RespConnectionManager.cs @@ -2181,7 +2181,7 @@ void Announce(EndPoint endpoint, RespEndpointExecutor executor) var context = new RespContext(_multiplexer.CommandMap, database: -1) .WithExecutor(executor.WithDatabase(-1)); _ = context.SendAsync( - $"{RedisCommand.CLIENT}{RedisLiterals.SETINFO}{RedisLiterals.lib_name}{(RedisValue)libraryName}", + $"{RedisCommand.CLIENT}{RedisLiterals.SETINFO}{RedisLiterals.lib_name}{libraryName.AsRedisValue()}", CommandFlags.FireAndForget); } catch diff --git a/src/StackExchange.Redis/Contexts/RespHandshake.cs b/src/StackExchange.Redis/Contexts/RespHandshake.cs index 5b287e083..566c772f4 100644 --- a/src/StackExchange.Redis/Contexts/RespHandshake.cs +++ b/src/StackExchange.Redis/Contexts/RespHandshake.cs @@ -202,12 +202,12 @@ internal static async Task PerformAsync( if (user is { Length: > 0 }) { log?.LogInformationAuthenticatingUserPassword(new(server!)); - await context.SendAsync($"{RedisCommand.AUTH}{(RedisValue)user}{(RedisValue)password}").ConfigureAwait(false); + await context.SendAsync($"{RedisCommand.AUTH}{user.AsRedisValue()}{password.AsRedisValue()}").ConfigureAwait(false); } else { log?.LogInformationAuthenticatingPassword(new(server!)); - await context.SendAsync($"{RedisCommand.AUTH}{(RedisValue)password}").ConfigureAwait(false); + await context.SendAsync($"{RedisCommand.AUTH}{password.AsRedisValue()}").ConfigureAwait(false); } LogResponse(log, server, connectionType, "AUTH", "OK"); @@ -290,7 +290,7 @@ internal static async Task PerformAsync( if (helloCarriesCredentials) log?.LogInformationAuthenticatingViaHello(new(server!)); var hello = helloCarriesCredentials ? await context.SendAsync( - $"{RedisCommand.HELLO}{protover}{RespLiterals.Auth}{(RedisValue)(user is { Length: > 0 } ? user : RedisLiterals.@default)}{(RedisValue)password!}", + $"{RedisCommand.HELLO}{protover}{RespLiterals.Auth}{(user is { Length: > 0 } ? user.AsRedisValue() : RedisLiterals.@default)}{password!.AsRedisValue()}", handler: HelloHandler.Instance) .ConfigureAwait(false) : await context.SendAsync( @@ -1299,11 +1299,11 @@ static async Task TellAsync(RespDatabaseContext context, RedisValue verb, RedisV { if (field.IsNull) { - await context.SendAsync($"{RedisCommand.CLIENT}{verb}{(RedisValue)value}").ConfigureAwait(false); + await context.SendAsync($"{RedisCommand.CLIENT}{verb}{value.AsRedisValue()}").ConfigureAwait(false); } else { - await context.SendAsync($"{RedisCommand.CLIENT}{verb}{field}{(RedisValue)value}").ConfigureAwait(false); + await context.SendAsync($"{RedisCommand.CLIENT}{verb}{field}{value.AsRedisValue()}").ConfigureAwait(false); } } catch (RedisServerException) diff --git a/src/StackExchange.Redis/Database/RedisDatabase.Scripts.cs b/src/StackExchange.Redis/Database/RedisDatabase.Scripts.cs index b051603c2..71bc11f95 100644 --- a/src/StackExchange.Redis/Database/RedisDatabase.Scripts.cs +++ b/src/StackExchange.Redis/Database/RedisDatabase.Scripts.cs @@ -57,7 +57,7 @@ private ValueTask EvalHash(RespDatabaseContext context, byte[] hash } return context.Scripts.EvaluateDirectResult( - (RedisValue)ToHex(hash), keys ?? [], values ?? [], isHash: true, readOnly, flags); + ToHex(hash).AsRedisValue(), keys ?? [], values ?? [], isHash: true, readOnly, flags); } private static string ToHex(byte[] hash) diff --git a/src/StackExchange.Redis/HotKeys.Server.cs b/src/StackExchange.Redis/HotKeys.Server.cs index 8057d7c19..048ad3994 100644 --- a/src/StackExchange.Redis/HotKeys.Server.cs +++ b/src/StackExchange.Redis/HotKeys.Server.cs @@ -56,32 +56,32 @@ public Task HotKeysResetAsync(CommandFlags flags = CommandFlags.None) /// private static RedisValue[] HotKeysStartArgs(HotKeysMetrics metrics, long count, TimeSpan duration, long sampleRatio, int[]? slots) { - var args = new List { "START", "METRICS" }; + var args = new List { RedisLiterals.START, RedisLiterals.METRICS }; bool cpu = (metrics & HotKeysMetrics.Cpu) != 0, net = (metrics & HotKeysMetrics.Network) != 0; args.Add((cpu ? 1 : 0) + (net ? 1 : 0)); - if (cpu) args.Add("CPU"); - if (net) args.Add("NET"); + if (cpu) args.Add(RedisLiterals.CPU); + if (net) args.Add(RedisLiterals.NET); if (count != 0) { - args.Add("COUNT"); + args.Add(RedisLiterals.COUNT); args.Add(count); } if (duration != TimeSpan.Zero) { - args.Add("DURATION"); + args.Add(RedisLiterals.DURATION); args.Add(Math.Ceiling(duration.TotalSeconds)); } if (sampleRatio != 1) { - args.Add("SAMPLE"); + args.Add(RedisLiterals.SAMPLE); args.Add(sampleRatio); } if (slots is { Length: > 0 }) { - args.Add("SLOTS"); + args.Add(RedisLiterals.SLOTS); args.Add(slots.Length); foreach (var slot in slots) args.Add(slot); } diff --git a/src/StackExchange.Redis/RedisLiterals.cs b/src/StackExchange.Redis/RedisLiterals.cs index 6a2bdc7e5..03f63eb71 100644 --- a/src/StackExchange.Redis/RedisLiterals.cs +++ b/src/StackExchange.Redis/RedisLiterals.cs @@ -28,6 +28,7 @@ public static readonly RedisValue CH = RedisValue.FromRaw("CH"u8), CHANNELS = RedisValue.FromRaw("CHANNELS"u8), COUNT = RedisValue.FromRaw("COUNT"u8), + CPU = RedisValue.FromRaw("CPU"u8), DB = RedisValue.FromRaw("DB"u8), @default = RedisValue.FromRaw("default"u8), DESC = RedisValue.FromRaw("DESC"u8), @@ -35,6 +36,7 @@ public static readonly RedisValue DIFF1 = RedisValue.FromRaw("DIFF1"u8), DISCARD = RedisValue.FromRaw("DISCARD"u8), DOCTOR = RedisValue.FromRaw("DOCTOR"u8), + DURATION = RedisValue.FromRaw("DURATION"u8), ENCODING = RedisValue.FromRaw("ENCODING"u8), EX = RedisValue.FromRaw("EX"u8), EXACTLY = RedisValue.FromRaw("EXACTLY"u8), @@ -83,13 +85,17 @@ public static readonly RedisValue MAX = RedisValue.FromRaw("MAX"u8), MAXAGE = RedisValue.FromRaw("MAXAGE"u8), MAXLEN = RedisValue.FromRaw("MAXLEN"u8), + METRICS = RedisValue.FromRaw("METRICS"u8), MIN = RedisValue.FromRaw("MIN"u8), MINMATCHLEN = RedisValue.FromRaw("MINMATCHLEN"u8), MODULE = RedisValue.FromRaw("MODULE"u8), + NET = RedisValue.FromRaw("NET"u8), NO = RedisValue.FromRaw("NO"u8), NODES = RedisValue.FromRaw("NODES"u8), NOSAVE = RedisValue.FromRaw("NOSAVE"u8), + SAMPLE = RedisValue.FromRaw("SAMPLE"u8), SLOTS = RedisValue.FromRaw("SLOTS"u8), + START = RedisValue.FromRaw("START"u8), nosort = RedisValue.FromRaw("nosort"u8), NOT = RedisValue.FromRaw("NOT"u8), NOVALUES = RedisValue.FromRaw("NOVALUES"u8), diff --git a/src/StackExchange.Redis/RedisServer.cs b/src/StackExchange.Redis/RedisServer.cs index c9797a46e..20138f432 100644 --- a/src/StackExchange.Redis/RedisServer.cs +++ b/src/StackExchange.Redis/RedisServer.cs @@ -307,7 +307,7 @@ private async Task RelearnSettingAsync(string name) { try { - foreach (var pair in await Context.Config.GetArray(name, CommandFlags.None).ConfigureAwait(false)) + foreach (var pair in await Context.Config.GetArray(name.AsRedisValue(), CommandFlags.None).ConfigureAwait(false)) { RespHandshake.ApplySetting(server, pair.Key, pair.Value); } diff --git a/toys/CoreBench/CoreBench.csproj b/toys/CoreBench/CoreBench.csproj index 214a3c661..ab548a824 100644 --- a/toys/CoreBench/CoreBench.csproj +++ b/toys/CoreBench/CoreBench.csproj @@ -11,7 +11,9 @@ true true - + + $(NoWarn);StringToRedisValue +