From f4f12d7c913ea3db1923b17f8df0c19b3e140140 Mon Sep 17 00:00:00 2001 From: Chris Philips Date: Wed, 6 Apr 2022 11:08:16 -0700 Subject: [PATCH] bugfix: stop creating/disposing connection multiplexer (#30) * bugfix: stop creating/disposing connection multiplexer according to docs it should be a very rare event that the connection multiplexer is disposed * bugfix(test): test error * Empty commit to make codacy send new result --- src/EntityDb.Redis/Sessions/IRedisSession.cs | 2 +- src/EntityDb.Redis/Sessions/RedisSession.cs | 102 ++++++++++++++---- .../Snapshots/RedisSnapshotRepository.cs | 4 +- .../RedisSnapshotRepositoryFactory.cs | 34 +++++- .../Sessions/RedisSessionTests.cs | 2 +- 5 files changed, 116 insertions(+), 28 deletions(-) diff --git a/src/EntityDb.Redis/Sessions/IRedisSession.cs b/src/EntityDb.Redis/Sessions/IRedisSession.cs index 2fce4480..4a8e0de9 100644 --- a/src/EntityDb.Redis/Sessions/IRedisSession.cs +++ b/src/EntityDb.Redis/Sessions/IRedisSession.cs @@ -9,5 +9,5 @@ internal interface IRedisSession : IDisposableResource { Task Insert(RedisKey redisKey, RedisValue redisValue); Task Find(RedisKey redisKey); - Task Delete(IEnumerable redisKeys); + Task Delete(RedisKey[] redisKeys); } diff --git a/src/EntityDb.Redis/Sessions/RedisSession.cs b/src/EntityDb.Redis/Sessions/RedisSession.cs index 87f8ab92..1881a19f 100644 --- a/src/EntityDb.Redis/Sessions/RedisSession.cs +++ b/src/EntityDb.Redis/Sessions/RedisSession.cs @@ -1,14 +1,23 @@ using EntityDb.Common.Disposables; using EntityDb.Common.Exceptions; using EntityDb.Common.Snapshots; +using EntityDb.Common.Transactions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using StackExchange.Redis; +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; namespace EntityDb.Redis.Sessions; -internal sealed record RedisSession(IConnectionMultiplexer ConnectionMultiplexer, SnapshotSessionOptions SnapshotSessionOptions) : DisposableResourceBaseRecord, IRedisSession +internal sealed record RedisSession +( + ILogger Logger, + IDatabase Database, + SnapshotSessionOptions SnapshotSessionOptions +) : DisposableResourceBaseRecord, IRedisSession { private CommandFlags GetCommandFlags() { @@ -34,29 +43,71 @@ public async Task Insert(RedisKey redisKey, RedisValue redisValue) { AssertNotReadOnly(); - ConnectionMultiplexer.GetDatabase(); + Logger + .LogInformation + ( + "Started Running Redis Insert on `{DatabaseIndex}.{RedisKey}`", + Database.Database, + redisKey.ToString() + ); + + var redisTransaction = Database.CreateTransaction(); - var redisTransaction = ConnectionMultiplexer.GetDatabase().CreateTransaction(); - var insertedTask = redisTransaction.StringSetAsync(redisKey, redisValue); await redisTransaction.ExecuteAsync(GetCommandFlags()); - return await insertedTask; + var inserted = await insertedTask; + + Logger + .LogInformation + ( + "Finished Running Redis Insert on `{DatabaseIndex}.{RedisKey}`\n\nInserted: {Inserted}", + Database.Database, + redisKey.ToString(), + inserted + ); + + return inserted; } public async Task Find(RedisKey redisKey) { - var redisDatabase = ConnectionMultiplexer.GetDatabase(); - - return await redisDatabase.StringGetAsync(redisKey, GetCommandFlags()); + Logger + .LogInformation + ( + "Started Running Redis Query on `{DatabaseIndex}.{RedisKey}`", + Database.Database, + redisKey.ToString() + ); + + var redisValue = await Database.StringGetAsync(redisKey, GetCommandFlags()); + + Logger + .LogInformation + ( + "Finished Running Redis Query on `{DatabaseIndex}.{RedisKey}`\n\nHas Value: {HasValue}", + Database.Database, + redisKey.ToString(), + redisValue.HasValue + ); + + return redisValue; } - public async Task Delete(IEnumerable redisKeys) + public async Task Delete(RedisKey[] redisKeys) { AssertNotReadOnly(); - var redisTransaction = ConnectionMultiplexer.GetDatabase().CreateTransaction(); + Logger + .LogInformation + ( + "Started Running Redis Delete on `{DatabaseIndex}` for {NumberOfKeys} Key(s)", + Database.Database, + redisKeys.Length + ); + + var redisTransaction = Database.CreateTransaction(); var deleteSnapshotTasks = redisKeys .Select(key => redisTransaction.KeyDeleteAsync(key)) @@ -65,19 +116,28 @@ public async Task Delete(IEnumerable redisKeys) await redisTransaction.ExecuteAsync(GetCommandFlags()); await Task.WhenAll(deleteSnapshotTasks); - - return deleteSnapshotTasks.All(task => task.Result); - } - - public override void Dispose() - { + var allDeleted = deleteSnapshotTasks.All(task => task.Result); + + Logger + .LogInformation + ( + "Finished Running Redis Delete on `{DatabaseIndex}` for {NumberOfKeys} Key(s)\n\nAll Deleted: {AllDeleted}", + Database.Database, + redisKeys.Length, + allDeleted + ); + + return allDeleted; } - - public override ValueTask DisposeAsync() + + public static IRedisSession Create + ( + IServiceProvider serviceProvider, + IDatabase database, + SnapshotSessionOptions snapshotSessionOptions + ) { - ConnectionMultiplexer.Dispose(); - - return ValueTask.CompletedTask; + return ActivatorUtilities.CreateInstance(serviceProvider, database, snapshotSessionOptions); } } diff --git a/src/EntityDb.Redis/Snapshots/RedisSnapshotRepository.cs b/src/EntityDb.Redis/Snapshots/RedisSnapshotRepository.cs index c3660b97..5e8b0f13 100644 --- a/src/EntityDb.Redis/Snapshots/RedisSnapshotRepository.cs +++ b/src/EntityDb.Redis/Snapshots/RedisSnapshotRepository.cs @@ -32,7 +32,7 @@ IRedisSession redisSession private RedisKey GetSnapshotKey(Id snapshotId) { - return $"{_keyNamespace}#{snapshotId}"; + return $"{_keyNamespace}#{snapshotId.Value}"; } public async Task PutSnapshot(Id snapshotId, TSnapshot snapshot, CancellationToken cancellationToken = default) @@ -61,7 +61,7 @@ public async Task PutSnapshot(Id snapshotId, TSnapshot snapshot, Cancellat public async Task DeleteSnapshots(Id[] snapshotIds, CancellationToken cancellationToken = default) { - var snapshotKeys = snapshotIds.Select(GetSnapshotKey); + var snapshotKeys = snapshotIds.Select(GetSnapshotKey).ToArray(); return await _redisSession.Delete(snapshotKeys).WaitAsync(cancellationToken); } diff --git a/src/EntityDb.Redis/Snapshots/RedisSnapshotRepositoryFactory.cs b/src/EntityDb.Redis/Snapshots/RedisSnapshotRepositoryFactory.cs index 64a97e92..e9c788f4 100644 --- a/src/EntityDb.Redis/Snapshots/RedisSnapshotRepositoryFactory.cs +++ b/src/EntityDb.Redis/Snapshots/RedisSnapshotRepositoryFactory.cs @@ -21,6 +21,9 @@ internal class RedisSnapshotRepositoryFactory : DisposableResourceBas private readonly IEnvelopeService _envelopeService; private readonly string _connectionString; private readonly string _keyNamespace; + private readonly SemaphoreSlim _connectionSemaphore = new(1); + + private IConnectionMultiplexer? _connectionMultiplexer; public RedisSnapshotRepositoryFactory ( @@ -39,13 +42,31 @@ string keyNamespace _keyNamespace = keyNamespace; } - private async Task CreateSession(SnapshotSessionOptions snapshotSessionOptions, CancellationToken cancellationToken) + private async Task OpenConnectionMultiplexer(CancellationToken cancellationToken) { + await _connectionSemaphore.WaitAsync(cancellationToken); + + if (_connectionMultiplexer != null) + { + _connectionSemaphore.Release(); + + return _connectionMultiplexer; + } + var configurationOptions = ConfigurationOptions.Parse(_connectionString); + + _connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(configurationOptions).WaitAsync(cancellationToken); - var connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(configurationOptions).WaitAsync(cancellationToken); + _connectionSemaphore.Release(); - return new RedisSession(connectionMultiplexer, snapshotSessionOptions); + return _connectionMultiplexer; + } + + private async Task CreateSession(SnapshotSessionOptions snapshotSessionOptions, CancellationToken cancellationToken) + { + var connectionMultiplexer = await OpenConnectionMultiplexer(cancellationToken); + + return RedisSession.Create(_serviceProvider, connectionMultiplexer.GetDatabase(), snapshotSessionOptions); } public async Task> CreateRepository(string snapshotSessionOptionsName, CancellationToken cancellationToken = default) @@ -74,4 +95,11 @@ public static RedisSnapshotRepositoryFactory Create(IServiceProvider keyNamespace ); } + + public override ValueTask DisposeAsync() + { + _connectionMultiplexer?.Dispose(); + + return ValueTask.CompletedTask; + } } diff --git a/test/EntityDb.Redis.Tests/Sessions/RedisSessionTests.cs b/test/EntityDb.Redis.Tests/Sessions/RedisSessionTests.cs index ea8c0cb8..241cc026 100644 --- a/test/EntityDb.Redis.Tests/Sessions/RedisSessionTests.cs +++ b/test/EntityDb.Redis.Tests/Sessions/RedisSessionTests.cs @@ -20,7 +20,7 @@ public async Task WhenExecutingWriteMethods_ThenThrow() { // ARRANGE - var readOnlyRedisSession = new RedisSession(default!, new SnapshotSessionOptions + var readOnlyRedisSession = new RedisSession(default!, default!, new SnapshotSessionOptions { ReadOnly = true });