From df5370515cd2ed22d9fa95e95642fb1a0f7e5929 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 24 May 2023 17:07:09 +0100 Subject: [PATCH 01/30] start spiking redis-cache output-cache implementation --- ...tensions.Caching.StackExchangeRedis.csproj | 4 + .../net462}/PublicAPI.Shipped.txt | 0 .../net462}/PublicAPI.Unshipped.txt | 0 .../PublicAPI/net8.0/PublicAPI.Shipped.txt | 26 ++ .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 + .../netstandard2.0/PublicAPI.Shipped.txt | 26 ++ .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../src/RedisOutputCacheStore.cs | 267 ++++++++++++++++++ .../src/RedisOutputCacheStoreImpl.cs | 24 ++ ...geRedisCacheServiceCollectionExtensions.cs | 22 ++ .../OutputCaching/OutputCaching.slnf | 2 + 11 files changed, 374 insertions(+) rename src/Caching/StackExchangeRedis/src/{ => PublicAPI/net462}/PublicAPI.Shipped.txt (100%) rename src/Caching/StackExchangeRedis/src/{ => PublicAPI/net462}/PublicAPI.Unshipped.txt (100%) create mode 100644 src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Shipped.txt create mode 100644 src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt create mode 100644 src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt create mode 100644 src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt create mode 100644 src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs create mode 100644 src/Caching/StackExchangeRedis/src/RedisOutputCacheStoreImpl.cs diff --git a/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj b/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj index b73694371d1e..706e4d566321 100644 --- a/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj +++ b/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj @@ -13,6 +13,7 @@ + @@ -21,6 +22,9 @@ + + + diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI.Shipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/net462/PublicAPI.Shipped.txt similarity index 100% rename from src/Caching/StackExchangeRedis/src/PublicAPI.Shipped.txt rename to src/Caching/StackExchangeRedis/src/PublicAPI/net462/PublicAPI.Shipped.txt diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI.Unshipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/net462/PublicAPI.Unshipped.txt similarity index 100% rename from src/Caching/StackExchangeRedis/src/PublicAPI.Unshipped.txt rename to src/Caching/StackExchangeRedis/src/PublicAPI/net462/PublicAPI.Unshipped.txt diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Shipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Shipped.txt new file mode 100644 index 000000000000..bb997fdc8643 --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Shipped.txt @@ -0,0 +1,26 @@ +#nullable enable +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Dispose() -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Get(string! key) -> byte[]? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.GetAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RedisCache(Microsoft.Extensions.Options.IOptions! optionsAccessor) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Refresh(string! key) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RefreshAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Remove(string! key) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RemoveAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Set(string! key, byte[]! value, Microsoft.Extensions.Caching.Distributed.DistributedCacheEntryOptions! options) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.SetAsync(string! key, byte[]! value, Microsoft.Extensions.Caching.Distributed.DistributedCacheEntryOptions! options, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.Configuration.get -> string? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.Configuration.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConfigurationOptions.get -> StackExchange.Redis.ConfigurationOptions? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConfigurationOptions.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConnectionMultiplexerFactory.get -> System.Func!>? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConnectionMultiplexerFactory.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.InstanceName.get -> string? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.InstanceName.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ProfilingSession.get -> System.Func? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ProfilingSession.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.RedisCacheOptions() -> void +Microsoft.Extensions.DependencyInjection.StackExchangeRedisCacheServiceCollectionExtensions +static Microsoft.Extensions.DependencyInjection.StackExchangeRedisCacheServiceCollectionExtensions.AddStackExchangeRedisCache(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! setupAction) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt new file mode 100644 index 000000000000..a38dbc1bfe7b --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -0,0 +1,2 @@ +#nullable enable +static Microsoft.Extensions.DependencyInjection.StackExchangeRedisCacheServiceCollectionExtensions.AddStackExchangeRedisOutputCache(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! setupAction) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt new file mode 100644 index 000000000000..bb997fdc8643 --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt @@ -0,0 +1,26 @@ +#nullable enable +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Dispose() -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Get(string! key) -> byte[]? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.GetAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RedisCache(Microsoft.Extensions.Options.IOptions! optionsAccessor) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Refresh(string! key) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RefreshAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Remove(string! key) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.RemoveAsync(string! key, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.Set(string! key, byte[]! value, Microsoft.Extensions.Caching.Distributed.DistributedCacheEntryOptions! options) -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.SetAsync(string! key, byte[]! value, Microsoft.Extensions.Caching.Distributed.DistributedCacheEntryOptions! options, System.Threading.CancellationToken token = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.Configuration.get -> string? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.Configuration.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConfigurationOptions.get -> StackExchange.Redis.ConfigurationOptions? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConfigurationOptions.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConnectionMultiplexerFactory.get -> System.Func!>? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ConnectionMultiplexerFactory.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.InstanceName.get -> string? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.InstanceName.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ProfilingSession.get -> System.Func? +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.ProfilingSession.set -> void +Microsoft.Extensions.Caching.StackExchangeRedis.RedisCacheOptions.RedisCacheOptions() -> void +Microsoft.Extensions.DependencyInjection.StackExchangeRedisCacheServiceCollectionExtensions +static Microsoft.Extensions.DependencyInjection.StackExchangeRedisCacheServiceCollectionExtensions.AddStackExchangeRedisCache(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! setupAction) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt new file mode 100644 index 000000000000..7dc5c58110bf --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +#nullable enable diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs new file mode 100644 index 000000000000..26c87e4fef74 --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -0,0 +1,267 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET7_0_OR_GREATER // IOutputCacheStore only exists from net7 + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Net.Sockets; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.OutputCaching; +using Microsoft.AspNetCore.Shared; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using StackExchange.Redis; + +namespace Microsoft.Extensions.Caching.StackExchangeRedis; + +internal class RedisOutputCacheStore : IOutputCacheStore, IDisposable +{ + private readonly RedisCacheOptions _options; + private readonly ILogger _logger; + private readonly RedisKey _valueKeyPrefix, _tagKeyPrefix, _tagMasterKey; + private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(initialCount: 1, maxCount: 1); + + private bool _disposed; + private volatile IDatabase? _cache; + private long _lastConnectTicks = DateTimeOffset.UtcNow.Ticks; + private long _firstErrorTimeTicks; + private long _previousErrorTimeTicks; + + // Never reconnect within 60 seconds of the last attempt to connect or reconnect. + private readonly TimeSpan ReconnectMinInterval = TimeSpan.FromSeconds(60); + // Only reconnect if errors have occurred for at least the last 30 seconds. + // This count resets if there are no errors for 30 seconds + private readonly TimeSpan ReconnectErrorThreshold = TimeSpan.FromSeconds(30); + + /// + /// Initializes a new instance of . + /// + /// The configuration options. + public RedisOutputCacheStore(IOptions optionsAccessor) // TODO: OC-specific options? + : this(optionsAccessor, Logging.Abstractions.NullLoggerFactory.Instance.CreateLogger()) + { + } + + /// + /// Initializes a new instance of . + /// + /// The configuration options. + /// The logger. + internal RedisOutputCacheStore(IOptions optionsAccessor, ILogger logger) + { + ArgumentNullThrowHelper.ThrowIfNull(optionsAccessor); + ArgumentNullThrowHelper.ThrowIfNull(logger); + + _options = optionsAccessor.Value; + _logger = logger; + + // This allows partitioning a single backend cache for use with multiple apps/services. + + // SE.Redis allows efficient append of key-prefix scenarios, but we can help it + // avoid some work/allocations by forcing the key-prefix to be a byte[]; SE.Redis + // would do this itself anyway, using UTF8 + _valueKeyPrefix = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCV_"); + _tagKeyPrefix = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT_"); + _tagMasterKey = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT"); + } + + ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancellationToken) + => throw new NotImplementedException(); + + async ValueTask IOutputCacheStore.GetAsync(string key, CancellationToken cancellationToken) + { + ArgumentNullThrowHelper.ThrowIfNull(key); + + var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); + Debug.Assert(cache is not null); + + try + { + return (byte[]?)(await cache.StringGetAsync(key).ConfigureAwait(false)); + } + catch (Exception ex) + { + OnRedisError(ex, cache); + throw; + } + } + + async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken) + { + if (tags is not null && tags.Length > 0) + { + throw new NotImplementedException("tags"); + } + + var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); + Debug.Assert(cache is not null); + + await cache.StringSetAsync(key, value, validFor).ConfigureAwait(false); + } + + private ValueTask ConnectAsync(CancellationToken token = default) + { + CheckDisposed(); + token.ThrowIfCancellationRequested(); + + var cache = _cache; + if (cache is not null) + { + Debug.Assert(_cache is not null); + return new(cache); + } + return ConnectSlowAsync(token); + } + private async ValueTask ConnectSlowAsync(CancellationToken token) + { + await _connectionLock.WaitAsync(token).ConfigureAwait(false); + try + { + var cache = _cache; + if (cache is null) + { + IConnectionMultiplexer connection; + if (_options.ConnectionMultiplexerFactory is null) + { + if (_options.ConfigurationOptions is not null) + { + connection = await ConnectionMultiplexer.ConnectAsync(_options.ConfigurationOptions).ConfigureAwait(false); + } + else + { + connection = await ConnectionMultiplexer.ConnectAsync(_options.Configuration!).ConfigureAwait(false); + } + } + else + { + connection = await _options.ConnectionMultiplexerFactory().ConfigureAwait(false); + } + + PrepareConnection(connection); + cache = _cache = connection.GetDatabase(); + } + Debug.Assert(_cache is not null); + return cache; + } + finally + { + _connectionLock.Release(); + } + } + + /// + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + ReleaseConnection(Interlocked.Exchange(ref _cache, null)); + } + + private void OnRedisError(Exception exception, IDatabase cache) + { + if (_options.UseForceReconnect && (exception is RedisConnectionException or SocketException)) + { + var utcNow = DateTimeOffset.UtcNow; + var previousConnectTime = ReadTimeTicks(ref _lastConnectTicks); + TimeSpan elapsedSinceLastReconnect = utcNow - previousConnectTime; + + // We want to limit how often we perform this top-level reconnect, so we check how long it's been since our last attempt. + if (elapsedSinceLastReconnect < ReconnectMinInterval) + { + return; + } + + var firstErrorTime = ReadTimeTicks(ref _firstErrorTimeTicks); + if (firstErrorTime == DateTimeOffset.MinValue) + { + // note: order/timing here (between the two fields) is not critical + WriteTimeTicks(ref _firstErrorTimeTicks, utcNow); + WriteTimeTicks(ref _previousErrorTimeTicks, utcNow); + return; + } + + TimeSpan elapsedSinceFirstError = utcNow - firstErrorTime; + TimeSpan elapsedSinceMostRecentError = utcNow - ReadTimeTicks(ref _previousErrorTimeTicks); + + bool shouldReconnect = + elapsedSinceFirstError >= ReconnectErrorThreshold // Make sure we gave the multiplexer enough time to reconnect on its own if it could. + && elapsedSinceMostRecentError <= ReconnectErrorThreshold; // Make sure we aren't working on stale data (e.g. if there was a gap in errors, don't reconnect yet). + + // Update the previousErrorTime timestamp to be now (e.g. this reconnect request). + WriteTimeTicks(ref _previousErrorTimeTicks, utcNow); + + if (!shouldReconnect) + { + return; + } + + WriteTimeTicks(ref _firstErrorTimeTicks, DateTimeOffset.MinValue); + WriteTimeTicks(ref _previousErrorTimeTicks, DateTimeOffset.MinValue); + + // wipe the shared field, but *only* if it is still the cache we were + // thinking about (once it is null, the next caller will reconnect) + ReleaseConnection(Interlocked.CompareExchange(ref _cache, null, cache)); + } + } + + private void PrepareConnection(IConnectionMultiplexer connection) + { + WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow); + // ValidateServerFeatures(connection); + TryRegisterProfiler(connection); + } + + private void TryRegisterProfiler(IConnectionMultiplexer connection) + { + _ = connection ?? throw new InvalidOperationException($"{nameof(connection)} cannot be null."); + + if (_options.ProfilingSession is not null) + { + connection.RegisterProfiler(_options.ProfilingSession); + } + } + + private static void WriteTimeTicks(ref long field, DateTimeOffset value) + { + var ticks = value == DateTimeOffset.MinValue ? 0L : value.UtcTicks; + Volatile.Write(ref field, ticks); // avoid torn values + } + + private void CheckDisposed() + { + ObjectDisposedThrowHelper.ThrowIf(_disposed, this); + } + + private static DateTimeOffset ReadTimeTicks(ref long field) + { + var ticks = Volatile.Read(ref field); // avoid torn values + return ticks == 0 ? DateTimeOffset.MinValue : new DateTimeOffset(ticks, TimeSpan.Zero); + } + + static void ReleaseConnection(IDatabase? cache) + { + var connection = cache?.Multiplexer; + if (connection is not null) + { + try + { + connection.Close(); + connection.Dispose(); + } + catch (Exception ex) + { + Debug.WriteLine(ex); + } + } + } +} +#endif diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStoreImpl.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStoreImpl.cs new file mode 100644 index 000000000000..e27638aa6400 --- /dev/null +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStoreImpl.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET7_0_OR_GREATER // IOutputCacheStore only exists from net7 + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Caching.StackExchangeRedis; + +internal sealed class RedisOutputCacheStoreImpl : RedisOutputCacheStore +{ + public RedisOutputCacheStoreImpl(IOptions optionsAccessor, ILogger logger) + : base(optionsAccessor, logger) + { + } + + public RedisOutputCacheStoreImpl(IOptions optionsAccessor) + : base(optionsAccessor) + { + } +} + +#endif diff --git a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs index dc9bc9ca5357..28820d725f60 100644 --- a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs +++ b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs @@ -32,4 +32,26 @@ public static IServiceCollection AddStackExchangeRedisCache(this IServiceCollect return services; } + +#if NET7_0_OR_GREATER + /// + /// Adds Redis distributed caching services to the specified . + /// + /// The to add services to. + /// An to configure the provided + /// . + /// The so that additional calls can be chained. + public static IServiceCollection AddStackExchangeRedisOutputCache(this IServiceCollection services, Action setupAction) + { + ArgumentNullThrowHelper.ThrowIfNull(services); + ArgumentNullThrowHelper.ThrowIfNull(setupAction); + + services.AddOptions(); + + services.Configure(setupAction); + services.Add(ServiceDescriptor.Singleton()); + + return services; + } +#endif } diff --git a/src/Middleware/OutputCaching/OutputCaching.slnf b/src/Middleware/OutputCaching/OutputCaching.slnf index 7d85ca7205c3..872e454585eb 100644 --- a/src/Middleware/OutputCaching/OutputCaching.slnf +++ b/src/Middleware/OutputCaching/OutputCaching.slnf @@ -2,6 +2,8 @@ "solution": { "path": "..\\..\\..\\AspNetCore.sln", "projects": [ + "src\\Caching\\StackExchangeRedis\\src\\Microsoft.Extensions.Caching.StackExchangeRedis.csproj", + "src\\Caching\\StackExchangeRedis\\test\\Microsoft.Extensions.Caching.StackExchangeRedis.Tests.csproj", "src\\Middleware\\OutputCaching\\samples\\OutputCachingSample\\OutputCachingSample.csproj", "src\\Middleware\\OutputCaching\\src\\Microsoft.AspNetCore.OutputCaching.csproj", "src\\Middleware\\OutputCaching\\test\\Microsoft.AspNetCore.OutputCaching.Tests.csproj" From 28bf900b54c170bac56a21c6db161f6dd106669b Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 25 May 2023 15:08:19 +0100 Subject: [PATCH 02/30] basic tests --- ...tensions.Caching.StackExchangeRedis.csproj | 1 + .../src/RedisOutputCacheStore.cs | 7 +- .../OutputCache/OutputCacheGetSetTests.cs | 81 +++++++++++ .../OutputCacheServiceExtensionsTests.cs | 129 ++++++++++++++++++ .../OutputCache/RedisConnectionFixture.cs | 27 ++++ 5 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs create mode 100644 src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheServiceExtensionsTests.cs create mode 100644 src/Caching/StackExchangeRedis/test/OutputCache/RedisConnectionFixture.cs diff --git a/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj b/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj index 706e4d566321..b5a3d703a4ab 100644 --- a/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj +++ b/src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj @@ -16,6 +16,7 @@ + diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 26c87e4fef74..eeb34a98e4b5 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -73,6 +73,9 @@ internal RedisOutputCacheStore(IOptions optionsAccessor, ILog ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancellationToken) => throw new NotImplementedException(); + private RedisKey GetValueKey(string key) + => _valueKeyPrefix.Append(key); + async ValueTask IOutputCacheStore.GetAsync(string key, CancellationToken cancellationToken) { ArgumentNullThrowHelper.ThrowIfNull(key); @@ -82,7 +85,7 @@ ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancel try { - return (byte[]?)(await cache.StringGetAsync(key).ConfigureAwait(false)); + return (byte[]?)(await cache.StringGetAsync(GetValueKey(key)).ConfigureAwait(false)); } catch (Exception ex) { @@ -101,7 +104,7 @@ async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? t var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); Debug.Assert(cache is not null); - await cache.StringSetAsync(key, value, validFor).ConfigureAwait(false); + await cache.StringSetAsync(GetValueKey(key), value, validFor).ConfigureAwait(false); } private ValueTask ConnectAsync(CancellationToken token = default) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs new file mode 100644 index 000000000000..4e9813758bd7 --- /dev/null +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -0,0 +1,81 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET7_0_OR_GREATER + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.OutputCaching; +using StackExchange.Redis; +using Xunit; + +namespace Microsoft.Extensions.Caching.StackExchangeRedis; + +public class OutputCacheGetSetTests : IClassFixture +{ + private readonly IOutputCacheStore _cache; + public OutputCacheGetSetTests(RedisConnectionFixture connection) + { + _fixture = connection; + _cache = new RedisOutputCacheStore(new RedisCacheOptions + { + ConnectionMultiplexerFactory = () => Task.FromResult(_fixture.Connection), + InstanceName = "TestPrefix", + }); + } + + private readonly RedisConnectionFixture _fixture; + + [Fact] + public async Task GetMissingKeyReturnsNull() + { + var result = await _cache.GetAsync("non-existent-key", CancellationToken.None); + Assert.Null(result); + } + + [Fact] + public async Task SetStoresValueWithPrefixAndTimeout() + { + var key = Guid.NewGuid().ToString(); + byte[] storedValue = new byte[1017]; + Random.Shared.NextBytes(storedValue); + RedisKey underlyingKey = "TestPrefix__MSOCV_" + key; + + // pre-check + var timeout = await _fixture.Database.KeyTimeToLiveAsync(underlyingKey); + Assert.Null(timeout); // means doesn't exist + + // act + await _cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + + // validate via redis direct + timeout = await _fixture.Database.KeyTimeToLiveAsync(underlyingKey); + Assert.NotNull(timeout); // means exists + var seconds = timeout.Value.TotalSeconds; + Assert.True(seconds >= 28 && seconds <= 32, "timeout should be in range"); + var redisValue = (byte[])(await _fixture.Database.StringGetAsync(underlyingKey)); + Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(redisValue), "payload should match"); + } + + [Fact] + public async Task CanFetchStoredValue() + { + var key = Guid.NewGuid().ToString(); + byte[] storedValue = new byte[1017]; + Random.Shared.NextBytes(storedValue); + + // pre-check + var fetchedValue = await _cache.GetAsync(key, CancellationToken.None); + Assert.Null(fetchedValue); + + // store and fetch via service + await _cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + fetchedValue = await _cache.GetAsync(key, CancellationToken.None); + Assert.NotNull(fetchedValue); + + Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); + } +} + +#endif diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheServiceExtensionsTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheServiceExtensionsTests.cs new file mode 100644 index 000000000000..37efde7c19da --- /dev/null +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheServiceExtensionsTests.cs @@ -0,0 +1,129 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET7_0_OR_GREATER + +using System.Linq; +using Microsoft.AspNetCore.OutputCaching; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Microsoft.Extensions.Caching.StackExchangeRedis; + +public class OutputCacheServiceExtensionsTests +{ + [Fact] + public void AddStackExchangeRedisOutputCache_RegistersOutputCacheStoreAsSingleton() + { + // Arrange + var services = new ServiceCollection(); + + // Act + services.AddStackExchangeRedisOutputCache(options => { }); + + // Assert + var outputCacheStore = services.FirstOrDefault(desc => desc.ServiceType == typeof(IOutputCacheStore)); + + Assert.NotNull(outputCacheStore); + Assert.Equal(ServiceLifetime.Singleton, outputCacheStore.Lifetime); + } + + [Fact] + public void AddStackExchangeRedisOutputCache_ReplacesPreviouslyUserRegisteredServices() + { + // Arrange + var services = new ServiceCollection(); + services.AddScoped(typeof(IOutputCacheStore), sp => Mock.Of()); + + // Act + services.AddStackExchangeRedisOutputCache(options => { }); + + // Assert + var serviceProvider = services.BuildServiceProvider(); + + var distributedCache = services.FirstOrDefault(desc => desc.ServiceType == typeof(IOutputCacheStore)); + + Assert.NotNull(distributedCache); + Assert.Equal(ServiceLifetime.Scoped, distributedCache.Lifetime); + Assert.IsAssignableFrom(serviceProvider.GetRequiredService()); + } + + [Fact] + public void AddStackExchangeRedisOutputCache_AllowsChaining() + { + var services = new ServiceCollection(); + + Assert.Same(services, services.AddStackExchangeRedisOutputCache(_ => { })); + } + + [Fact] + public void AddStackExchangeRedisOutputCache_IOutputCacheStoreWithoutLoggingCanBeResolved() + { + // Arrange + var services = new ServiceCollection(); + + // Act + services.AddStackExchangeRedisOutputCache(options => { }); + + // Assert + using var serviceProvider = services.BuildServiceProvider(); + var outputCacheStore = serviceProvider.GetRequiredService(); + + Assert.NotNull(outputCacheStore); + } + + [Fact] + public void AddStackExchangeRedisOutputCache_IOutputCacheStoreWithLoggingCanBeResolved() + { + // Arrange + var services = new ServiceCollection(); + + // Act + services.AddStackExchangeRedisOutputCache(options => { }); + services.AddLogging(); + + // Assert + using var serviceProvider = services.BuildServiceProvider(); + var outputCacheStore = serviceProvider.GetRequiredService(); + + Assert.NotNull(outputCacheStore); + } + + [Fact] + public void AddStackExchangeRedisOutputCache_UsesLoggerFactoryAlreadyRegisteredWithServiceCollection() + { + // Arrange + var services = new ServiceCollection(); + services.AddScoped(typeof(IOutputCacheStore), sp => Mock.Of()); + + var loggerFactory = new Mock(); + + loggerFactory + .Setup(lf => lf.CreateLogger(It.IsAny())) + .Returns((string name) => NullLoggerFactory.Instance.CreateLogger(name)) + .Verifiable(); + + services.AddScoped(typeof(ILoggerFactory), _ => loggerFactory.Object); + + // Act + services.AddLogging(); + services.AddStackExchangeRedisOutputCache(options => { }); + + // Assert + var serviceProvider = services.BuildServiceProvider(); + + var distributedCache = services.FirstOrDefault(desc => desc.ServiceType == typeof(IOutputCacheStore)); + + Assert.NotNull(distributedCache); + Assert.Equal(ServiceLifetime.Scoped, distributedCache.Lifetime); + Assert.IsAssignableFrom(serviceProvider.GetRequiredService()); + + loggerFactory.Verify(); + } +} + +#endif diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/RedisConnectionFixture.cs b/src/Caching/StackExchangeRedis/test/OutputCache/RedisConnectionFixture.cs new file mode 100644 index 000000000000..6188beb0d691 --- /dev/null +++ b/src/Caching/StackExchangeRedis/test/OutputCache/RedisConnectionFixture.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET7_0_OR_GREATER + +using System; +using StackExchange.Redis; +using Xunit; + +namespace Microsoft.Extensions.Caching.StackExchangeRedis; + +public class RedisConnectionFixture : IDisposable +{ + private readonly ConnectionMultiplexer _muxer; + public RedisConnectionFixture() + { + _muxer = ConnectionMultiplexer.Connect("127.0.0.1:6379"); + } + + public IDatabase Database => _muxer.GetDatabase(); + + public IConnectionMultiplexer Connection => _muxer; + + public void Dispose() => _muxer.Dispose(); +} + +#endif From 276da17d75e8aa82cb6687e5e77e3cd34f3ebc93 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 25 May 2023 15:30:02 +0100 Subject: [PATCH 03/30] detect server features --- .../src/RedisOutputCacheStore.cs | 33 ++++++++++++++++++- .../OutputCache/OutputCacheGetSetTests.cs | 29 ++++++++++++---- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index eeb34a98e4b5..0713a62b7749 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -31,6 +31,7 @@ internal class RedisOutputCacheStore : IOutputCacheStore, IDisposable private long _lastConnectTicks = DateTimeOffset.UtcNow.Ticks; private long _firstErrorTimeTicks; private long _previousErrorTimeTicks; + private bool _useMultiExec, _use62Features; // Never reconnect within 60 seconds of the last attempt to connect or reconnect. private readonly TimeSpan ReconnectMinInterval = TimeSpan.FromSeconds(60); @@ -47,6 +48,12 @@ public RedisOutputCacheStore(IOptions optionsAccessor) // TOD { } + internal async ValueTask GetConfigurationInfo(CancellationToken cancellationToken = default) + { + await ConnectAsync(cancellationToken).ConfigureAwait(false); + return $"redis output-cache; MULTI/EXEC: {_useMultiExec}, v6.2+: {_use62Features}"; + } + /// /// Initializes a new instance of . /// @@ -219,10 +226,34 @@ private void OnRedisError(Exception exception, IDatabase cache) private void PrepareConnection(IConnectionMultiplexer connection) { WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow); - // ValidateServerFeatures(connection); + ValidateServerFeatures(connection); TryRegisterProfiler(connection); } + private void ValidateServerFeatures(IConnectionMultiplexer connection) + { + int serverCount = 0, standaloneCount = 0, v62_Count = 0; + foreach (var ep in connection.GetEndPoints()) + { + var server = connection.GetServer(ep); + if (server is null) + { + continue; // wat? + } + serverCount++; + if (server.ServerType == ServerType.Standalone) + { + standaloneCount++; + } + if (server.Features.SortedSetRangeStore) // just a random v6.2 feature + { + v62_Count++; + } + } + _useMultiExec = serverCount == standaloneCount; + _use62Features = serverCount == v62_Count; + } + private void TryRegisterProfiler(IConnectionMultiplexer connection) { _ = connection ?? throw new InvalidOperationException($"{nameof(connection)} cannot be null."); diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 4e9813758bd7..1c0f848266a1 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -9,13 +9,17 @@ using Microsoft.AspNetCore.OutputCaching; using StackExchange.Redis; using Xunit; +using Xunit.Abstractions; namespace Microsoft.Extensions.Caching.StackExchangeRedis; public class OutputCacheGetSetTests : IClassFixture { private readonly IOutputCacheStore _cache; - public OutputCacheGetSetTests(RedisConnectionFixture connection) + private readonly RedisConnectionFixture _fixture; + private readonly ITestOutputHelper Log; + + public OutputCacheGetSetTests(RedisConnectionFixture connection, ITestOutputHelper log) { _fixture = connection; _cache = new RedisOutputCacheStore(new RedisCacheOptions @@ -23,20 +27,30 @@ public OutputCacheGetSetTests(RedisConnectionFixture connection) ConnectionMultiplexerFactory = () => Task.FromResult(_fixture.Connection), InstanceName = "TestPrefix", }); + Log = log; } - private readonly RedisConnectionFixture _fixture; + private async ValueTask Cache() + { + if (_cache is RedisOutputCacheStore real) + { + Log.WriteLine(await real.GetConfigurationInfo().ConfigureAwait(false)); + } + return _cache; + } [Fact] public async Task GetMissingKeyReturnsNull() { - var result = await _cache.GetAsync("non-existent-key", CancellationToken.None); + var cache = await Cache().ConfigureAwait(false); + var result = await cache.GetAsync("non-existent-key", CancellationToken.None); Assert.Null(result); } [Fact] public async Task SetStoresValueWithPrefixAndTimeout() { + var cache = await Cache().ConfigureAwait(false); var key = Guid.NewGuid().ToString(); byte[] storedValue = new byte[1017]; Random.Shared.NextBytes(storedValue); @@ -47,7 +61,7 @@ public async Task SetStoresValueWithPrefixAndTimeout() Assert.Null(timeout); // means doesn't exist // act - await _cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); // validate via redis direct timeout = await _fixture.Database.KeyTimeToLiveAsync(underlyingKey); @@ -61,17 +75,18 @@ public async Task SetStoresValueWithPrefixAndTimeout() [Fact] public async Task CanFetchStoredValue() { + var cache = await Cache().ConfigureAwait(false); var key = Guid.NewGuid().ToString(); byte[] storedValue = new byte[1017]; Random.Shared.NextBytes(storedValue); // pre-check - var fetchedValue = await _cache.GetAsync(key, CancellationToken.None); + var fetchedValue = await cache.GetAsync(key, CancellationToken.None); Assert.Null(fetchedValue); // store and fetch via service - await _cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); - fetchedValue = await _cache.GetAsync(key, CancellationToken.None); + await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + fetchedValue = await cache.GetAsync(key, CancellationToken.None); Assert.NotNull(fetchedValue); Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); From 1e1ff1969c67892b1d2e50a226eed2580502dd63 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 25 May 2023 15:48:16 +0100 Subject: [PATCH 04/30] add DIM for read-only-sequence API --- .../src/RedisOutputCacheStore.cs | 34 +++++++++++++++++++ .../OutputCache/OutputCacheGetSetTests.cs | 31 +++++++++++++---- .../OutputCaching/src/IOutputCacheStore.cs | 17 ++++++++++ .../OutputCaching/src/PublicAPI.Unshipped.txt | 1 + 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 0713a62b7749..894899a8a5ca 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -4,6 +4,7 @@ #if NET7_0_OR_GREATER // IOutputCacheStore only exists from net7 using System; +using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -114,6 +115,39 @@ async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? t await cache.StringSetAsync(GetValueKey(key), value, validFor).ConfigureAwait(false); } + async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence value, IReadOnlySet? tags, TimeSpan validFor, CancellationToken cancellationToken) + { + if (tags is not null && tags.Count > 0) + { + throw new NotImplementedException("tags"); + } + + var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); + Debug.Assert(cache is not null); + + byte[]? leased = null; + ReadOnlyMemory singleChunk; + if (value.IsSingleSegment) + { + singleChunk = value.First; + } + else + { + int len = checked((int)value.Length); + leased = ArrayPool.Shared.Rent(len); + value.CopyTo(leased); + singleChunk = new(leased, 0, len); + } + + await cache.StringSetAsync(GetValueKey(key), singleChunk, validFor).ConfigureAwait(false); + + // only return lease on success + if (leased is not null) + { + ArrayPool.Shared.Return(leased); + } + } + private ValueTask ConnectAsync(CancellationToken token = default) { CheckDisposed(); diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 1c0f848266a1..ae8ec69a4317 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -4,6 +4,7 @@ #if NET7_0_OR_GREATER using System; +using System.Buffers; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.OutputCaching; @@ -47,8 +48,10 @@ public async Task GetMissingKeyReturnsNull() Assert.Null(result); } - [Fact] - public async Task SetStoresValueWithPrefixAndTimeout() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence) { var cache = await Cache().ConfigureAwait(false); var key = Guid.NewGuid().ToString(); @@ -61,7 +64,14 @@ public async Task SetStoresValueWithPrefixAndTimeout() Assert.Null(timeout); // means doesn't exist // act - await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + if (useReadOnlySequence) + { + await cache.SetAsync(key, new ReadOnlySequence(storedValue), null, TimeSpan.FromSeconds(30), CancellationToken.None); + } + else + { + await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + } // validate via redis direct timeout = await _fixture.Database.KeyTimeToLiveAsync(underlyingKey); @@ -72,8 +82,10 @@ public async Task SetStoresValueWithPrefixAndTimeout() Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(redisValue), "payload should match"); } - [Fact] - public async Task CanFetchStoredValue() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanFetchStoredValue(bool useReadOnlySequence) { var cache = await Cache().ConfigureAwait(false); var key = Guid.NewGuid().ToString(); @@ -85,7 +97,14 @@ public async Task CanFetchStoredValue() Assert.Null(fetchedValue); // store and fetch via service - await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + if (useReadOnlySequence) + { + await cache.SetAsync(key, new ReadOnlySequence(storedValue), null, TimeSpan.FromSeconds(30), CancellationToken.None); + } + else + { + await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + } fetchedValue = await cache.GetAsync(key, CancellationToken.None); Assert.NotNull(fetchedValue); diff --git a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs index ffba017ef6f9..70281285e944 100644 --- a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs +++ b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs @@ -1,6 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; +using System.Linq; + namespace Microsoft.AspNetCore.OutputCaching; /// @@ -33,4 +36,18 @@ public interface IOutputCacheStore /// The amount of time the entry will be kept in the cache before expiring, relative to now. /// Indicates that the operation should be cancelled. ValueTask SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken); + + /// + /// Stores the given response in the response cache. + /// + /// The cache key to store the response under. + /// The response cache entry to store; this value is only defined for the duration of the method, and should not be stored without making a copy. + /// The tags associated with the cache entry to store. + /// The amount of time the entry will be kept in the cache before expiring, relative to now. + /// Indicates that the operation should be cancelled. + ValueTask SetAsync(string key, ReadOnlySequence value, IReadOnlySet? tags, TimeSpan validFor, CancellationToken cancellationToken) + { + // compatibility implementation using the original API + return SetAsync(key, value.ToArray(), tags is { Count: > 0 } nonEmpty ? nonEmpty.ToArray() : null, validFor, cancellationToken); + } } diff --git a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt index ebd6fd195922..832b816ccabb 100644 --- a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt @@ -1,3 +1,4 @@ #nullable enable +Microsoft.AspNetCore.OutputCaching.IOutputCacheStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.Collections.Generic.IReadOnlySet? tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.get -> string![]? Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.init -> void From 8c4efa0d295b630c277fefd1da8bba9f27f67c78 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 25 May 2023 18:36:18 +0100 Subject: [PATCH 05/30] spike buffer writer store --- .../src/RedisOutputCacheStore.cs | 38 +++++++++++++++++-- .../OutputCache/OutputCacheGetSetTests.cs | 4 +- .../OutputCaching/src/IOutputCacheStore.cs | 20 +++++++++- .../src/OutputCacheEntryFormatter.cs | 8 +++- .../OutputCaching/src/PublicAPI.Unshipped.txt | 4 +- 5 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 894899a8a5ca..d7325b6a4115 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -20,7 +20,7 @@ namespace Microsoft.Extensions.Caching.StackExchangeRedis; -internal class RedisOutputCacheStore : IOutputCacheStore, IDisposable +internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferWriterStore, IDisposable { private readonly RedisCacheOptions _options; private readonly ILogger _logger; @@ -49,11 +49,13 @@ public RedisOutputCacheStore(IOptions optionsAccessor) // TOD { } - internal async ValueTask GetConfigurationInfo(CancellationToken cancellationToken = default) +#if DEBUG + internal async ValueTask GetConfigurationInfoAsync(CancellationToken cancellationToken = default) { await ConnectAsync(cancellationToken).ConfigureAwait(false); return $"redis output-cache; MULTI/EXEC: {_useMultiExec}, v6.2+: {_use62Features}"; } +#endif /// /// Initializes a new instance of . @@ -102,6 +104,34 @@ private RedisKey GetValueKey(string key) } } + async ValueTask IOutputCacheBufferWriterStore.GetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken) + { + ArgumentNullThrowHelper.ThrowIfNull(key); + ArgumentNullThrowHelper.ThrowIfNull(destination); + + var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); + Debug.Assert(cache is not null); + + Lease? result = null; + try + { + result = await cache.StringGetLeaseAsync(GetValueKey(key)).ConfigureAwait(false); + if (result is null) + { + return false; + } + + destination.Write(result.Span); + return true; + } + catch (Exception ex) + { + result?.Dispose(); + OnRedisError(ex, cache); + throw; + } + } + async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken) { if (tags is not null && tags.Length > 0) @@ -115,9 +145,9 @@ async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? t await cache.StringSetAsync(GetValueKey(key), value, validFor).ConfigureAwait(false); } - async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence value, IReadOnlySet? tags, TimeSpan validFor, CancellationToken cancellationToken) + async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) { - if (tags is not null && tags.Count > 0) + if (!tags.IsEmpty) { throw new NotImplementedException("tags"); } diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index ae8ec69a4317..4fac972a559d 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -33,10 +33,12 @@ public OutputCacheGetSetTests(RedisConnectionFixture connection, ITestOutputHelp private async ValueTask Cache() { +#if DEBUG if (_cache is RedisOutputCacheStore real) { - Log.WriteLine(await real.GetConfigurationInfo().ConfigureAwait(false)); + Log.WriteLine(await real.GetConfigurationInfoAsync().ConfigureAwait(false)); } +#endif return _cache; } diff --git a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs index 70281285e944..2495aa48e906 100644 --- a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs +++ b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs @@ -45,9 +45,25 @@ public interface IOutputCacheStore /// The tags associated with the cache entry to store. /// The amount of time the entry will be kept in the cache before expiring, relative to now. /// Indicates that the operation should be cancelled. - ValueTask SetAsync(string key, ReadOnlySequence value, IReadOnlySet? tags, TimeSpan validFor, CancellationToken cancellationToken) + ValueTask SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) { // compatibility implementation using the original API - return SetAsync(key, value.ToArray(), tags is { Count: > 0 } nonEmpty ? nonEmpty.ToArray() : null, validFor, cancellationToken); + return SetAsync(key, value.ToArray(), tags.ToArray(), validFor, cancellationToken); } } + +/// +/// Represents a store for cached responses that uses a as the target. +/// +public interface IOutputCacheBufferWriterStore : IOutputCacheStore +{ + /// + /// Gets the cached response for the given key, if it exists. + /// If no cached response exists for the given key, null is returned. + /// + /// The cache key to look up. + /// The location to which the value should be written. + /// Indicates that the operation should be cancelled. + /// True if the response cache entry if it exists; otherwise False. + ValueTask GetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken); +} diff --git a/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs b/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs index 50a9066fde5b..13be1acdda48 100644 --- a/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs +++ b/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; using System.Linq; using System.Text; using Microsoft.AspNetCore.OutputCaching.Serialization; @@ -78,7 +79,12 @@ public static async ValueTask StoreAsync(string key, OutputCacheEntry value, Tim Serialize(bufferStream, formatterEntry); - await store.SetAsync(key, bufferStream.ToArray(), value.Tags ?? Array.Empty(), duration, cancellationToken); + if (!bufferStream.TryGetBuffer(out var segment)) + { + segment = bufferStream.ToArray(); + } + + await store.SetAsync(key, new ReadOnlySequence(segment.Array!, segment.Offset, segment.Count), value.Tags, duration, cancellationToken); } // Format: diff --git a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt index 832b816ccabb..38795f5caed6 100644 --- a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt @@ -1,4 +1,6 @@ #nullable enable -Microsoft.AspNetCore.OutputCaching.IOutputCacheStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.Collections.Generic.IReadOnlySet? tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferWriterStore +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferWriterStore.GetAsync(string! key, System.Buffers.IBufferWriter! destination, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +Microsoft.AspNetCore.OutputCaching.IOutputCacheStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.ReadOnlyMemory tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.get -> string![]? Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.init -> void From 131f6bad7b8c160a2e514d3370a7972b251789fb Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 10:48:32 +0100 Subject: [PATCH 06/30] implement and test tag-based evictions --- .../src/RedisOutputCacheStore.cs | 84 ++++++++++--- .../OutputCache/OutputCacheGetSetTests.cs | 115 +++++++++++++++++- .../OutputCaching/src/IOutputCacheStore.cs | 2 +- 3 files changed, 177 insertions(+), 24 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index d7325b6a4115..0cf37d92d220 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -7,7 +7,6 @@ using System.Buffers; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Net.Sockets; using System.Text; using System.Threading; @@ -25,6 +24,7 @@ internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferWrit private readonly RedisCacheOptions _options; private readonly ILogger _logger; private readonly RedisKey _valueKeyPrefix, _tagKeyPrefix, _tagMasterKey; + private readonly RedisKey[] _tagMasterKeyArray; // for use with Lua if needed (to avoid array allocs) private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(initialCount: 1, maxCount: 1); private bool _disposed; @@ -78,14 +78,32 @@ internal RedisOutputCacheStore(IOptions optionsAccessor, ILog _valueKeyPrefix = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCV_"); _tagKeyPrefix = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT_"); _tagMasterKey = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT"); + _tagMasterKeyArray = new[] { _tagMasterKey }; } - ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancellationToken) - => throw new NotImplementedException(); + async ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancellationToken) + { + var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); + Debug.Assert(cache is not null); + + // we'll use fire-and-forget on individual deletes, relying on the paging mechanism + // of ZSCAN to detect fundamental connection problems - so failure will still be reported + const CommandFlags DeleteFlags = CommandFlags.FireAndForget; + + var tagKey = GetTagKey(tag); + await foreach (var entry in cache.SortedSetScanAsync(tagKey).WithCancellation(cancellationToken)) + { + await cache.KeyDeleteAsync(GetValueKey((string)entry.Element!), DeleteFlags).ConfigureAwait(false); + await cache.SortedSetRemoveAsync(tagKey, entry.Element, DeleteFlags).ConfigureAwait(false); + } + } private RedisKey GetValueKey(string key) => _valueKeyPrefix.Append(key); + private RedisKey GetTagKey(string tag) + => _tagKeyPrefix.Append(tag); + async ValueTask IOutputCacheStore.GetAsync(string key, CancellationToken cancellationToken) { ArgumentNullThrowHelper.ThrowIfNull(key); @@ -132,26 +150,14 @@ async ValueTask IOutputCacheBufferWriterStore.GetAsync(string key, IBuffer } } - async ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken) + ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken) { - if (tags is not null && tags.Length > 0) - { - throw new NotImplementedException("tags"); - } - - var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); - Debug.Assert(cache is not null); - - await cache.StringSetAsync(GetValueKey(key), value, validFor).ConfigureAwait(false); + ArgumentNullException.ThrowIfNull(value); + return ((IOutputCacheStore)this).SetAsync(key, new ReadOnlySequence(value), tags.AsMemory(), validFor, cancellationToken); } async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) { - if (!tags.IsEmpty) - { - throw new NotImplementedException("tags"); - } - var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); Debug.Assert(cache is not null); @@ -171,6 +177,40 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va await cache.StringSetAsync(GetValueKey(key), singleChunk, validFor).ConfigureAwait(false); + if (!tags.IsEmpty) + { + long expiryTimestamp = GetExpirationTimestamp(validFor); + var len = tags.Length; + RedisValue[] argv = _use62Features ? null! : new RedisValue[] { RedisValue.Null, expiryTimestamp }; + + // tags are secondary; to avoid latency costs, we'll use fire-and-forget when adding tags - this does + // mean that in theory tag-related error may go undetected, but: this is an acceptable trade-off + const CommandFlags TagCommandFlags = CommandFlags.FireAndForget; + + for (int i = 0; i < len; i++) // can't use span in async method, so: eat a little overhead here + { + var tag = tags.Span[i]; + if (_use62Features) + { + await cache.SortedSetAddAsync(_tagMasterKey, tag, expiryTimestamp, SortedSetWhen.GreaterThan, TagCommandFlags).ConfigureAwait(false); + } + else + { + // semantic equivalent of ZADD GT + const string ZADD_GT = """ + local oldScore = tonumber(redis.call('ZSCORE', KEYS[1], ARGV[1])) + if oldScore == nil or oldScore < tonumber(ARGV[2]) then + redis.call('ZADD', KEYS[1], ARGV[2], ARGV[1]) + end + """; + + argv[0] = tag; + await cache.ScriptEvaluateAsync(ZADD_GT, _tagMasterKeyArray, argv, TagCommandFlags).ConfigureAwait(false); + } + await cache.SortedSetAddAsync(GetTagKey(tag), key, expiryTimestamp, SortedSetWhen.Always, TagCommandFlags).ConfigureAwait(false); + } + } + // only return lease on success if (leased is not null) { @@ -178,6 +218,14 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va } } + // note that by necessity we're interleaving two time systems here; the local time, and the + // time according to redis (and used internally for redis TTLs); in reality, if we disagree + // on time, we have bigger problems, so: this will have to suffice - we cannot reasonably + // push our in-proc time into out-of-proc redis + // TODO: TimeProvider? ISystemClock? + private static long GetExpirationTimestamp(TimeSpan timeout) => + (long)((DateTime.UtcNow + timeout) - DateTime.UnixEpoch).TotalMilliseconds; + private ValueTask ConnectAsync(CancellationToken token = default) { CheckDisposed(); diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 4fac972a559d..0a312db26d46 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.OutputCaching; @@ -51,9 +52,11 @@ public async Task GetMissingKeyReturnsNull() } [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence) + [InlineData(true, false)] + [InlineData(true, true)] + [InlineData(false, false)] + [InlineData(false, true)] + public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence, bool withTags) { var cache = await Cache().ConfigureAwait(false); var key = Guid.NewGuid().ToString(); @@ -62,17 +65,24 @@ public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence) RedisKey underlyingKey = "TestPrefix__MSOCV_" + key; // pre-check + await _fixture.Database.KeyDeleteAsync(new RedisKey[] { "TestPrefix__MSOCT", "TestPrefix__MSOCT_tagA", "TestPrefix__MSOCT_tagB" }); var timeout = await _fixture.Database.KeyTimeToLiveAsync(underlyingKey); Assert.Null(timeout); // means doesn't exist + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT")); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_tagA")); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_tagB")); // act + var actTime = DateTime.UtcNow; + var ttl = TimeSpan.FromSeconds(30); + var tags = withTags ? new[] { "tagA", "tagB" } : null; if (useReadOnlySequence) { - await cache.SetAsync(key, new ReadOnlySequence(storedValue), null, TimeSpan.FromSeconds(30), CancellationToken.None); + await cache.SetAsync(key, new ReadOnlySequence(storedValue), tags, ttl, CancellationToken.None); } else { - await cache.SetAsync(key, storedValue, null, TimeSpan.FromSeconds(30), CancellationToken.None); + await cache.SetAsync(key, storedValue, tags, ttl, CancellationToken.None); } // validate via redis direct @@ -82,6 +92,24 @@ public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence) Assert.True(seconds >= 28 && seconds <= 32, "timeout should be in range"); var redisValue = (byte[])(await _fixture.Database.StringGetAsync(underlyingKey)); Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(redisValue), "payload should match"); + + double expected = (long)((actTime + ttl) - DateTime.UnixEpoch).TotalMilliseconds; + if (withTags) + { + // we expect the tag structure to now exist, with the scores within a bit of a second + const double Tolerance = 100.0; + Assert.Equal((await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "tagA")).Value, expected, Tolerance); + Assert.Equal((await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "tagB")).Value, expected, Tolerance); + Assert.Equal((await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_tagA", key)).Value, expected, Tolerance); + Assert.Equal((await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_tagB", key)).Value, expected, Tolerance); + } + else + { + // we do *not* expect the tag structure to exist + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT")); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_tagA")); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_tagB")); + } } [Theory] @@ -112,6 +140,83 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); } + + [Fact] + public async Task CanEvictByTag() + { + // store some data + var cache = await Cache().ConfigureAwait(false); + byte[] storedValue = new byte[1017]; + Random.Shared.NextBytes(storedValue); + var ttl = TimeSpan.FromSeconds(30); + + var noTags = Guid.NewGuid().ToString(); + await cache.SetAsync(noTags, storedValue, null, ttl, CancellationToken.None); + + var foo = Guid.NewGuid().ToString(); + await cache.SetAsync(foo, storedValue, new[] {"foo"}, ttl, CancellationToken.None); + + var bar = Guid.NewGuid().ToString(); + await cache.SetAsync(bar, storedValue, new[] { "bar" }, ttl, CancellationToken.None); + + var fooBar = Guid.NewGuid().ToString(); + await cache.SetAsync(fooBar, storedValue, new[] { "foo", "bar" }, ttl, CancellationToken.None); + + // assert prior state + Assert.NotNull(await cache.GetAsync(noTags, CancellationToken.None)); + Assert.NotNull(await cache.GetAsync(foo, CancellationToken.None)); + Assert.NotNull(await cache.GetAsync(bar, CancellationToken.None)); + Assert.NotNull(await cache.GetAsync(fooBar, CancellationToken.None)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", noTags)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", foo)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", bar)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", fooBar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", noTags)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", foo)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", bar)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", fooBar)); + + // act + for (int i = 0; i < 2; i++) // loop is to ensure no oddity when working on tags that *don't* have entries + { + await cache.EvictByTagAsync("foo", CancellationToken.None); + Assert.NotNull(await cache.GetAsync(noTags, CancellationToken.None)); + Assert.Null(await cache.GetAsync(foo, CancellationToken.None)); + Assert.NotNull(await cache.GetAsync(bar, CancellationToken.None)); + Assert.Null(await cache.GetAsync(fooBar, CancellationToken.None)); + } + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", noTags)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", foo)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", bar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", fooBar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", noTags)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", foo)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", bar)); + Assert.NotNull(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", fooBar)); + + for (int i = 0; i < 2; i++) // loop is to ensure no oddity when working on tags that *don't* have entries + { + await cache.EvictByTagAsync("bar", CancellationToken.None); + Assert.NotNull(await cache.GetAsync(noTags, CancellationToken.None)); + Assert.Null(await cache.GetAsync(foo, CancellationToken.None)); + Assert.Null(await cache.GetAsync(bar, CancellationToken.None)); + Assert.Null(await cache.GetAsync(fooBar, CancellationToken.None)); + } + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", noTags)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", foo)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", bar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_foo", fooBar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", noTags)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", foo)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", bar)); + Assert.Null(await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT_bar", fooBar)); + + // assert expected state + Assert.NotNull(await cache.GetAsync(noTags, CancellationToken.None)); + Assert.Null(await cache.GetAsync(foo, CancellationToken.None)); + Assert.Null(await cache.GetAsync(bar, CancellationToken.None)); + Assert.Null(await cache.GetAsync(fooBar, CancellationToken.None)); + } } #endif diff --git a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs index 2495aa48e906..7e90a92c9926 100644 --- a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs +++ b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs @@ -48,7 +48,7 @@ public interface IOutputCacheStore ValueTask SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) { // compatibility implementation using the original API - return SetAsync(key, value.ToArray(), tags.ToArray(), validFor, cancellationToken); + return SetAsync(key, value.ToArray(), tags.IsEmpty ? null : tags.ToArray(), validFor, cancellationToken); } } From ef7dfd8b5b7abb881b3a7fc390ee25af2df6e2f1 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 10:55:51 +0100 Subject: [PATCH 07/30] test score increase behavior --- .../OutputCache/OutputCacheGetSetTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 0a312db26d46..cd7eca61e251 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -141,6 +141,33 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); } + [Fact] + public async Task TagScoreWorksWithGreaterThan() + { + // store some data + var cache = await Cache().ConfigureAwait(false); + byte[] storedValue = new byte[1017]; + Random.Shared.NextBytes(storedValue); + var tags = new[] { "gtonly" }; + await _fixture.Database.KeyDeleteAsync("TestPrefix__MSOCT"); // start from nil state + + await cache.SetAsync(Guid.NewGuid().ToString(), storedValue, tags, TimeSpan.FromSeconds(30), CancellationToken.None); + var originalScore = await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "gtonly"); + Assert.NotNull(originalScore); + + // now store something with a shorter ttl; the score should not change + await cache.SetAsync(Guid.NewGuid().ToString(), storedValue, tags, TimeSpan.FromSeconds(15), CancellationToken.None); + var newScore = await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "gtonly"); + Assert.NotNull(newScore); + Assert.Equal(originalScore, newScore); + + // now store something with a longer ttl; the score should increase + await cache.SetAsync(Guid.NewGuid().ToString(), storedValue, tags, TimeSpan.FromSeconds(45), CancellationToken.None); + newScore = await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "gtonly"); + Assert.NotNull(newScore); + Assert.True(newScore > originalScore, "should increase"); + } + [Fact] public async Task CanEvictByTag() { From e6a55c204ccf704a79dcf9ca4619a5d187f6b245 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 10:56:51 +0100 Subject: [PATCH 08/30] naming is hard --- .../test/OutputCache/OutputCacheGetSetTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index cd7eca61e251..128c55c27b44 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -142,7 +142,7 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) } [Fact] - public async Task TagScoreWorksWithGreaterThan() + public async Task MasterTagScoreShouldOnlyIncrease() { // store some data var cache = await Cache().ConfigureAwait(false); From 965ba0148974ac9658e7c4e32193baf6b836bb7d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 11:12:15 +0100 Subject: [PATCH 09/30] nit: let's always use ARGV in forwards order (easier to grok) --- .../StackExchangeRedis/src/RedisOutputCacheStore.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 0cf37d92d220..39c056889cd9 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -181,7 +181,7 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va { long expiryTimestamp = GetExpirationTimestamp(validFor); var len = tags.Length; - RedisValue[] argv = _use62Features ? null! : new RedisValue[] { RedisValue.Null, expiryTimestamp }; + RedisValue[] argv = _use62Features ? null! : new RedisValue[] { expiryTimestamp, RedisValue.Null }; // tags are secondary; to avoid latency costs, we'll use fire-and-forget when adding tags - this does // mean that in theory tag-related error may go undetected, but: this is an acceptable trade-off @@ -198,13 +198,13 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va { // semantic equivalent of ZADD GT const string ZADD_GT = """ - local oldScore = tonumber(redis.call('ZSCORE', KEYS[1], ARGV[1])) - if oldScore == nil or oldScore < tonumber(ARGV[2]) then - redis.call('ZADD', KEYS[1], ARGV[2], ARGV[1]) + local oldScore = tonumber(redis.call('ZSCORE', KEYS[1], ARGV[2])) + if oldScore == nil or oldScore < tonumber(ARGV[1]) then + redis.call('ZADD', KEYS[1], ARGV[1], ARGV[2]) end """; - argv[0] = tag; + argv[1] = tag; await cache.ScriptEvaluateAsync(ZADD_GT, _tagMasterKeyArray, argv, TagCommandFlags).ConfigureAwait(false); } await cache.SortedSetAddAsync(GetTagKey(tag), key, expiryTimestamp, SortedSetWhen.Always, TagCommandFlags).ConfigureAwait(false); From 8193ec6719e541d94bf1016f14aa43103ca075e6 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 11:27:45 +0100 Subject: [PATCH 10/30] can't share ARGV between calls because of F+F/timing --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 39c056889cd9..199606b5468f 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -181,7 +181,6 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va { long expiryTimestamp = GetExpirationTimestamp(validFor); var len = tags.Length; - RedisValue[] argv = _use62Features ? null! : new RedisValue[] { expiryTimestamp, RedisValue.Null }; // tags are secondary; to avoid latency costs, we'll use fire-and-forget when adding tags - this does // mean that in theory tag-related error may go undetected, but: this is an acceptable trade-off @@ -204,8 +203,9 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va end """; - argv[1] = tag; - await cache.ScriptEvaluateAsync(ZADD_GT, _tagMasterKeyArray, argv, TagCommandFlags).ConfigureAwait(false); + // note we're not sharing an ARGV array between tags here because then we'd need to wait on latency to avoid conflicts; + // in reality most caches have very limited tags (if any), so this is not perceived as an issue + await cache.ScriptEvaluateAsync(ZADD_GT, _tagMasterKeyArray, new RedisValue[] { expiryTimestamp, tag }, TagCommandFlags).ConfigureAwait(false); } await cache.SortedSetAddAsync(GetTagKey(tag), key, expiryTimestamp, SortedSetWhen.Always, TagCommandFlags).ConfigureAwait(false); } From 01e371f4cbd9b64473fe099ada036ef6962b4e0d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 11:30:56 +0100 Subject: [PATCH 11/30] return leased buffer sooner (after StringSetAsync) --- .../StackExchangeRedis/src/RedisOutputCacheStore.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 199606b5468f..9c2b64484070 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -176,6 +176,11 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va } await cache.StringSetAsync(GetValueKey(key), singleChunk, validFor).ConfigureAwait(false); + // only return lease on success + if (leased is not null) + { + ArrayPool.Shared.Return(leased); + } if (!tags.IsEmpty) { @@ -210,12 +215,6 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va await cache.SortedSetAddAsync(GetTagKey(tag), key, expiryTimestamp, SortedSetWhen.Always, TagCommandFlags).ConfigureAwait(false); } } - - // only return lease on success - if (leased is not null) - { - ArrayPool.Shared.Return(leased); - } } // note that by necessity we're interleaving two time systems here; the local time, and the From ac80c0754594ebaa0bbe19f7ca3ba402e76b61af Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 11:49:43 +0100 Subject: [PATCH 12/30] test multi-segment writes and read-via-buffer-writer API --- .../OutputCache/OutputCacheGetSetTests.cs | 79 ++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 128c55c27b44..447e15df4ec2 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -141,6 +141,17 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); } + [Fact] + public async Task GetMissingKeyReturnsFalse_BufferWriter() // "true" result checked in MultiSegmentWriteWorksAsExpected_BufferWriter + { + var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); + var key = Guid.NewGuid().ToString(); + + var bufferWriter = new ArrayBufferWriter(); + Assert.False(await cache.GetAsync(key, bufferWriter, CancellationToken.None)); + Assert.Equal(0, bufferWriter.WrittenCount); + } + [Fact] public async Task MasterTagScoreShouldOnlyIncrease() { @@ -181,7 +192,7 @@ public async Task CanEvictByTag() await cache.SetAsync(noTags, storedValue, null, ttl, CancellationToken.None); var foo = Guid.NewGuid().ToString(); - await cache.SetAsync(foo, storedValue, new[] {"foo"}, ttl, CancellationToken.None); + await cache.SetAsync(foo, storedValue, new[] { "foo" }, ttl, CancellationToken.None); var bar = Guid.NewGuid().ToString(); await cache.SetAsync(bar, storedValue, new[] { "bar" }, ttl, CancellationToken.None); @@ -244,6 +255,72 @@ public async Task CanEvictByTag() Assert.Null(await cache.GetAsync(bar, CancellationToken.None)); Assert.Null(await cache.GetAsync(fooBar, CancellationToken.None)); } + + [Fact] + public async Task MultiSegmentWriteWorksAsExpected_Array() + { + // store some data + var first = new Segment(1024, null); + var second = new Segment(1024, first); + var third = new Segment(1024, second); + + Random.Shared.NextBytes(first.Array); + Random.Shared.NextBytes(second.Array); + Random.Shared.NextBytes(third.Array); + var payload = new ReadOnlySequence(first, 800, third, 42); + Assert.False(payload.IsSingleSegment, "multi-segment"); + Assert.Equal(1290, payload.Length); // partial from first and last + + var cache = await Cache().ConfigureAwait(false); + var key = Guid.NewGuid().ToString(); + await cache.SetAsync(key, payload, default, TimeSpan.FromSeconds(30), CancellationToken.None); + + var fetched = await cache.GetAsync(key, CancellationToken.None); + Assert.NotNull(fetched); + ReadOnlyMemory linear = payload.ToArray(); + Assert.True(linear.Span.SequenceEqual(fetched), "payload match"); + } + + [Fact] + public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() + { + // store some data + var first = new Segment(1024, null); + var second = new Segment(1024, first); + var third = new Segment(1024, second); + + Random.Shared.NextBytes(first.Array); + Random.Shared.NextBytes(second.Array); + Random.Shared.NextBytes(third.Array); + var payload = new ReadOnlySequence(first, 800, third, 42); + Assert.False(payload.IsSingleSegment, "multi-segment"); + Assert.Equal(1290, payload.Length); // partial from first and last + + var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); + var key = Guid.NewGuid().ToString(); + await cache.SetAsync(key, payload, default, TimeSpan.FromSeconds(30), CancellationToken.None); + + var bufferWriter = new ArrayBufferWriter(); + Assert.True(await cache.GetAsync(key, bufferWriter, CancellationToken.None)); + Assert.Equal(1290, bufferWriter.WrittenCount); + ReadOnlyMemory linear = payload.ToArray(); + Assert.True(linear.Span.SequenceEqual(bufferWriter.WrittenSpan), "payload match"); + } + + private class Segment : ReadOnlySequenceSegment + { + public Segment(int length, Segment previous = null) + { + if (previous is not null) + { + previous.Next = this; + RunningIndex = previous.RunningIndex + previous.Memory.Length; + } + Array = new byte[length]; + Memory = Array; + } + public byte[] Array { get; } + } } #endif From 8d56814a4273a2a9637ce1f028d2e07f9a8ebe4c Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 16:37:53 +0100 Subject: [PATCH 13/30] implement GC mechanism --- .../src/RedisOutputCacheStore.cs | 73 ++++++++++++++++ .../OutputCache/OutputCacheGetSetTests.cs | 86 ++++++++++++++++++- 2 files changed, 158 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 9c2b64484070..e5ffe1a8ba9d 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -34,6 +34,8 @@ internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferWrit private long _previousErrorTimeTicks; private bool _useMultiExec, _use62Features; + internal bool GarbageCollectionEnabled { get; set; } = true; + // Never reconnect within 60 seconds of the last attempt to connect or reconnect. private readonly TimeSpan ReconnectMinInterval = TimeSpan.FromSeconds(60); // Only reconnect if errors have occurred for at least the last 30 seconds. @@ -79,6 +81,77 @@ internal RedisOutputCacheStore(IOptions optionsAccessor, ILog _tagKeyPrefix = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT_"); _tagMasterKey = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT"); _tagMasterKeyArray = new[] { _tagMasterKey }; + + _ = Task.Run(RunGarbageCollectionLoopAsync); + } + + private async Task RunGarbageCollectionLoopAsync() + { + try + { + while (!Volatile.Read(ref _disposed)) + { + // approx every 5 minutes, with some randomization to prevent spikes of pile-on + var secondsWithJitter = 300 + Random.Shared.Next(-30, 30); + Debug.Assert(secondsWithJitter >= 270 && secondsWithJitter <= 330); + await Task.Delay(TimeSpan.FromSeconds(secondsWithJitter)).ConfigureAwait(false); + try + { + if (GarbageCollectionEnabled) + { + await ExecuteGarbageCollectionAsync(GetExpirationTimestamp(TimeSpan.Zero)).ConfigureAwait(false); + } + } + catch (Exception ex) + { + // this sweep failed; log it + _logger.LogDebug(ex, "Transient error occurred executing redis output-cache GC loop"); + } + } + } + catch (Exception ex) + { + // the entire loop is dead + _logger.LogDebug(ex, "Fatal error occurred executing redis output-cache GC loop"); + } + } + + internal async ValueTask ExecuteGarbageCollectionAsync(long keepValuesGreaterThan, CancellationToken cancellationToken = default) + { + var cache = await ConnectAsync(CancellationToken.None).ConfigureAwait(false); + + var gcKey = _tagMasterKey.Append("GC"); + var gcLifetime = TimeSpan.FromMinutes(5); + if (!await cache.StringSetAsync(gcKey, "", gcLifetime, when: When.NotExists).ConfigureAwait(false)) + { + return -1; // signal competition + } + try + { + // we'll rely on the enumeration of ZSCAN to spot connection failures, and use "best efforts" + // on the individual operations - this avoids per-call latency + const CommandFlags GarbageCollectionFlags = CommandFlags.FireAndForget; + + // the score is the effective timeout, so we simply need to cull everything with scores below "cull", + // for the individual tag sorted-sets, and also the master sorted-set + const int EXTEND_EVERY = 250; // some non-trivial number of work + int extendCountdown = EXTEND_EVERY; + await foreach (var entry in cache.SortedSetScanAsync(_tagMasterKey).WithCancellation(cancellationToken)) + { + await cache.SortedSetRemoveRangeByScoreAsync(GetTagKey((string)entry.Element!), start: 0, stop: keepValuesGreaterThan, flags: GarbageCollectionFlags).ConfigureAwait(false); + if (--extendCountdown <= 0) + { + await cache.KeyExpireAsync(gcKey, gcLifetime).ConfigureAwait(false); + extendCountdown = EXTEND_EVERY; + } + } + // paying latency on the final master-tag purge: is fine + return await cache.SortedSetRemoveRangeByScoreAsync(_tagMasterKey, start: 0, stop: keepValuesGreaterThan).ConfigureAwait(false); + } + finally + { + await cache.KeyDeleteAsync(gcKey, CommandFlags.FireAndForget).ConfigureAwait(false); + } } async ValueTask IOutputCacheStore.EvictByTagAsync(string tag, CancellationToken cancellationToken) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 447e15df4ec2..5babed5528a9 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -6,9 +6,11 @@ using System; using System.Buffers; using System.Runtime.CompilerServices; +using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.OutputCaching; +using Microsoft.Diagnostics.Runtime.Interop; using StackExchange.Redis; using Xunit; using Xunit.Abstractions; @@ -28,7 +30,10 @@ public OutputCacheGetSetTests(RedisConnectionFixture connection, ITestOutputHelp { ConnectionMultiplexerFactory = () => Task.FromResult(_fixture.Connection), InstanceName = "TestPrefix", - }); + }) + { + GarbageCollectionEnabled = false, + }; Log = log; } @@ -307,6 +312,85 @@ public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() Assert.True(linear.Span.SequenceEqual(bufferWriter.WrittenSpan), "payload match"); } + [Fact] + public async Task GarbageCollectionDoesNotRunWhenGCKeyHeld() + { + var cache = await Cache().ConfigureAwait(false); + var impl = Assert.IsAssignableFrom(cache); + await _fixture.Database.StringSetAsync("TestPrefix__MSOCTGC", "dummy", TimeSpan.FromMinutes(1)); + try + { + Assert.Equal(-1, await impl.ExecuteGarbageCollectionAsync(42)); + } + finally + { + await _fixture.Database.KeyDeleteAsync("TestPrefix__MSOCTGC"); + } + } + + [Fact] + public async Task GarbageCollectionCleansUpTagData() + { + // importantly, we're not interested in the lifetime of the *values* - redis deals with that + // itself; we're only interested in the tag-expiry metadata + var blob = new byte[16]; + Random.Shared.NextBytes(blob); + var cache = await Cache().ConfigureAwait(false); + var impl = Assert.IsAssignableFrom(cache); + + // start vanilla + await _fixture.Database.KeyDeleteAsync(new RedisKey[] { "TestPrefix__MSOCT", + "TestPrefix__MSOCT_a", "TestPrefix__MSOCT_b", + "TestPrefix__MSOCT_c", "TestPrefix__MSOCT_d" }); + + await cache.SetAsync(Guid.NewGuid().ToString(), blob, new[] { "a", "b" }, TimeSpan.FromSeconds(5), CancellationToken.None); // a=b=5 + await cache.SetAsync(Guid.NewGuid().ToString(), blob, new[] { "b", "c" }, TimeSpan.FromSeconds(10), CancellationToken.None); // a=5, b=c=10 + await cache.SetAsync(Guid.NewGuid().ToString(), blob, new[] { "c", "d" }, TimeSpan.FromSeconds(15), CancellationToken.None); // a=5, b=10, c=d=15 + + long aScore = (long)await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "a"), + bScore = (long)await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "b"), + cScore = (long)await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "c"), + dScore = (long)await _fixture.Database.SortedSetScoreAsync("TestPrefix__MSOCT", "d"); + + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCTGC"), "GC key should not exist"); + Assert.True(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT"), "master tag should exist"); + Assert.True(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_a"), "tag a should exist"); + Assert.True(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_b"), "tag b should exist"); + Assert.True(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_c"), "tag c should exist"); + Assert.True(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_d"), "tag d should exist"); + + await CheckCounts(4, 1, 2, 2, 1); + Assert.Equal(0, await impl.ExecuteGarbageCollectionAsync(0)); // should not change anything + await CheckCounts(4, 1, 2, 2, 1); + Assert.Equal(1, await impl.ExecuteGarbageCollectionAsync(aScore)); // 1=removes a + await CheckCounts(3, 0, 1, 2, 1); + Assert.Equal(1, await impl.ExecuteGarbageCollectionAsync(bScore)); // 1=removes b + await CheckCounts(2, 0, 0, 1, 1); + Assert.Equal(2, await impl.ExecuteGarbageCollectionAsync(cScore)); // 2=removes c+d + await CheckCounts(0, 0, 0, 0, 0); + Assert.Equal(0, await impl.ExecuteGarbageCollectionAsync(dScore)); + await CheckCounts(0, 0, 0, 0, 0); + Assert.Equal(0, await impl.ExecuteGarbageCollectionAsync(dScore + 1000)); // should have nothing left to do + await CheckCounts(0, 0, 0, 0, 0); + + // we should now not have any of these left + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCTGC"), "GC key should not exist"); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT"), "master tag still exists"); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_a"), "tag a still exists"); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_b"), "tag b still exists"); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_c"), "tag c still exists"); + Assert.False(await _fixture.Database.KeyExistsAsync("TestPrefix__MSOCT_d"), "tag d still exists"); + + async Task CheckCounts(int master, int a, int b, int c, int d) + { + Assert.Equal(master, (int)await _fixture.Database.SortedSetLengthAsync("TestPrefix__MSOCT")); + Assert.Equal(a, (int)await _fixture.Database.SortedSetLengthAsync("TestPrefix__MSOCT_a")); + Assert.Equal(b, (int)await _fixture.Database.SortedSetLengthAsync("TestPrefix__MSOCT_b")); + Assert.Equal(c, (int)await _fixture.Database.SortedSetLengthAsync("TestPrefix__MSOCT_c")); + Assert.Equal(d, (int)await _fixture.Database.SortedSetLengthAsync("TestPrefix__MSOCT_d")); + } + } + private class Segment : ReadOnlySequenceSegment { public Segment(int length, Segment previous = null) From 101bcce1685b782076381bae4310f8ea060c64af Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 May 2023 17:08:14 +0100 Subject: [PATCH 14/30] use "now" as the value in the GC key --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index e5ffe1a8ba9d..84a6dcc860a7 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -7,6 +7,7 @@ using System.Buffers; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Net.Sockets; using System.Text; using System.Threading; @@ -122,7 +123,8 @@ internal async ValueTask ExecuteGarbageCollectionAsync(long keepValuesGrea var gcKey = _tagMasterKey.Append("GC"); var gcLifetime = TimeSpan.FromMinutes(5); - if (!await cache.StringSetAsync(gcKey, "", gcLifetime, when: When.NotExists).ConfigureAwait(false)) + // value is purely placeholder; it is the existence that matters + if (!await cache.StringSetAsync(gcKey, DateTime.UtcNow.ToString(CultureInfo.InvariantCulture), gcLifetime, when: When.NotExists).ConfigureAwait(false)) { return -1; // signal competition } From 85d439fe802e627a4fc051b72d3f9271309b044d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 15:48:06 +0100 Subject: [PATCH 15/30] disable tests for CI; addressing separately --- .../OutputCache/OutputCacheGetSetTests.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 5babed5528a9..1cc874d0d4f2 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -19,6 +19,10 @@ namespace Microsoft.Extensions.Caching.StackExchangeRedis; public class OutputCacheGetSetTests : IClassFixture { + private const string SkipReason = "TODO: Disabled due to CI failure. " + + "These tests require Redis server to be started on the machine. Make sure to change the value of" + + "\"RedisTestConfig.RedisPort\" accordingly."; + private readonly IOutputCacheStore _cache; private readonly RedisConnectionFixture _fixture; private readonly ITestOutputHelper Log; @@ -48,7 +52,7 @@ private async ValueTask Cache() return _cache; } - [Fact] + [Fact(Skip = SkipReason)] public async Task GetMissingKeyReturnsNull() { var cache = await Cache().ConfigureAwait(false); @@ -56,7 +60,7 @@ public async Task GetMissingKeyReturnsNull() Assert.Null(result); } - [Theory] + [Theory(Skip = SkipReason)] [InlineData(true, false)] [InlineData(true, true)] [InlineData(false, false)] @@ -117,7 +121,7 @@ public async Task SetStoresValueWithPrefixAndTimeout(bool useReadOnlySequence, b } } - [Theory] + [Theory(Skip = SkipReason)] [InlineData(true)] [InlineData(false)] public async Task CanFetchStoredValue(bool useReadOnlySequence) @@ -146,7 +150,7 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) Assert.True(((ReadOnlySpan)storedValue).SequenceEqual(fetchedValue), "payload should match"); } - [Fact] + [Fact(Skip = SkipReason)] public async Task GetMissingKeyReturnsFalse_BufferWriter() // "true" result checked in MultiSegmentWriteWorksAsExpected_BufferWriter { var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); @@ -157,7 +161,7 @@ public async Task GetMissingKeyReturnsFalse_BufferWriter() // "true" result chec Assert.Equal(0, bufferWriter.WrittenCount); } - [Fact] + [Fact(Skip = SkipReason)] public async Task MasterTagScoreShouldOnlyIncrease() { // store some data @@ -184,7 +188,7 @@ public async Task MasterTagScoreShouldOnlyIncrease() Assert.True(newScore > originalScore, "should increase"); } - [Fact] + [Fact(Skip = SkipReason)] public async Task CanEvictByTag() { // store some data @@ -261,7 +265,7 @@ public async Task CanEvictByTag() Assert.Null(await cache.GetAsync(fooBar, CancellationToken.None)); } - [Fact] + [Fact(Skip = SkipReason)] public async Task MultiSegmentWriteWorksAsExpected_Array() { // store some data @@ -286,7 +290,7 @@ public async Task MultiSegmentWriteWorksAsExpected_Array() Assert.True(linear.Span.SequenceEqual(fetched), "payload match"); } - [Fact] + [Fact(Skip = SkipReason)] public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() { // store some data @@ -312,7 +316,7 @@ public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() Assert.True(linear.Span.SequenceEqual(bufferWriter.WrittenSpan), "payload match"); } - [Fact] + [Fact(Skip = SkipReason)] public async Task GarbageCollectionDoesNotRunWhenGCKeyHeld() { var cache = await Cache().ConfigureAwait(false); @@ -328,7 +332,7 @@ public async Task GarbageCollectionDoesNotRunWhenGCKeyHeld() } } - [Fact] + [Fact(Skip = SkipReason)] public async Task GarbageCollectionCleansUpTagData() { // importantly, we're not interested in the lifetime of the *values* - redis deals with that From 63af7c97bb71659a649085575107af7fcf99cb07 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 15:51:59 +0100 Subject: [PATCH 16/30] Update src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 84a6dcc860a7..10f8ed222448 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -266,7 +266,7 @@ async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence va // mean that in theory tag-related error may go undetected, but: this is an acceptable trade-off const CommandFlags TagCommandFlags = CommandFlags.FireAndForget; - for (int i = 0; i < len; i++) // can't use span in async method, so: eat a little overhead here + for (var i = 0; i < len; i++) // can't use span in async method, so: eat a little overhead here { var tag = tags.Span[i]; if (_use62Features) From e3baab3ad74af9c10207e09c8a32d4c076e7b3cf Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 15:54:39 +0100 Subject: [PATCH 17/30] use TaskCreationOptions.LongRunning --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 84a6dcc860a7..ed04f6ecfac3 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -83,7 +83,7 @@ internal RedisOutputCacheStore(IOptions optionsAccessor, ILog _tagMasterKey = (RedisKey)Encoding.UTF8.GetBytes(_options.InstanceName + "__MSOCT"); _tagMasterKeyArray = new[] { _tagMasterKey }; - _ = Task.Run(RunGarbageCollectionLoopAsync); + _ = Task.Factory.StartNew(RunGarbageCollectionLoopAsync, default, TaskCreationOptions.LongRunning, TaskScheduler.Current); } private async Task RunGarbageCollectionLoopAsync() From f909280e3a4339041e1215901c290c63a780dd2e Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 16:26:21 +0100 Subject: [PATCH 18/30] fix CI fail --- .../test/OutputCache/OutputCacheGetSetTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 1cc874d0d4f2..588250ea1818 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -41,16 +41,18 @@ public OutputCacheGetSetTests(RedisConnectionFixture connection, ITestOutputHelp Log = log; } +#if DEBUG private async ValueTask Cache() { -#if DEBUG if (_cache is RedisOutputCacheStore real) { Log.WriteLine(await real.GetConfigurationInfoAsync().ConfigureAwait(false)); } -#endif return _cache; } +#else + private ValueTask Cache() => new(_cache); // avoid CS1998 - no "await" +#endif [Fact(Skip = SkipReason)] public async Task GetMissingKeyReturnsNull() From 2b4740d2ef2d696abdff76ee76ccaca6fc091a89 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 17:16:23 +0100 Subject: [PATCH 19/30] prefer TryAddSingleton --- .../src/StackExchangeRedisCacheServiceCollectionExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs index 28820d725f60..b90aec80eafc 100644 --- a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs +++ b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Shared; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Caching.StackExchangeRedis; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection; @@ -49,7 +50,7 @@ public static IServiceCollection AddStackExchangeRedisOutputCache(this IServiceC services.AddOptions(); services.Configure(setupAction); - services.Add(ServiceDescriptor.Singleton()); + services.TryAddSingleton(); return services; } From 3cd39673b490cea871732cd13cf9833c85cf70db Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Jun 2023 17:17:18 +0100 Subject: [PATCH 20/30] split fields onto separate lines --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 3fc7033c35ac..fd6558001a42 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -24,7 +24,9 @@ internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferWrit { private readonly RedisCacheOptions _options; private readonly ILogger _logger; - private readonly RedisKey _valueKeyPrefix, _tagKeyPrefix, _tagMasterKey; + private readonly RedisKey _valueKeyPrefix; + private readonly RedisKey _tagKeyPrefix; + private readonly RedisKey _tagMasterKey; private readonly RedisKey[] _tagMasterKeyArray; // for use with Lua if needed (to avoid array allocs) private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(initialCount: 1, maxCount: 1); From ee5125f638a37dd3f78f424e7ac8395eafb7ab17 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 15:52:03 +0100 Subject: [PATCH 21/30] update API as per discussion --- .../src/RedisOutputCacheStore.cs | 8 ++--- .../OutputCache/OutputCacheGetSetTests.cs | 12 ++++---- .../OutputCaching/src/IOutputCacheStore.cs | 29 ++++++++----------- .../src/OutputCacheEntryFormatter.cs | 11 ++++++- .../OutputCaching/src/PublicAPI.Unshipped.txt | 6 ++-- 5 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index fd6558001a42..c8ff244d162a 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -20,7 +20,7 @@ namespace Microsoft.Extensions.Caching.StackExchangeRedis; -internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferWriterStore, IDisposable +internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferStore, IDisposable { private readonly RedisCacheOptions _options; private readonly ILogger _logger; @@ -199,7 +199,7 @@ private RedisKey GetTagKey(string tag) } } - async ValueTask IOutputCacheBufferWriterStore.GetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken) + async ValueTask IOutputCacheBufferStore.TryGetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken) { ArgumentNullThrowHelper.ThrowIfNull(key); ArgumentNullThrowHelper.ThrowIfNull(destination); @@ -230,10 +230,10 @@ async ValueTask IOutputCacheBufferWriterStore.GetAsync(string key, IBuffer ValueTask IOutputCacheStore.SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(value); - return ((IOutputCacheStore)this).SetAsync(key, new ReadOnlySequence(value), tags.AsMemory(), validFor, cancellationToken); + return ((IOutputCacheBufferStore)this).SetAsync(key, new ReadOnlySequence(value), tags.AsMemory(), validFor, cancellationToken); } - async ValueTask IOutputCacheStore.SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) + async ValueTask IOutputCacheBufferStore.SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) { var cache = await ConnectAsync(cancellationToken).ConfigureAwait(false); Debug.Assert(cache is not null); diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 588250ea1818..4930e8731e74 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -23,7 +23,7 @@ public class OutputCacheGetSetTests : IClassFixture "These tests require Redis server to be started on the machine. Make sure to change the value of" + "\"RedisTestConfig.RedisPort\" accordingly."; - private readonly IOutputCacheStore _cache; + private readonly IOutputCacheBufferStore _cache; private readonly RedisConnectionFixture _fixture; private readonly ITestOutputHelper Log; @@ -42,7 +42,7 @@ public OutputCacheGetSetTests(RedisConnectionFixture connection, ITestOutputHelp } #if DEBUG - private async ValueTask Cache() + private async ValueTask Cache() { if (_cache is RedisOutputCacheStore real) { @@ -155,11 +155,11 @@ public async Task CanFetchStoredValue(bool useReadOnlySequence) [Fact(Skip = SkipReason)] public async Task GetMissingKeyReturnsFalse_BufferWriter() // "true" result checked in MultiSegmentWriteWorksAsExpected_BufferWriter { - var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); + var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); var key = Guid.NewGuid().ToString(); var bufferWriter = new ArrayBufferWriter(); - Assert.False(await cache.GetAsync(key, bufferWriter, CancellationToken.None)); + Assert.False(await cache.TryGetAsync(key, bufferWriter, CancellationToken.None)); Assert.Equal(0, bufferWriter.WrittenCount); } @@ -307,12 +307,12 @@ public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() Assert.False(payload.IsSingleSegment, "multi-segment"); Assert.Equal(1290, payload.Length); // partial from first and last - var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); + var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); var key = Guid.NewGuid().ToString(); await cache.SetAsync(key, payload, default, TimeSpan.FromSeconds(30), CancellationToken.None); var bufferWriter = new ArrayBufferWriter(); - Assert.True(await cache.GetAsync(key, bufferWriter, CancellationToken.None)); + Assert.True(await cache.TryGetAsync(key, bufferWriter, CancellationToken.None)); Assert.Equal(1290, bufferWriter.WrittenCount); ReadOnlyMemory linear = payload.ToArray(); Assert.True(linear.Span.SequenceEqual(bufferWriter.WrittenSpan), "payload match"); diff --git a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs index 7e90a92c9926..06201b15c8b2 100644 --- a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs +++ b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; -using System.Linq; namespace Microsoft.AspNetCore.OutputCaching; @@ -36,26 +35,12 @@ public interface IOutputCacheStore /// The amount of time the entry will be kept in the cache before expiring, relative to now. /// Indicates that the operation should be cancelled. ValueTask SetAsync(string key, byte[] value, string[]? tags, TimeSpan validFor, CancellationToken cancellationToken); - - /// - /// Stores the given response in the response cache. - /// - /// The cache key to store the response under. - /// The response cache entry to store; this value is only defined for the duration of the method, and should not be stored without making a copy. - /// The tags associated with the cache entry to store. - /// The amount of time the entry will be kept in the cache before expiring, relative to now. - /// Indicates that the operation should be cancelled. - ValueTask SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken) - { - // compatibility implementation using the original API - return SetAsync(key, value.ToArray(), tags.IsEmpty ? null : tags.ToArray(), validFor, cancellationToken); - } } /// /// Represents a store for cached responses that uses a as the target. /// -public interface IOutputCacheBufferWriterStore : IOutputCacheStore +public interface IOutputCacheBufferStore : IOutputCacheStore { /// /// Gets the cached response for the given key, if it exists. @@ -65,5 +50,15 @@ public interface IOutputCacheBufferWriterStore : IOutputCacheStore /// The location to which the value should be written. /// Indicates that the operation should be cancelled. /// True if the response cache entry if it exists; otherwise False. - ValueTask GetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken); + ValueTask TryGetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken); + + /// + /// Stores the given response in the response cache. + /// + /// The cache key to store the response under. + /// The response cache entry to store; this value is only defined for the duration of the method, and should not be stored without making a copy. + /// The tags associated with the cache entry to store. + /// The amount of time the entry will be kept in the cache before expiring, relative to now. + /// Indicates that the operation should be cancelled. + ValueTask SetAsync(string key, ReadOnlySequence value, ReadOnlyMemory tags, TimeSpan validFor, CancellationToken cancellationToken); } diff --git a/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs b/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs index 13be1acdda48..963c6fbef24d 100644 --- a/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs +++ b/src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs @@ -84,7 +84,16 @@ public static async ValueTask StoreAsync(string key, OutputCacheEntry value, Tim segment = bufferStream.ToArray(); } - await store.SetAsync(key, new ReadOnlySequence(segment.Array!, segment.Offset, segment.Count), value.Tags, duration, cancellationToken); + var payload = new ReadOnlySequence(segment.Array!, segment.Offset, segment.Count); + if (store is IOutputCacheBufferStore bufferStore) + { + await bufferStore.SetAsync(key, payload, value.Tags, duration, cancellationToken); + } + else + { + // legacy API/in-proc: create an isolated right-sized byte[] for the payload + await store.SetAsync(key, payload.ToArray(), value.Tags, duration, cancellationToken); + } } // Format: diff --git a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt index 38795f5caed6..9d6a001aa038 100644 --- a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable -Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferWriterStore -Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferWriterStore.GetAsync(string! key, System.Buffers.IBufferWriter! destination, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask -Microsoft.AspNetCore.OutputCaching.IOutputCacheStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.ReadOnlyMemory tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.ReadOnlyMemory tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore.TryGetAsync(string! key, System.Buffers.IBufferWriter! destination, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.get -> string![]? Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.init -> void From fb7b1e6e7e3e6bb88f77c4b412ef57703a64a4d7 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 16:01:07 +0100 Subject: [PATCH 22/30] fix failing test (from nit fixes) --- .../src/StackExchangeRedisCacheServiceCollectionExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs index b90aec80eafc..d6fca729bf20 100644 --- a/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs +++ b/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs @@ -50,7 +50,8 @@ public static IServiceCollection AddStackExchangeRedisOutputCache(this IServiceC services.AddOptions(); services.Configure(setupAction); - services.TryAddSingleton(); + // replace here (Add vs TryAdd) is intentional and part of test conditions + services.AddSingleton(); return services; } From 8dd9a210d50efdb90e3e31fe82c8cdb7c65c15c8 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 16:10:36 +0100 Subject: [PATCH 23/30] use CTS to interrupt server GC --- .../StackExchangeRedis/src/RedisOutputCacheStore.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index c8ff244d162a..3a56c6ca237b 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -29,6 +29,7 @@ internal class RedisOutputCacheStore : IOutputCacheStore, IOutputCacheBufferStor private readonly RedisKey _tagMasterKey; private readonly RedisKey[] _tagMasterKeyArray; // for use with Lua if needed (to avoid array allocs) private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(initialCount: 1, maxCount: 1); + private readonly CancellationTokenSource _disposalCancellation = new(); private bool _disposed; private volatile IDatabase? _cache; @@ -102,9 +103,13 @@ private async Task RunGarbageCollectionLoopAsync() { if (GarbageCollectionEnabled) { - await ExecuteGarbageCollectionAsync(GetExpirationTimestamp(TimeSpan.Zero)).ConfigureAwait(false); + await ExecuteGarbageCollectionAsync(GetExpirationTimestamp(TimeSpan.Zero), _disposalCancellation.Token).ConfigureAwait(false); } } + catch (OperationCanceledException) when (_disposed) + { + // fine, service exiting + } catch (Exception ex) { // this sweep failed; log it @@ -359,8 +364,8 @@ public void Dispose() { return; } - _disposed = true; + _disposalCancellation.Cancel(); ReleaseConnection(Interlocked.Exchange(ref _cache, null)); } From f79e65dfd47383abe99a54dec4ea322650130e0e Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 16:13:30 +0100 Subject: [PATCH 24/30] switch from magic number in `long` result to `long?` --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 3a56c6ca237b..ebed99a3adb8 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -124,7 +124,7 @@ private async Task RunGarbageCollectionLoopAsync() } } - internal async ValueTask ExecuteGarbageCollectionAsync(long keepValuesGreaterThan, CancellationToken cancellationToken = default) + internal async ValueTask ExecuteGarbageCollectionAsync(long keepValuesGreaterThan, CancellationToken cancellationToken = default) { var cache = await ConnectAsync(CancellationToken.None).ConfigureAwait(false); @@ -133,7 +133,7 @@ internal async ValueTask ExecuteGarbageCollectionAsync(long keepValuesGrea // value is purely placeholder; it is the existence that matters if (!await cache.StringSetAsync(gcKey, DateTime.UtcNow.ToString(CultureInfo.InvariantCulture), gcLifetime, when: When.NotExists).ConfigureAwait(false)) { - return -1; // signal competition + return null; // competition from another node; not even "nothing" } try { From a74ec4357171dc24d121b49f980d1f4692bbea9a Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 16:20:56 +0100 Subject: [PATCH 25/30] delete copy/pasta --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index ebed99a3adb8..81711304fada 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -315,7 +315,6 @@ private ValueTask ConnectAsync(CancellationToken token = default) var cache = _cache; if (cache is not null) { - Debug.Assert(_cache is not null); return new(cache); } return ConnectSlowAsync(token); From e5fa5012e61e8b3fe5c039eb9b7f985f1c970336 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 19 Jun 2023 16:52:02 +0100 Subject: [PATCH 26/30] fix DEBUG/RELEASE snafu --- .../test/OutputCache/OutputCacheGetSetTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 4930e8731e74..64e854502207 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -51,7 +51,7 @@ private async ValueTask Cache() return _cache; } #else - private ValueTask Cache() => new(_cache); // avoid CS1998 - no "await" + private ValueTask Cache() => new(_cache); // avoid CS1998 - no "await" #endif [Fact(Skip = SkipReason)] From 757441c54b3b82ffab5de56266c6e0d39f1d71f6 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 20 Jun 2023 15:53:42 +0100 Subject: [PATCH 27/30] include output cache middleware in caching proj --- src/Caching/Caching.slnf | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Caching/Caching.slnf b/src/Caching/Caching.slnf index ebc2330a777e..dcecdb8a91c7 100644 --- a/src/Caching/Caching.slnf +++ b/src/Caching/Caching.slnf @@ -5,7 +5,8 @@ "src\\Caching\\SqlServer\\src\\Microsoft.Extensions.Caching.SqlServer.csproj", "src\\Caching\\SqlServer\\test\\Microsoft.Extensions.Caching.SqlServer.Tests.csproj", "src\\Caching\\StackExchangeRedis\\src\\Microsoft.Extensions.Caching.StackExchangeRedis.csproj", - "src\\Caching\\StackExchangeRedis\\test\\Microsoft.Extensions.Caching.StackExchangeRedis.Tests.csproj" + "src\\Caching\\StackExchangeRedis\\test\\Microsoft.Extensions.Caching.StackExchangeRedis.Tests.csproj", + "src\\Middleware\\OutputCaching\\src\\Microsoft.AspNetCore.OutputCaching.csproj" ] } -} +} \ No newline at end of file From 2e1d87dd7e6e470154d72eb6f6c13a007a09c872 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 20 Jun 2023 16:47:43 +0100 Subject: [PATCH 28/30] API feedback; IBufferWriter -> PipeWriter --- .../src/RedisOutputCacheStore.cs | 4 +- .../OutputCache/OutputCacheGetSetTests.cs | 51 +++++++++++++++---- .../OutputCaching/src/IOutputCacheStore.cs | 3 +- .../OutputCaching/src/PublicAPI.Unshipped.txt | 2 +- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 81711304fada..96f62b8c7b4e 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Globalization; +using System.IO.Pipelines; using System.Net.Sockets; using System.Text; using System.Threading; @@ -204,7 +205,7 @@ private RedisKey GetTagKey(string tag) } } - async ValueTask IOutputCacheBufferStore.TryGetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken) + async ValueTask IOutputCacheBufferStore.TryGetAsync(string key, PipeWriter destination, CancellationToken cancellationToken) { ArgumentNullThrowHelper.ThrowIfNull(key); ArgumentNullThrowHelper.ThrowIfNull(destination); @@ -222,6 +223,7 @@ async ValueTask IOutputCacheBufferStore.TryGetAsync(string key, IBufferWri } destination.Write(result.Span); + await destination.FlushAsync(cancellationToken).ConfigureAwait(false); return true; } catch (Exception ex) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 64e854502207..7643d515c724 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -5,12 +5,13 @@ using System; using System.Buffers; +using System.IO.Pipelines; using System.Runtime.CompilerServices; using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.OutputCaching; -using Microsoft.Diagnostics.Runtime.Interop; +using Pipelines.Sockets.Unofficial.Buffers; using StackExchange.Redis; using Xunit; using Xunit.Abstractions; @@ -158,9 +159,13 @@ public async Task GetMissingKeyReturnsFalse_BufferWriter() // "true" result chec var cache = Assert.IsAssignableFrom(await Cache().ConfigureAwait(false)); var key = Guid.NewGuid().ToString(); - var bufferWriter = new ArrayBufferWriter(); - Assert.False(await cache.TryGetAsync(key, bufferWriter, CancellationToken.None)); - Assert.Equal(0, bufferWriter.WrittenCount); + var pipe = new Pipe(); + Assert.False(await cache.TryGetAsync(key, pipe.Writer, CancellationToken.None)); + pipe.Writer.Complete(); + var read = await pipe.Reader.ReadAsync(); + Assert.True(read.IsCompleted); + Assert.True(read.Buffer.IsEmpty); + pipe.Reader.AdvanceTo(read.Buffer.End); } [Fact(Skip = SkipReason)] @@ -311,11 +316,39 @@ public async Task MultiSegmentWriteWorksAsExpected_BufferWriter() var key = Guid.NewGuid().ToString(); await cache.SetAsync(key, payload, default, TimeSpan.FromSeconds(30), CancellationToken.None); - var bufferWriter = new ArrayBufferWriter(); - Assert.True(await cache.TryGetAsync(key, bufferWriter, CancellationToken.None)); - Assert.Equal(1290, bufferWriter.WrittenCount); - ReadOnlyMemory linear = payload.ToArray(); - Assert.True(linear.Span.SequenceEqual(bufferWriter.WrittenSpan), "payload match"); + var pipe = new Pipe(); + Assert.True(await cache.TryGetAsync(key, pipe.Writer, CancellationToken.None)); + pipe.Writer.Complete(); + var read = await pipe.Reader.ReadAsync(); + Assert.True(read.IsCompleted); + Assert.Equal(1290, read.Buffer.Length); + + using (Linearize(payload, out var linearPayload)) + using (Linearize(read.Buffer, out var linearRead)) + { + Assert.True(linearPayload.Span.SequenceEqual(linearRead.Span), "payload match"); + } + pipe.Reader.AdvanceTo(read.Buffer.End); + + static IMemoryOwner Linearize(ReadOnlySequence payload, out ReadOnlyMemory linear) + { + if (payload.IsEmpty) + { + linear = default; + return null; + } + if (payload.IsSingleSegment) + { + linear = payload.First; + return null; + } + var len = checked((int)payload.Length); + var lease = MemoryPool.Shared.Rent(len); + var memory = lease.Memory.Slice(0, len); + payload.CopyTo(memory.Span); + linear = memory; + return lease; + } } [Fact(Skip = SkipReason)] diff --git a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs index 06201b15c8b2..9b9e0738cb4b 100644 --- a/src/Middleware/OutputCaching/src/IOutputCacheStore.cs +++ b/src/Middleware/OutputCaching/src/IOutputCacheStore.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; +using System.IO.Pipelines; namespace Microsoft.AspNetCore.OutputCaching; @@ -50,7 +51,7 @@ public interface IOutputCacheBufferStore : IOutputCacheStore /// The location to which the value should be written. /// Indicates that the operation should be cancelled. /// True if the response cache entry if it exists; otherwise False. - ValueTask TryGetAsync(string key, IBufferWriter destination, CancellationToken cancellationToken); + ValueTask TryGetAsync(string key, PipeWriter destination, CancellationToken cancellationToken); /// /// Stores the given response in the response cache. diff --git a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt index 9d6a001aa038..828a06a00180 100644 --- a/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/OutputCaching/src/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore.SetAsync(string! key, System.Buffers.ReadOnlySequence value, System.ReadOnlyMemory tags, System.TimeSpan validFor, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask -Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore.TryGetAsync(string! key, System.Buffers.IBufferWriter! destination, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +Microsoft.AspNetCore.OutputCaching.IOutputCacheBufferStore.TryGetAsync(string! key, System.IO.Pipelines.PipeWriter! destination, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.get -> string![]? Microsoft.AspNetCore.OutputCaching.OutputCacheAttribute.Tags.init -> void From 56baf49a9530eaac6539e689769932a5b3728dd4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 20 Jun 2023 16:53:15 +0100 Subject: [PATCH 29/30] fix magic numbers test snafu --- .../test/OutputCache/OutputCacheGetSetTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs index 7643d515c724..088d7c76822d 100644 --- a/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs +++ b/src/Caching/StackExchangeRedis/test/OutputCache/OutputCacheGetSetTests.cs @@ -359,7 +359,7 @@ public async Task GarbageCollectionDoesNotRunWhenGCKeyHeld() await _fixture.Database.StringSetAsync("TestPrefix__MSOCTGC", "dummy", TimeSpan.FromMinutes(1)); try { - Assert.Equal(-1, await impl.ExecuteGarbageCollectionAsync(42)); + Assert.Null(await impl.ExecuteGarbageCollectionAsync(42)); } finally { From b1feb8f5d05c4cfb283ff40aeeeb7883e9891e19 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 20 Jun 2023 16:56:24 +0100 Subject: [PATCH 30/30] clarify future PipeWriter usage --- src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs index 96f62b8c7b4e..5e4905c0cc05 100644 --- a/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs +++ b/src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs @@ -222,6 +222,8 @@ async ValueTask IOutputCacheBufferStore.TryGetAsync(string key, PipeWriter return false; } + // future implementation will pass PipeWriter all the way down through redis, + // to allow end-to-end back-pressure; new SE.Redis API required destination.Write(result.Span); await destination.FlushAsync(cancellationToken).ConfigureAwait(false); return true;