From fc613ba19055457619e0b6b19029c365b76f94d0 Mon Sep 17 00:00:00 2001 From: Aliaksandr Vashetsin Date: Thu, 31 Oct 2024 14:22:29 +0300 Subject: [PATCH 1/3] LT-5770: [Trading Core] Potential Deduplication Lock Failure --- .../Infrastructure/StartupDeduplicationService.cs | 14 +++----------- src/MarginTrading.Backend/Program.cs | 5 ++++- src/MarginTrading.Backend/Startup.cs | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs index 95ded70b9..2eae63b5c 100644 --- a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs +++ b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs @@ -15,7 +15,7 @@ namespace MarginTrading.Backend.Services.Infrastructure /// /// Ensure that only single instance of the app is running. /// - public class StartupDeduplicationService : IDisposable + public class StartupDeduplicationService: IDisposable { private const string LockKey = "TradingEngine:DeduplicationLock"; private readonly string _lockValue = Environment.MachineName; @@ -23,8 +23,6 @@ public class StartupDeduplicationService : IDisposable private readonly MarginTradingSettings _marginTradingSettings; private readonly IDatabase _database; - private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); - public StartupDeduplicationService( IWebHostEnvironment hostingEnvironment, MarginTradingSettings marginTradingSettings, @@ -47,7 +45,7 @@ public StartupDeduplicationService( /// But the probability of such situation is extremely small, so current implementation neglects it. /// In case if it is required to assure safety in clustered/replicated mode RedLock algorithm may be used. /// - public void HoldLock() + public void HoldLock(CancellationTokenSource cancellationTokenSource) { if (_hostingEnvironment.IsDevelopment()) { @@ -60,10 +58,6 @@ public void HoldLock() // exception is logged by the global handler } - Exception workerException = null; - // ReSharper disable once PossibleNullReferenceException - _cancellationTokenSource.Token.Register(() => throw workerException); - Task.Run(async () => { try @@ -79,15 +73,13 @@ await _database.LockExtendAsync(LockKey, _lockValue, } catch (Exception exception) { - workerException = exception; - _cancellationTokenSource.Cancel(); + cancellationTokenSource.Cancel(); } }); } public void Dispose() { - _cancellationTokenSource.Dispose(); } } } \ No newline at end of file diff --git a/src/MarginTrading.Backend/Program.cs b/src/MarginTrading.Backend/Program.cs index dd4e37ef2..bc62fd79c 100644 --- a/src/MarginTrading.Backend/Program.cs +++ b/src/MarginTrading.Backend/Program.cs @@ -20,6 +20,7 @@ namespace MarginTrading.Backend public class Program { internal static IHost AppHost { get; private set; } + internal static CancellationTokenSource CancellationTokenSource; public static async Task Main(string[] args) { @@ -44,6 +45,7 @@ public static async Task Main(string[] args) { try { + CancellationTokenSource = new CancellationTokenSource(); fatalErrorOccured = false; var configuration = new ConfigurationBuilder() @@ -65,10 +67,11 @@ public static async Task Main(string[] args) }) .Build(); - await AppHost.RunAsync(); + await AppHost.RunAsync(CancellationTokenSource.Token); } catch (Exception e) { + CancellationTokenSource.Dispose(); fatalErrorOccured = true; Console.WriteLine( $@"Error: {e.Message}{Environment.NewLine}{e.StackTrace}{Environment.NewLine}Restarting..."); diff --git a/src/MarginTrading.Backend/Startup.cs b/src/MarginTrading.Backend/Startup.cs index c8b567d53..857bf2b55 100644 --- a/src/MarginTrading.Backend/Startup.cs +++ b/src/MarginTrading.Backend/Startup.cs @@ -358,7 +358,7 @@ private StartupDeduplicationService RunHealthChecks(IConnectionMultiplexer redis marginTradingSettings, redis); - deduplicationService.HoldLock(); + deduplicationService.HoldLock(Program.CancellationTokenSource); new QueueValidationService(marginTradingSettings.StartupQueuesChecker.ConnectionString, new[] From 305786fb6e69fbf4fe791bc8c2e6e6ea5a5bd137 Mon Sep 17 00:00:00 2001 From: Aliaksandr Vashetsin Date: Thu, 31 Oct 2024 17:28:05 +0300 Subject: [PATCH 2/3] Revert "LT-5770: [Trading Core] Potential Deduplication Lock Failure" This reverts commit fc613ba19055457619e0b6b19029c365b76f94d0. --- .../Infrastructure/StartupDeduplicationService.cs | 14 +++++++++++--- src/MarginTrading.Backend/Program.cs | 5 +---- src/MarginTrading.Backend/Startup.cs | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs index 2eae63b5c..95ded70b9 100644 --- a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs +++ b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs @@ -15,7 +15,7 @@ namespace MarginTrading.Backend.Services.Infrastructure /// /// Ensure that only single instance of the app is running. /// - public class StartupDeduplicationService: IDisposable + public class StartupDeduplicationService : IDisposable { private const string LockKey = "TradingEngine:DeduplicationLock"; private readonly string _lockValue = Environment.MachineName; @@ -23,6 +23,8 @@ public class StartupDeduplicationService: IDisposable private readonly MarginTradingSettings _marginTradingSettings; private readonly IDatabase _database; + private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); + public StartupDeduplicationService( IWebHostEnvironment hostingEnvironment, MarginTradingSettings marginTradingSettings, @@ -45,7 +47,7 @@ public StartupDeduplicationService( /// But the probability of such situation is extremely small, so current implementation neglects it. /// In case if it is required to assure safety in clustered/replicated mode RedLock algorithm may be used. /// - public void HoldLock(CancellationTokenSource cancellationTokenSource) + public void HoldLock() { if (_hostingEnvironment.IsDevelopment()) { @@ -58,6 +60,10 @@ public void HoldLock(CancellationTokenSource cancellationTokenSource) // exception is logged by the global handler } + Exception workerException = null; + // ReSharper disable once PossibleNullReferenceException + _cancellationTokenSource.Token.Register(() => throw workerException); + Task.Run(async () => { try @@ -73,13 +79,15 @@ await _database.LockExtendAsync(LockKey, _lockValue, } catch (Exception exception) { - cancellationTokenSource.Cancel(); + workerException = exception; + _cancellationTokenSource.Cancel(); } }); } public void Dispose() { + _cancellationTokenSource.Dispose(); } } } \ No newline at end of file diff --git a/src/MarginTrading.Backend/Program.cs b/src/MarginTrading.Backend/Program.cs index bc62fd79c..dd4e37ef2 100644 --- a/src/MarginTrading.Backend/Program.cs +++ b/src/MarginTrading.Backend/Program.cs @@ -20,7 +20,6 @@ namespace MarginTrading.Backend public class Program { internal static IHost AppHost { get; private set; } - internal static CancellationTokenSource CancellationTokenSource; public static async Task Main(string[] args) { @@ -45,7 +44,6 @@ public static async Task Main(string[] args) { try { - CancellationTokenSource = new CancellationTokenSource(); fatalErrorOccured = false; var configuration = new ConfigurationBuilder() @@ -67,11 +65,10 @@ public static async Task Main(string[] args) }) .Build(); - await AppHost.RunAsync(CancellationTokenSource.Token); + await AppHost.RunAsync(); } catch (Exception e) { - CancellationTokenSource.Dispose(); fatalErrorOccured = true; Console.WriteLine( $@"Error: {e.Message}{Environment.NewLine}{e.StackTrace}{Environment.NewLine}Restarting..."); diff --git a/src/MarginTrading.Backend/Startup.cs b/src/MarginTrading.Backend/Startup.cs index 857bf2b55..c8b567d53 100644 --- a/src/MarginTrading.Backend/Startup.cs +++ b/src/MarginTrading.Backend/Startup.cs @@ -358,7 +358,7 @@ private StartupDeduplicationService RunHealthChecks(IConnectionMultiplexer redis marginTradingSettings, redis); - deduplicationService.HoldLock(Program.CancellationTokenSource); + deduplicationService.HoldLock(); new QueueValidationService(marginTradingSettings.StartupQueuesChecker.ConnectionString, new[] From beafc4eb1e57e496186dd6fb60b39853e47480c9 Mon Sep 17 00:00:00 2001 From: Aliaksandr Vashetsin Date: Thu, 31 Oct 2024 19:03:45 +0300 Subject: [PATCH 3/3] LT-5770: requested changes --- .../StartupDeduplicationService.cs | 55 +++++++++++-------- .../WebHostProcessTerminator.cs | 30 ++++++++++ src/MarginTrading.Backend/Program.cs | 43 +++++++++------ src/MarginTrading.Backend/Startup.cs | 4 +- 4 files changed, 91 insertions(+), 41 deletions(-) create mode 100644 src/MarginTrading.Backend.Services/Infrastructure/WebHostProcessTerminator.cs diff --git a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs index 95ded70b9..b904e0794 100644 --- a/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs +++ b/src/MarginTrading.Backend.Services/Infrastructure/StartupDeduplicationService.cs @@ -4,9 +4,15 @@ using System; using System.Threading; using System.Threading.Tasks; + +using Lykke.Common.Log; + using MarginTrading.Backend.Core.Settings; +using MarginTrading.Common.Services; + using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Hosting; + using StackExchange.Redis; namespace MarginTrading.Backend.Services.Infrastructure @@ -19,17 +25,18 @@ public class StartupDeduplicationService : IDisposable { private const string LockKey = "TradingEngine:DeduplicationLock"; private readonly string _lockValue = Environment.MachineName; + private readonly WebHostProcessTerminator _processTerminator; private readonly IWebHostEnvironment _hostingEnvironment; private readonly MarginTradingSettings _marginTradingSettings; private readonly IDatabase _database; - private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); - public StartupDeduplicationService( + WebHostProcessTerminator processTerminator, IWebHostEnvironment hostingEnvironment, MarginTradingSettings marginTradingSettings, IConnectionMultiplexer redis) { + _processTerminator = processTerminator; _hostingEnvironment = hostingEnvironment; _marginTradingSettings = marginTradingSettings; _database = redis.GetDatabase(); @@ -60,34 +67,38 @@ public void HoldLock() // exception is logged by the global handler } - Exception workerException = null; - // ReSharper disable once PossibleNullReferenceException - _cancellationTokenSource.Token.Register(() => throw workerException); - - Task.Run(async () => - { - try + Task.Run( + async () => { - while (true) + var run = true; + while (run) { - // wait and extend lock - await Task.Delay(_marginTradingSettings.DeduplicationLockExtensionPeriod); + try + { + // wait and extend lock + await Task.Delay(_marginTradingSettings.DeduplicationLockExtensionPeriod); - await _database.LockExtendAsync(LockKey, _lockValue, - _marginTradingSettings.DeduplicationLockExpiryPeriod); + var extendResult = await _database.LockExtendAsync( + LockKey, + _lockValue, + _marginTradingSettings.DeduplicationLockExpiryPeriod); + if (!extendResult) + { + LogLocator.CommonLog?.Error("DeduplicationService", message: "Lock is taken already."); + _processTerminator.TerminateProcess(); + run = false; + } + } + catch (Exception exception) + { + LogLocator.CommonLog?.Warning("DeduplicationService","Failed to extend lock.", exception); + } } - } - catch (Exception exception) - { - workerException = exception; - _cancellationTokenSource.Cancel(); - } - }); + }); } public void Dispose() { - _cancellationTokenSource.Dispose(); } } } \ No newline at end of file diff --git a/src/MarginTrading.Backend.Services/Infrastructure/WebHostProcessTerminator.cs b/src/MarginTrading.Backend.Services/Infrastructure/WebHostProcessTerminator.cs new file mode 100644 index 000000000..9bfdf65fc --- /dev/null +++ b/src/MarginTrading.Backend.Services/Infrastructure/WebHostProcessTerminator.cs @@ -0,0 +1,30 @@ +// Copyright (c) 2019 Lykke Corp. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; + +namespace MarginTrading.Backend.Services.Infrastructure +{ + public sealed class WebHostProcessTerminator : IDisposable + { + private readonly CancellationTokenSource _cancellationTokenSource; + + public WebHostProcessTerminator() + { + _cancellationTokenSource = new CancellationTokenSource(); + } + + public CancellationToken CancellationToken => _cancellationTokenSource.Token; + + public void TerminateProcess() + { + _cancellationTokenSource?.Cancel(); + } + + public void Dispose() + { + _cancellationTokenSource?.Dispose(); + } + } +} \ No newline at end of file diff --git a/src/MarginTrading.Backend/Program.cs b/src/MarginTrading.Backend/Program.cs index dd4e37ef2..2f2d77530 100644 --- a/src/MarginTrading.Backend/Program.cs +++ b/src/MarginTrading.Backend/Program.cs @@ -6,9 +6,12 @@ using System.Threading.Tasks; using Autofac.Extensions.DependencyInjection; using JetBrains.Annotations; + +using MarginTrading.Backend.Services.Infrastructure; using MarginTrading.Common.Services; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.PlatformAbstractions; @@ -20,6 +23,7 @@ namespace MarginTrading.Backend public class Program { internal static IHost AppHost { get; private set; } + internal static WebHostProcessTerminator ProcessTerminator { get; private set; } public static async Task Main(string[] args) { @@ -46,26 +50,29 @@ public static async Task Main(string[] args) { fatalErrorOccured = false; - var configuration = new ConfigurationBuilder() - .AddJsonFile("appsettings.json", optional: true) - .AddUserSecrets() - .AddEnvironmentVariables() - .Build(); + using (ProcessTerminator = new WebHostProcessTerminator()) + { + var configuration = new ConfigurationBuilder() + .AddJsonFile("appsettings.json", optional: true) + .AddUserSecrets() + .AddEnvironmentVariables() + .Build(); - AppHost = Host.CreateDefaultBuilder(args) - .UseServiceProviderFactory(new AutofacServiceProviderFactory()) - .ConfigureWebHostDefaults(webBuilder => - { - webBuilder.ConfigureKestrel(serverOptions => - { - // Set properties and call methods on options - }) - .UseConfiguration(configuration) - .UseStartup(); - }) - .Build(); + AppHost = Host.CreateDefaultBuilder(args) + .UseServiceProviderFactory(new AutofacServiceProviderFactory()) + .ConfigureWebHostDefaults(webBuilder => + { + webBuilder.ConfigureKestrel(serverOptions => + { + // Set properties and call methods on options + }) + .UseConfiguration(configuration) + .UseStartup(); + }) + .Build(); - await AppHost.RunAsync(); + await AppHost.RunAsync(ProcessTerminator.CancellationToken); + } } catch (Exception e) { diff --git a/src/MarginTrading.Backend/Startup.cs b/src/MarginTrading.Backend/Startup.cs index c8b567d53..b0a4764f7 100644 --- a/src/MarginTrading.Backend/Startup.cs +++ b/src/MarginTrading.Backend/Startup.cs @@ -354,7 +354,9 @@ private void InitializeJobs() private StartupDeduplicationService RunHealthChecks(IConnectionMultiplexer redis, MarginTradingSettings marginTradingSettings) { - var deduplicationService = new StartupDeduplicationService(Environment, + var deduplicationService = new StartupDeduplicationService( + Program.ProcessTerminator, + Environment, marginTradingSettings, redis);