Skip to content

Commit 792f1f2

Browse files
mgravellmgravell
authored
Redis distributed/output cache: include scenario in connection metadata (#49291)
* - advertise scenario in the "CLIENT SETINFO" metadata - rev se.redis library version (this API is new) * Centralise exception handling of output cache read/write (much more important now that we have out-of-process implementations) * include lib hint in test config * rerun baseline generator * use RedisChannel.Literal on pub/sub; for pub, this makes no difference; for sub, this means "single channel, not a glob pattern" * supply logger param in tests * use [LoggerMessage] consistently * include SignalR in the redis client library name * don't use nameof for LoggerExtensions
1 parent b4d0e85 commit 792f1f2

File tree

12 files changed

+91
-46
lines changed

12 files changed

+91
-46
lines changed

‎eng/Versions.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@
315315
<SeleniumWebDriverVersion>4.10.0</SeleniumWebDriverVersion>
316316
<SerilogExtensionsLoggingVersion>1.4.0</SerilogExtensionsLoggingVersion>
317317
<SerilogSinksFileVersion>4.0.0</SerilogSinksFileVersion>
318-
<StackExchangeRedisVersion>2.6.90</StackExchangeRedisVersion>
318+
<StackExchangeRedisVersion>2.6.122</StackExchangeRedisVersion>
319319
<SystemReactiveLinqVersion>5.0.0</SystemReactiveLinqVersion>
320320
<SwashbuckleAspNetCoreVersion>6.4.0</SwashbuckleAspNetCoreVersion>
321321
<XunitAbstractionsVersion>2.0.3</XunitAbstractionsVersion>

‎src/Caching/StackExchangeRedis/src/RedisCache.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ private ValueTask<IDatabase> ConnectAsync(CancellationToken token = default)
296296
}
297297
return ConnectSlowAsync(token);
298298
}
299+
299300
private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)
300301
{
301302
await _connectionLock.WaitAsync(token).ConfigureAwait(false);
@@ -307,14 +308,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)
307308
IConnectionMultiplexer connection;
308309
if (_options.ConnectionMultiplexerFactory is null)
309310
{
310-
if (_options.ConfigurationOptions is not null)
311-
{
312-
connection = await ConnectionMultiplexer.ConnectAsync(_options.ConfigurationOptions).ConfigureAwait(false);
313-
}
314-
else
315-
{
316-
connection = await ConnectionMultiplexer.ConnectAsync(_options.Configuration!).ConfigureAwait(false);
317-
}
311+
connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net DC")).ConfigureAwait(false);
318312
}
319313
else
320314
{

‎src/Caching/StackExchangeRedis/src/RedisCacheOptions.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Threading.Tasks;
66
using Microsoft.Extensions.Options;
77
using StackExchange.Redis;
8+
using StackExchange.Redis.Configuration;
89
using StackExchange.Redis.Profiling;
910

1011
namespace Microsoft.Extensions.Caching.StackExchangeRedis;
@@ -57,4 +58,19 @@ static bool GetDefaultValue() =>
5758
}
5859
set => _useForceReconnect = value;
5960
}
61+
62+
internal ConfigurationOptions GetConfiguredOptions(string libSuffix)
63+
{
64+
var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!);
65+
66+
// we don't want an initially unavailable server to prevent DI creating the service itself
67+
options.AbortOnConnectFail = false;
68+
69+
if (!string.IsNullOrWhiteSpace(libSuffix))
70+
{
71+
var provider = DefaultOptionsProvider.GetProvider(options.EndPoints);
72+
options.LibraryName = $"{provider.LibraryName} {libSuffix}";
73+
}
74+
return options;
75+
}
6076
}

‎src/Caching/StackExchangeRedis/src/RedisOutputCacheStore.cs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -334,14 +334,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)
334334
IConnectionMultiplexer connection;
335335
if (_options.ConnectionMultiplexerFactory is null)
336336
{
337-
if (_options.ConfigurationOptions is not null)
338-
{
339-
connection = await ConnectionMultiplexer.ConnectAsync(_options.ConfigurationOptions).ConfigureAwait(false);
340-
}
341-
else
342-
{
343-
connection = await ConnectionMultiplexer.ConnectAsync(_options.Configuration!).ConfigureAwait(false);
344-
}
337+
connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net OC")).ConfigureAwait(false);
345338
}
346339
else
347340
{

‎src/Caching/StackExchangeRedis/test/OutputCache/RedisConnectionFixture.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ public class RedisConnectionFixture : IDisposable
1414
private readonly ConnectionMultiplexer _muxer;
1515
public RedisConnectionFixture()
1616
{
17-
_muxer = ConnectionMultiplexer.Connect("127.0.0.1:6379");
17+
var options = new RedisCacheOptions
18+
{
19+
Configuration = "127.0.0.1:6379", // TODO: CI test config here
20+
}.GetConfiguredOptions("CI test");
21+
_muxer = ConnectionMultiplexer.Connect(options);
1822
}
1923

2024
public IDatabase Database => _muxer.GetDatabase();

‎src/Middleware/OutputCaching/src/LoggerExtensions.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,10 @@ internal static partial class LoggerExtensions
4949
EventName = "ExpirationExpiresExceeded")]
5050
internal static partial void ExpirationExpiresExceeded(this ILogger logger, DateTimeOffset responseTime);
5151

