LT-5770: [Trading Core] Potential Deduplication Lock Failure #560
Conversation
|
| @@ -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. | |||
| /// </summary> | |||
| public void HoldLock() | |||
| public void HoldLock(CancellationTokenSource cancellationTokenSource) | |||
There was a problem hiding this comment.
I think to clarify you better call it hostRunCancellationToken or something similar.
| @@ -79,15 +73,13 @@ await _database.LockExtendAsync(LockKey, _lockValue, | |||
| } | |||
| catch (Exception exception) | |||
| { | |||
| workerException = exception; | |||
| _cancellationTokenSource.Cancel(); | |||
| cancellationTokenSource.Cancel(); | |||
| } | |||
| }); | |||
| } | |||
|
|
|||
| public void Dispose() | |||
There was a problem hiding this comment.
Do you still need deduplication service to implement Idisposable interface?
| @@ -79,15 +73,13 @@ await _database.LockExtendAsync(LockKey, _lockValue, | |||
| } | |||
| catch (Exception exception) | |||
| { | |||
| workerException = exception; | |||
| _cancellationTokenSource.Cancel(); | |||
| cancellationTokenSource.Cancel(); | |||
There was a problem hiding this comment.
Imagine trading core started normally and processing requests. After an hour the redis is not available for some reason or just failed to extend the lock. This probably will lead to trading core app termination and attempt to immediately restart which is not acceptable.
src/MarginTrading.Backend/Program.cs
Outdated
| } | ||
| catch (Exception e) | ||
| { | ||
| CancellationTokenSource.Dispose(); |
There was a problem hiding this comment.
What if CancellationTokenSource is null here?
There was a problem hiding this comment.
Do you need StartupDeduplicationService still to implement IDisposable interface?
No description provided.