Caching: SE.Redis update and fix naming inconsistency #54239
Conversation
- ensure sync/async connect use consistent path
mgravell
requested review from
a team,
captainsafia,
halter73 and
wtgodbe
as code owners
ghost
added
the
area-infrastructure
- use new AddLibraryNameSuffix API (2.7.27) - do not use ConfigurationOptions.Clone() - impacts key rotation - attach suffix even if ConnectionMultiplexerFactory used (Aspire) - revert changes to Baseline.Designer.props
|
SignalR (test fail) should just be some |
| internal ConfigurationOptions GetConfiguredOptions(string libSuffix) | ||
| internal ConfigurationOptions GetConfiguredOptions() | ||
| { | ||
| var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!); |
There was a problem hiding this comment.
I know you said it breaks something to call Clone. But we call Clone because this ConfigurationOptions can be used elsewhere and we modify the settings which could cause issues with the other uses of the same options reference.
There was a problem hiding this comment.
@BrennanConroy the thing potentially it breaks can fundamentally cause access to the servers to be lost (key rotation for managed identity); let me review the azure redis documentation to see if we can find a compromise here
There was a problem hiding this comment.
@BrennanConroy this Clone() is a recent addition; I'm not concerned by sharing the connect-fail change here, so IMO we're restoring the correct behaviour here;
...eware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheStore.Log.cs
Outdated
Show resolved
Hide resolved
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/aspnetcore/actions/runs/8189114024 |
|
@mgravell backporting to release/8.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: - SE.Redis lib update - ensure sync/async connect use consistent path
Using index info to reconstruct a base tree...
M eng/Baseline.Designer.props
M eng/Versions.props
M src/Caching/StackExchangeRedis/src/RedisCache.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Caching/StackExchangeRedis/src/RedisCache.cs
Auto-merging eng/Versions.props
Auto-merging eng/Baseline.Designer.props
CONFLICT (content): Merge conflict in eng/Baseline.Designer.props
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 - SE.Redis lib update - ensure sync/async connect use consistent path
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@mgravell an error occurred while backporting to release/8.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
* - rev to SE.Redis 2.7.27 - use new AddLibraryNameSuffix API (2.7.27) - do not use ConfigurationOptions.Clone() - impacts key rotation - attach suffix even if ConnectionMultiplexerFactory used (Aspire)
- Fix Redis output and distributed caching tests in response to new tracing events emitted from dotnet/aspnetcore#54239
Resolve redis cache naming inconsistencies
The cache intent is included in the connection metadata ... or should be.
manual test evidence (from running tests with a pause in):