Don't commit an SMT engine while deleting (#591)

* Don't commit an SMT engine while deleting - #590 * Update from review
sillsdev · Jan 6, 2025 · b2e75c8 · b2e75c8
1 parent b1063c6
commit b2e75c8
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 12 deletions.
diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineService.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineService.cs
@@ -64,21 +64,22 @@ public async Task<TranslationEngine> CreateAsync(
 
     public async Task DeleteAsync(string engineId, CancellationToken cancellationToken = default)
     {
-        await CancelBuildJobAsync(engineId, cancellationToken);
+        // there is no way to cancel this call
+        SmtTransferEngineState state = _stateService.Get(engineId);
+        state.IsMarkedForDeletion = true;
+
+        await CancelBuildJobAsync(engineId, CancellationToken.None);
 
         await _dataAccessContext.WithTransactionAsync(
             async ct =>
             {
                 await _engines.DeleteAsync(e => e.EngineId == engineId, ct);
                 await _trainSegmentPairs.DeleteAllAsync(p => p.TranslationEngineRef == engineId, ct);
             },
-            cancellationToken: cancellationToken
+            cancellationToken: CancellationToken.None
         );
         await _buildJobService.DeleteEngineAsync(engineId, CancellationToken.None);
-
-        SmtTransferEngineState state = _stateService.Get(engineId);
         _stateService.Remove(engineId);
-        // there is no way to cancel this call
         state.DeleteData();
         state.Dispose();
         await _lockFactory.DeleteAsync(engineId, CancellationToken.None);
@@ -93,6 +94,8 @@ public async Task<IReadOnlyList<TranslationResult>> TranslateAsync(
     {
         TranslationEngine engine = await GetBuiltEngineAsync(engineId, cancellationToken);
         SmtTransferEngineState state = _stateService.Get(engineId);
+        if (state.IsMarkedForDeletion)
+            throw new InvalidOperationException("Engine is marked for deletion.");
 
         IDistributedReaderWriterLock @lock = await _lockFactory.CreateAsync(engineId, cancellationToken);
         IReadOnlyList<TranslationResult> results = await @lock.ReaderLockAsync(
@@ -117,6 +120,8 @@ public async Task<WordGraph> GetWordGraphAsync(
     {
         TranslationEngine engine = await GetBuiltEngineAsync(engineId, cancellationToken);
         SmtTransferEngineState state = _stateService.Get(engineId);
+        if (state.IsMarkedForDeletion)
+            throw new InvalidOperationException("Engine is marked for deletion.");
 
         IDistributedReaderWriterLock @lock = await _lockFactory.CreateAsync(engineId, cancellationToken);
         WordGraph result = await @lock.ReaderLockAsync(
@@ -142,6 +147,8 @@ public async Task TrainSegmentPairAsync(
     )
     {
         SmtTransferEngineState state = _stateService.Get(engineId);
+        if (state.IsMarkedForDeletion)
+            throw new InvalidOperationException("Engine is marked for deletion.");
 
         IDistributedReaderWriterLock @lock = await _lockFactory.CreateAsync(engineId, cancellationToken);
         await @lock.WriterLockAsync(

diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineState.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineState.cs
@@ -22,6 +22,7 @@ string engineId
     public string EngineId { get; } = engineId;
 
     public bool IsUpdated { get; set; }
+    public bool IsMarkedForDeletion { get; set; }
     public int CurrentBuildRevision { get; set; } = -1;
     public DateTime LastUsedTime { get; private set; } = DateTime.UtcNow;
     public bool IsLoaded => _hybridEngine != null;
@@ -39,6 +40,9 @@ public async Task<HybridTranslationEngine> GetHybridEngineAsync(
         CancellationToken cancellationToken = default
     )
     {
+        if (IsMarkedForDeletion)
+            throw new InvalidOperationException("Engine is marked for deletion");
+
         using (await _lock.LockAsync(cancellationToken))
         {
             if (_hybridEngine is not null && CurrentBuildRevision != -1 && buildRevision != CurrentBuildRevision)
@@ -84,6 +88,9 @@ public void Commit(int buildRevision, TimeSpan inactiveTimeout)
 
         if (CurrentBuildRevision == -1)
             CurrentBuildRevision = buildRevision;
+
+        SaveModel();
+
         if (buildRevision != CurrentBuildRevision)
         {
             Unload();
@@ -93,10 +100,6 @@ public void Commit(int buildRevision, TimeSpan inactiveTimeout)
         {
             Unload();
         }
-        else
-        {
-            SaveModel();
-        }
     }
 
     public void Touch()
@@ -106,7 +109,7 @@ public void Touch()
 
     private void SaveModel()
     {
-        if (_smtModel is not null && IsUpdated)
+        if (_smtModel is not null && IsUpdated && !IsMarkedForDeletion)
         {
             _smtModel.Save();
             IsUpdated = false;
@@ -118,8 +121,6 @@ private void Unload()
         if (_hybridEngine is null)
             return;
 
-        SaveModel();
-
         _hybridEngine.Dispose();
 
         _smtModel = null;

diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineStateService.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferEngineStateService.cs
@@ -38,6 +38,11 @@ public async Task CommitAsync(
     {
         foreach (SmtTransferEngineState state in _engineStates.Values)
         {
+            if (!state.IsLoaded || state.IsMarkedForDeletion)
+            {
+                continue;
+            }
+
             try
             {
                 IDistributedReaderWriterLock @lock = await lockFactory.CreateAsync(state.EngineId, cancellationToken);