From e09391f4959f67b2463d13367bd414ace60430ed Mon Sep 17 00:00:00 2001 From: stidsborg Date: Wed, 31 Jul 2024 09:43:07 +0200 Subject: [PATCH] Only perform I/O for existing registered timeouts in TimeoutProvider --- .../InMemoryTests/TimeoutStoreTests.cs | 4 ++ .../TestTemplates/TimeoutStoreTests.cs | 42 ++++++++++++++++--- .../CoreRuntime/TimeoutProvider.cs | 3 +- .../TimeoutStoreTests.cs | 4 ++ .../TimeoutStoreTests.cs | 4 ++ .../TimeoutStoreTests.cs | 4 ++ 6 files changed, 55 insertions(+), 6 deletions(-) diff --git a/Core/Cleipnir.ResilientFunctions.Tests/InMemoryTests/TimeoutStoreTests.cs b/Core/Cleipnir.ResilientFunctions.Tests/InMemoryTests/TimeoutStoreTests.cs index 57fb4347..123e7247 100644 --- a/Core/Cleipnir.ResilientFunctions.Tests/InMemoryTests/TimeoutStoreTests.cs +++ b/Core/Cleipnir.ResilientFunctions.Tests/InMemoryTests/TimeoutStoreTests.cs @@ -35,4 +35,8 @@ public override Task RegisteredTimeoutIsReturnedFromTimeoutProviderForFunctionId [TestMethod] public override Task TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout() => TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout(new InMemoryTimeoutStore().CastTo().ToTask()); + + [TestMethod] + public override Task CancellingNonExistingTimeoutDoesNotResultInIO() + => CancellingNonExistingTimeoutDoesNotResultInIO(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); } \ No newline at end of file diff --git a/Core/Cleipnir.ResilientFunctions.Tests/TestTemplates/TimeoutStoreTests.cs b/Core/Cleipnir.ResilientFunctions.Tests/TestTemplates/TimeoutStoreTests.cs index 52e5f57b..d581ba78 100644 --- a/Core/Cleipnir.ResilientFunctions.Tests/TestTemplates/TimeoutStoreTests.cs +++ b/Core/Cleipnir.ResilientFunctions.Tests/TestTemplates/TimeoutStoreTests.cs @@ -181,16 +181,45 @@ protected async Task TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeo await timeoutProvider.RegisterTimeout("timeoutId1", expiresIn: TimeSpan.FromHours(1), overwrite: false); upsertCount.ShouldBe(1); } + + public abstract Task CancellingNonExistingTimeoutDoesNotResultInIO(); + protected async Task CancellingNonExistingTimeoutDoesNotResultInIO(Task storeTask) + { + var removeCount = 0; + var store = new TimeoutStoreDecorator(await storeTask, removeTimeoutCallback: () => removeCount++); + var functionId = TestFlowId.Create(); + + var timeoutProvider = new TimeoutProvider( + functionId, + store, + messageWriter: null, + timeoutCheckFrequency: TimeSpan.Zero + ); + + var pendingTimeouts = await timeoutProvider.PendingTimeouts(); + pendingTimeouts.ShouldBeEmpty(); + await timeoutProvider.RegisterTimeout("SomeOtherTimeoutId", expiresIn: TimeSpan.FromHours(1)); + + await timeoutProvider.CancelTimeout("SomeTimeoutId"); + + removeCount.ShouldBe(0); + } + private class TimeoutStoreDecorator : ITimeoutStore { private readonly ITimeoutStore _inner; - private readonly Action _upsertTimeoutCallback; + private readonly Action? _upsertTimeoutCallback; + private readonly Action? _removeTimeoutCallback; - public TimeoutStoreDecorator(ITimeoutStore inner, Action upsertTimeoutCallback) + public TimeoutStoreDecorator( + ITimeoutStore inner, + Action? upsertTimeoutCallback = null, + Action? removeTimeoutCallback = null) { _inner = inner; _upsertTimeoutCallback = upsertTimeoutCallback; + _removeTimeoutCallback = removeTimeoutCallback; } public Task Initialize() => _inner.Initialize(); @@ -198,13 +227,16 @@ public TimeoutStoreDecorator(ITimeoutStore inner, Action upsertTimeoutCallback) public Task UpsertTimeout(StoredTimeout storedTimeout, bool overwrite) { - _upsertTimeoutCallback(); + _upsertTimeoutCallback?.Invoke(); return _inner.UpsertTimeout(storedTimeout, overwrite); } public Task RemoveTimeout(FlowId flowId, string timeoutId) - => _inner.RemoveTimeout(flowId, timeoutId); - + { + _removeTimeoutCallback?.Invoke(); + return _inner.RemoveTimeout(flowId, timeoutId); + } + public Task Remove(FlowId flowId) => _inner.Remove(flowId); diff --git a/Core/Cleipnir.ResilientFunctions/CoreRuntime/TimeoutProvider.cs b/Core/Cleipnir.ResilientFunctions/CoreRuntime/TimeoutProvider.cs index c666aece..9bc9769a 100644 --- a/Core/Cleipnir.ResilientFunctions/CoreRuntime/TimeoutProvider.cs +++ b/Core/Cleipnir.ResilientFunctions/CoreRuntime/TimeoutProvider.cs @@ -77,7 +77,8 @@ await _messageWriter.AppendMessage( public async Task CancelTimeout(string timeoutId) { lock (_sync) - _localTimeouts.Remove(timeoutId); + if (!_localTimeouts.Remove(timeoutId)) + return; await _timeoutStore.RemoveTimeout(_flowId, timeoutId); } diff --git a/Stores/MySQL/Cleipnir.ResilientFunctions.MySQL.Tests/TimeoutStoreTests.cs b/Stores/MySQL/Cleipnir.ResilientFunctions.MySQL.Tests/TimeoutStoreTests.cs index 8e1ef064..81d594c3 100644 --- a/Stores/MySQL/Cleipnir.ResilientFunctions.MySQL.Tests/TimeoutStoreTests.cs +++ b/Stores/MySQL/Cleipnir.ResilientFunctions.MySQL.Tests/TimeoutStoreTests.cs @@ -33,4 +33,8 @@ public override Task RegisteredTimeoutIsReturnedFromTimeoutProviderForFunctionId [TestMethod] public override Task TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout() => TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); + + [TestMethod] + public override Task CancellingNonExistingTimeoutDoesNotResultInIO() + => CancellingNonExistingTimeoutDoesNotResultInIO(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); } \ No newline at end of file diff --git a/Stores/PostgreSQL/Cleipnir.ResilientFunctions.PostgreSQL.Tests/TimeoutStoreTests.cs b/Stores/PostgreSQL/Cleipnir.ResilientFunctions.PostgreSQL.Tests/TimeoutStoreTests.cs index 27400036..757a5fa3 100644 --- a/Stores/PostgreSQL/Cleipnir.ResilientFunctions.PostgreSQL.Tests/TimeoutStoreTests.cs +++ b/Stores/PostgreSQL/Cleipnir.ResilientFunctions.PostgreSQL.Tests/TimeoutStoreTests.cs @@ -27,6 +27,10 @@ public override Task RegisteredTimeoutIsReturnedFromTimeoutProviderForFunctionId public override Task TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout() => TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); + [TestMethod] + public override Task CancellingNonExistingTimeoutDoesNotResultInIO() + => CancellingNonExistingTimeoutDoesNotResultInIO(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); + [TestMethod] public override Task RegisteredTimeoutIsReturnedFromTimeoutProvider() => RegisteredTimeoutIsReturnedFromTimeoutProvider(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); diff --git a/Stores/SqlServer/Cleipnir.ResilientFunctions.SqlServer.Tests/TimeoutStoreTests.cs b/Stores/SqlServer/Cleipnir.ResilientFunctions.SqlServer.Tests/TimeoutStoreTests.cs index 27baf7b6..ca3894bb 100644 --- a/Stores/SqlServer/Cleipnir.ResilientFunctions.SqlServer.Tests/TimeoutStoreTests.cs +++ b/Stores/SqlServer/Cleipnir.ResilientFunctions.SqlServer.Tests/TimeoutStoreTests.cs @@ -34,4 +34,8 @@ public override Task RegisteredTimeoutIsReturnedFromTimeoutProviderForFunctionId [TestMethod] public override Task TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout() => TimeoutIsNotRegisteredAgainWhenProviderAlreadyContainsTimeout(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); + + [TestMethod] + public override Task CancellingNonExistingTimeoutDoesNotResultInIO() + => CancellingNonExistingTimeoutDoesNotResultInIO(FunctionStoreFactory.Create().SelectAsync(s => s.TimeoutStore)); } \ No newline at end of file