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; } 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/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/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() { 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 +