From 2dfebd487de093bbe30f1d8751769e9ec9972e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Wed, 19 Mar 2025 12:47:19 +0100 Subject: [PATCH 1/8] Cleanup --- .../AsyncTimer.cs | 139 +++++++++--------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index 34aa3b62b9..d34baa9a89 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -1,96 +1,95 @@ -namespace ServiceControl.Infrastructure.BackgroundTasks +namespace ServiceControl.Infrastructure.BackgroundTasks; + +using System; +using System.Threading; +using System.Threading.Tasks; + +public enum TimerJobExecutionResult { - using System; - using System.Threading; - using System.Threading.Tasks; + ScheduleNextExecution, + ExecuteImmediately, + DoNotContinueExecuting +} - public enum TimerJobExecutionResult +public class TimerJob +{ + public TimerJob(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback) { - ScheduleNextExecution, - ExecuteImmediately, - DoNotContinueExecuting - } + tokenSource = new CancellationTokenSource(); + var token = tokenSource.Token; - public class TimerJob - { - public TimerJob(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback) + task = Task.Run(async () => { - tokenSource = new CancellationTokenSource(); - var token = tokenSource.Token; - - task = Task.Run(async () => + try { - try - { - await Task.Delay(due, token).ConfigureAwait(false); + await Task.Delay(due, token).ConfigureAwait(false); - while (!token.IsCancellationRequested) + while (!token.IsCancellationRequested) + { + try { - try - { - var result = await callback(token).ConfigureAwait(false); - if (result == TimerJobExecutionResult.DoNotContinueExecuting) - { - tokenSource.Cancel(); - } - else if (result == TimerJobExecutionResult.ScheduleNextExecution) - { - await Task.Delay(interval, token).ConfigureAwait(false); - } - - //Otherwise execute immediately - } - catch (OperationCanceledException) when (token.IsCancellationRequested) + var result = await callback(token).ConfigureAwait(false); + if (result == TimerJobExecutionResult.DoNotContinueExecuting) { - break; + tokenSource.Cancel(); } - catch (Exception ex) + else if (result == TimerJobExecutionResult.ScheduleNextExecution) { - errorCallback(ex); + await Task.Delay(interval, token).ConfigureAwait(false); } + + //Otherwise execute immediately + } + catch (OperationCanceledException) when (token.IsCancellationRequested) + { + break; + } + catch (Exception ex) + { + errorCallback(ex); } } - catch (OperationCanceledException) when (token.IsCancellationRequested) - { - // no-op - } - }, CancellationToken.None); + } + catch (OperationCanceledException) when (token.IsCancellationRequested) + { + // no-op + } + }, CancellationToken.None); + } + + public async Task Stop() + { + if (tokenSource == null) + { + return; } - public async Task Stop() + await tokenSource.CancelAsync().ConfigureAwait(false); + tokenSource.Dispose(); + + if (task != null) { - if (tokenSource == null) + try { - return; + await task.ConfigureAwait(false); } - - await tokenSource.CancelAsync().ConfigureAwait(false); - tokenSource.Dispose(); - - if (task != null) + catch (OperationCanceledException) when (tokenSource.IsCancellationRequested) { - try - { - await task.ConfigureAwait(false); - } - catch (OperationCanceledException) when (tokenSource.IsCancellationRequested) - { - //NOOP - } + //NOOP } } - - Task task; - CancellationTokenSource tokenSource; } - public interface IAsyncTimer - { - TimerJob Schedule(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback); - } + readonly Task task; + readonly CancellationTokenSource tokenSource; +} - public class AsyncTimer : IAsyncTimer - { - public TimerJob Schedule(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback) => new TimerJob(callback, due, interval, errorCallback); - } +public interface IAsyncTimer +{ + TimerJob Schedule(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback); +} + +public class AsyncTimer : IAsyncTimer +{ + public TimerJob Schedule(Func> callback, TimeSpan due, TimeSpan interval, Action errorCallback) => new(callback, due, interval, errorCallback); } \ No newline at end of file From ce72d493d184258165076340a38d07b4edf12f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Wed, 19 Mar 2025 12:54:45 +0100 Subject: [PATCH 2/8] Add token to timer stop --- .../AsyncTimer.cs | 22 ++++++++++--------- .../Licensing/LicenseCheckHostedService.cs | 2 +- .../InternalCustomCheckManager.cs | 2 +- .../Licensing/LicenseCheckHostedService.cs | 2 +- .../HeartbeatMonitoringHostedService.cs | 12 +--------- .../Recoverability/RecoverabilityComponent.cs | 15 +++---------- 6 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index d34baa9a89..f3781d2796 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -57,7 +57,7 @@ public TimerJob(Func> callback, }, CancellationToken.None); } - public async Task Stop() + public async Task Stop(CancellationToken cancellationToken) { if (tokenSource == null) { @@ -67,16 +67,18 @@ public async Task Stop() await tokenSource.CancelAsync().ConfigureAwait(false); tokenSource.Dispose(); - if (task != null) + if (task == null) { - try - { - await task.ConfigureAwait(false); - } - catch (OperationCanceledException) when (tokenSource.IsCancellationRequested) - { - //NOOP - } + return; + } + + try + { + Task.WaitAll([task], cancellationToken); + } + catch (OperationCanceledException) when (tokenSource.IsCancellationRequested) + { + //NOOP } } diff --git a/src/ServiceControl.Monitoring/Licensing/LicenseCheckHostedService.cs b/src/ServiceControl.Monitoring/Licensing/LicenseCheckHostedService.cs index 1842f8603d..a5b10f73b5 100644 --- a/src/ServiceControl.Monitoring/Licensing/LicenseCheckHostedService.cs +++ b/src/ServiceControl.Monitoring/Licensing/LicenseCheckHostedService.cs @@ -20,7 +20,7 @@ public Task StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } - public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(); + public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(cancellationToken); TimerJob timer; diff --git a/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs b/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs index 38853a667c..e3443cd2e0 100644 --- a/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs +++ b/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs @@ -67,7 +67,7 @@ async Task Run(CancellationToken cancellationToken) : TimerJobExecutionResult.DoNotContinueExecuting; } - public Task Stop() => timer?.Stop() ?? Task.CompletedTask; + public Task Stop() => timer?.Stop(CancellationToken.None) ?? Task.CompletedTask; TimerJob timer; readonly ICustomCheck check; diff --git a/src/ServiceControl/Licensing/LicenseCheckHostedService.cs b/src/ServiceControl/Licensing/LicenseCheckHostedService.cs index 0bcb97608d..3afcc3c2ba 100644 --- a/src/ServiceControl/Licensing/LicenseCheckHostedService.cs +++ b/src/ServiceControl/Licensing/LicenseCheckHostedService.cs @@ -21,7 +21,7 @@ public Task StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } - public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(); + public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(cancellationToken); TimerJob timer; diff --git a/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs b/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs index a3d86a01ba..1f9d6edaa5 100644 --- a/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs +++ b/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs @@ -24,17 +24,7 @@ public async Task StartAsync(CancellationToken cancellationToken) timer = scheduler.Schedule(_ => CheckEndpoints(), TimeSpan.Zero, TimeSpan.FromSeconds(5), e => { log.Error("Exception occurred when monitoring endpoint instances", e); }); } - public async Task StopAsync(CancellationToken cancellationToken) - { - try - { - await timer.Stop(); - } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) - { - //NOOP, invoked Stop does not - } - } + public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(cancellationToken); async Task CheckEndpoints() { diff --git a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs index ce2abbfa7c..1736572799 100644 --- a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs +++ b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs @@ -174,10 +174,7 @@ public Task StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } - public Task StopAsync(CancellationToken cancellationToken) - { - return timer?.Stop() ?? Task.CompletedTask; - } + public Task StopAsync(CancellationToken cancellationToken) => timer?.Stop(cancellationToken) ?? Task.CompletedTask; async Task ProcessRequestedBulkRetryOperations() { @@ -237,10 +234,7 @@ public Task StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } - public Task StopAsync(CancellationToken cancellationToken) - { - return timer.Stop(); - } + public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(cancellationToken); TimerJob timer; readonly IAsyncTimer scheduler; @@ -266,10 +260,7 @@ public Task StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } - public async Task StopAsync(CancellationToken cancellationToken) - { - await timer.Stop(); - } + public Task StopAsync(CancellationToken cancellationToken) => timer.Stop(cancellationToken); async Task Process(CancellationToken cancellationToken) { From 664e51d3aac78a67208c91295c45b48c3f12d8ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Wed, 19 Mar 2025 13:43:06 +0100 Subject: [PATCH 3/8] Add exponential backoff delay --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index f3781d2796..22583baeb0 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -24,11 +24,15 @@ public TimerJob(Func> callback, { await Task.Delay(due, token).ConfigureAwait(false); + var consecutiveFailures = 0; + while (!token.IsCancellationRequested) { try { var result = await callback(token).ConfigureAwait(false); + + consecutiveFailures = 0; if (result == TimerJobExecutionResult.DoNotContinueExecuting) { tokenSource.Cancel(); @@ -46,6 +50,11 @@ public TimerJob(Func> callback, } catch (Exception ex) { + consecutiveFailures++; + var exponentialBackoffDelay = TimeSpan.FromSeconds(int.Max(60, consecutiveFailures * consecutiveFailures)); + + await Task.Delay(exponentialBackoffDelay, token).ConfigureAwait(false); + errorCallback(ex); } } From 9ccab61c3b8f7e445cc08ce31765ca2b0cd3f2ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Wed, 19 Mar 2025 13:56:06 +0100 Subject: [PATCH 4/8] Use min --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index 22583baeb0..01675e9172 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -51,7 +51,7 @@ public TimerJob(Func> callback, catch (Exception ex) { consecutiveFailures++; - var exponentialBackoffDelay = TimeSpan.FromSeconds(int.Max(60, consecutiveFailures * consecutiveFailures)); + var exponentialBackoffDelay = TimeSpan.FromSeconds(int.Min(60, consecutiveFailures * consecutiveFailures)); await Task.Delay(exponentialBackoffDelay, token).ConfigureAwait(false); From f0f5fa2308e61590677e1a946078d03045ce69c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Wed, 19 Mar 2025 19:37:34 +0100 Subject: [PATCH 5/8] Use 10, 20, 30..60 as back off instead --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index 01675e9172..78922d2c92 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -51,9 +51,9 @@ public TimerJob(Func> callback, catch (Exception ex) { consecutiveFailures++; - var exponentialBackoffDelay = TimeSpan.FromSeconds(int.Min(60, consecutiveFailures * consecutiveFailures)); + var backoffDelay = TimeSpan.FromSeconds(int.Min(6, consecutiveFailures) * 10); - await Task.Delay(exponentialBackoffDelay, token).ConfigureAwait(false); + await Task.Delay(backoffDelay, token).ConfigureAwait(false); errorCallback(ex); } From 0d7b585a06988ce4c1752b5a5194933543e36173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Thu, 20 Mar 2025 08:19:38 +0100 Subject: [PATCH 6/8] use task.wait --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index 78922d2c92..97ae3dcf19 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -83,7 +83,7 @@ public async Task Stop(CancellationToken cancellationToken) try { - Task.WaitAll([task], cancellationToken); + await task.WaitAsync(cancellationToken).ConfigureAwait(false); } catch (OperationCanceledException) when (tokenSource.IsCancellationRequested) { From fb0bf12dfd0634ba2e77799ede9d73749c1b40dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Thu, 20 Mar 2025 18:05:02 +0100 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Ramon Smits --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index 97ae3dcf19..e482b488e0 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -51,7 +51,9 @@ public TimerJob(Func> callback, catch (Exception ex) { consecutiveFailures++; - var backoffDelay = TimeSpan.FromSeconds(int.Min(6, consecutiveFailures) * 10); + const int MaxDelayDurationInSeconds = 60; + var delayInSeconds = consecutiveFailures * 10; + var backoffDelay = TimeSpan.FromSeconds(int.Min(MaxDelayDurationInSeconds, delayInSeconds)); await Task.Delay(backoffDelay, token).ConfigureAwait(false); @@ -75,6 +77,7 @@ public async Task Stop(CancellationToken cancellationToken) await tokenSource.CancelAsync().ConfigureAwait(false); tokenSource.Dispose(); + tokenSource = null; if (task == null) { From 31d9ea65c029b1c12210291cacc65f4bf8dc3beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20=C3=96hlund?= Date: Fri, 21 Mar 2025 07:01:59 +0100 Subject: [PATCH 8/8] Tokensource is readonly --- src/ServiceControl.Infrastructure/AsyncTimer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs index e482b488e0..045e8d229e 100644 --- a/src/ServiceControl.Infrastructure/AsyncTimer.cs +++ b/src/ServiceControl.Infrastructure/AsyncTimer.cs @@ -77,7 +77,6 @@ public async Task Stop(CancellationToken cancellationToken) await tokenSource.CancelAsync().ConfigureAwait(false); tokenSource.Dispose(); - tokenSource = null; if (task == null) {