52+
[LoggerMessage(12, LogLevel.Error, "Unable to query output cache.", EventName = "UnableToQueryOutputCache")]
53+
internal static partial void UnableToQueryOutputCache(this ILogger logger, Exception exception);
54+
55+
[LoggerMessage(13, LogLevel.Error, "Unable to write to output-cache.", EventName = "UnableToWriteToOutputCache")]
56+
internal static partial void UnableToWriteToOutputCache(this ILogger logger, Exception exception);
57+
5258
}

‎src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Linq;
66
using System.Text;
77
using Microsoft.AspNetCore.OutputCaching.Serialization;
8+
using Microsoft.Extensions.Logging;
89

910
namespace Microsoft.AspNetCore.OutputCaching;
1011
/// <summary>
@@ -52,7 +53,7 @@ internal static class OutputCacheEntryFormatter
5253
return outputCacheEntry;
5354
}
5455

55-
public static async ValueTask StoreAsync(string key, OutputCacheEntry value, TimeSpan duration, IOutputCacheStore store, CancellationToken cancellationToken)
56+
public static async ValueTask StoreAsync(string key, OutputCacheEntry value, TimeSpan duration, IOutputCacheStore store, ILogger logger, CancellationToken cancellationToken)
5657
{
5758
ArgumentNullException.ThrowIfNull(value);
5859
ArgumentNullException.ThrowIfNull(value.Body);
@@ -85,14 +86,25 @@ public static async ValueTask StoreAsync(string key, OutputCacheEntry value, Tim
8586
}
8687

8788
var payload = new ReadOnlySequence<byte>(segment.Array!, segment.Offset, segment.Count);
88-
if (store is IOutputCacheBufferStore bufferStore)
89+
try
8990
{
90-
await bufferStore.SetAsync(key, payload, value.Tags, duration, cancellationToken);
91+
if (store is IOutputCacheBufferStore bufferStore)
92+
{
93+
await bufferStore.SetAsync(key, payload, value.Tags, duration, cancellationToken);
94+
}
95+
else
96+
{
97+
// legacy API/in-proc: create an isolated right-sized byte[] for the payload
98+
await store.SetAsync(key, payload.ToArray(), value.Tags, duration, cancellationToken);
99+
}
100+
}
101+
catch (OperationCanceledException)
102+
{
103+
// don't report as failure
91104
}
92-
else
105+
catch (Exception ex)
93106
{
94-
// legacy API/in-proc: create an isolated right-sized byte[] for the payload
95-
await store.SetAsync(key, payload.ToArray(), value.Tags, duration, cancellationToken);
107+
logger.UnableToWriteToOutputCache(ex);
96108
}
97109
}
98110

‎src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,23 @@ internal async Task<bool> TryServeFromCacheAsync(OutputCacheContext cacheContext
336336
// Locking cache lookups by default
337337
// TODO: should it be part of the cache implementations or can we assume all caches would benefit from it?
338338
// It makes sense for caches that use IO (disk, network) or need to deserialize the state but could also be a global option
339+
OutputCacheEntry? cacheEntry;
340+
try
341+
{
342+
cacheEntry = await _outputCacheEntryDispatcher.ScheduleAsync(cacheContext.CacheKey, (Store: _store, CacheContext: cacheContext), static async (key, state) => await OutputCacheEntryFormatter.GetAsync(key, state.Store, state.CacheContext.HttpContext.RequestAborted));
343+
}
344+
catch (OperationCanceledException)
345+
{
346+
// don't report as failure
347+
cacheEntry = null;
348+
}
349+
catch (Exception ex)
350+
{
351+
_logger.UnableToQueryOutputCache(ex);
352+
cacheEntry = null;
353+
}
339354

340-
var cacheEntry = await _outputCacheEntryDispatcher.ScheduleAsync(cacheContext.CacheKey, (Store: _store, CacheContext: cacheContext), static async (key, state) => await OutputCacheEntryFormatter.GetAsync(key, state.Store, state.CacheContext.HttpContext.RequestAborted));
341-
342-
if (await TryServeCachedResponseAsync(cacheContext, cacheEntry, policies))
355+
if (cacheEntry is not null && await TryServeCachedResponseAsync(cacheContext, cacheEntry, policies))
343356
{
344357
return true;
345358
}
@@ -440,7 +453,7 @@ internal async ValueTask FinalizeCacheBodyAsync(OutputCacheContext context)
440453
else
441454
{
442455
_logger.ResponseCached();
443-
await OutputCacheEntryFormatter.StoreAsync(context.CacheKey, context.CachedResponse, context.CachedResponseValidFor, _store, context.HttpContext.RequestAborted);
456+
await OutputCacheEntryFormatter.StoreAsync(context.CacheKey, context.CachedResponse, context.CachedResponseValidFor, _store, _logger, context.HttpContext.RequestAborted);
444457
}
445458
}
446459
else

‎src/Middleware/OutputCaching/test/OutputCacheEntryFormatterTests.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using Microsoft.AspNetCore.Http;
5-
using Microsoft.Extensions.Primitives;
5+
using Microsoft.Extensions.Logging.Abstractions;
66
using Microsoft.Net.Http.Headers;
77

88
namespace Microsoft.AspNetCore.OutputCaching.Tests;
@@ -23,7 +23,7 @@ public async Task StoreAndGet_StoresEmptyValues()
2323
Tags = Array.Empty<string>()
2424
};
2525

26-
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, default);
26+
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, NullLogger.Instance, default);
2727

2828
var result = await OutputCacheEntryFormatter.GetAsync(key, store, default);
2929

@@ -47,7 +47,7 @@ public async Task StoreAndGet_StoresAllValues()
4747
Tags = new[] { "tag", "タグ" }
4848
};
4949

50-
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, default);
50+
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, NullLogger.Instance, default);
5151

5252
var result = await OutputCacheEntryFormatter.GetAsync(key, store, default);
5353

@@ -66,7 +66,7 @@ public async Task StoreAndGet_StoresNullTags()
6666
Tags = new[] { null, null, "", "tag" }
6767
};
6868

69-
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, default);
69+
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, NullLogger.Instance, default);
7070

7171
var result = await OutputCacheEntryFormatter.GetAsync(key, store, default);
7272

@@ -89,7 +89,7 @@ public async Task StoreAndGet_StoresNullHeaders()
8989
Tags = Array.Empty<string>()
9090
};
9191

92-
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, default);
92+
await OutputCacheEntryFormatter.StoreAsync(key, entry, TimeSpan.Zero, store, NullLogger.Instance, default);
9393

9494
var result = await OutputCacheEntryFormatter.GetAsync(key, store, default);
9595

‎src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Globalization;
54
using System.Text;
6-
using System.Text.Unicode;
75
using Microsoft.AspNetCore.Http;
86
using Microsoft.AspNetCore.Http.Features;
97
using Microsoft.AspNetCore.OutputCaching.Memory;
108
using Microsoft.AspNetCore.Testing;
119
using Microsoft.Extensions.Caching.Memory;
10+
using Microsoft.Extensions.Logging.Abstractions;
1211
using Microsoft.Extensions.Logging.Testing;
1312
using Microsoft.Extensions.Primitives;
1413
using Microsoft.Net.Http.Headers;
@@ -71,6 +70,7 @@ await OutputCacheEntryFormatter.StoreAsync(
7170
},
7271
TimeSpan.Zero,
7372
cache,
73+
NullLogger.Instance,
7474
default);
7575

7676
Assert.True(await middleware.TryServeFromCacheAsync(context, policies));
@@ -102,6 +102,7 @@ await OutputCacheEntryFormatter.StoreAsync(context.CacheKey,
102102
},
103103
TimeSpan.Zero,
104104
cache,
105+
NullLogger.Instance,
105106
default);
106107

107108
Assert.True(await middleware.TryServeFromCacheAsync(context, policies));
@@ -130,6 +131,7 @@ await OutputCacheEntryFormatter.StoreAsync("BaseKey",
130131
},
131132
TimeSpan.Zero,
132133
cache,
134+
NullLogger.Instance,
133135
default);
134136

135137
Assert.True(await middleware.TryServeFromCacheAsync(context, policies));

0 commit comments

Comments
 (0)