diff --git a/src/Serval/src/Serval.Shared/Models/IInitializableEntity.cs b/src/Serval/src/Serval.Shared/Models/IInitializableEntity.cs new file mode 100644 index 00000000..cef5c884 --- /dev/null +++ b/src/Serval/src/Serval.Shared/Models/IInitializableEntity.cs @@ -0,0 +1,7 @@ +namespace Serval.Shared.Models; + +public interface IInitializableEntity : IEntity +{ + bool? IsInitialized { get; set; } + DateTime? DateCreated { get; set; } +} diff --git a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs index 7c067e90..0affde0f 100644 --- a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs +++ b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs @@ -1297,7 +1297,7 @@ private Engine Map(TranslationEngineConfigDto source) Owner = Owner, Corpora = [], IsModelPersisted = source.IsModelPersisted, - SuccessfullyCreated = false + IsInitialized = false }; } @@ -1310,7 +1310,7 @@ private static Build Map(Engine engine, TranslationBuildConfigDto source) Pretranslate = Map(engine, source.Pretranslate), TrainOn = Map(engine, source.TrainOn), Options = Map(source.Options), - SuccessfullyStarted = false + IsInitialized = false }; } diff --git a/src/Serval/src/Serval.Translation/Models/Build.cs b/src/Serval/src/Serval.Translation/Models/Build.cs index 0ae291e7..ddfaee32 100644 --- a/src/Serval/src/Serval.Translation/Models/Build.cs +++ b/src/Serval/src/Serval.Translation/Models/Build.cs @@ -1,6 +1,6 @@ namespace Serval.Translation.Models; -public record Build : IEntity +public record Build : IInitializableEntity { public string Id { get; set; } = ""; public int Revision { get; set; } = 1; @@ -15,5 +15,6 @@ public record Build : IEntity public JobState State { get; init; } = JobState.Pending; public DateTime? DateFinished { get; init; } public IReadOnlyDictionary? Options { get; init; } - public bool SuccessfullyStarted { get; init; } + public bool? IsInitialized { get; set; } + public DateTime? DateCreated { get; set; } } diff --git a/src/Serval/src/Serval.Translation/Models/Engine.cs b/src/Serval/src/Serval.Translation/Models/Engine.cs index 94722674..df8fd26b 100644 --- a/src/Serval/src/Serval.Translation/Models/Engine.cs +++ b/src/Serval/src/Serval.Translation/Models/Engine.cs @@ -1,6 +1,6 @@ namespace Serval.Translation.Models; -public record Engine : IOwnedEntity +public record Engine : IOwnedEntity, IInitializableEntity { public string Id { get; set; } = ""; public int Revision { get; set; } = 1; @@ -16,5 +16,6 @@ public record Engine : IOwnedEntity public int ModelRevision { get; init; } public double Confidence { get; init; } public int CorpusSize { get; init; } - public bool SuccessfullyCreated { get; init; } + public bool? IsInitialized { get; set; } + public DateTime? DateCreated { get; set; } } diff --git a/src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs b/src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs index 7885d6a9..f8fa99b9 100644 --- a/src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs +++ b/src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs @@ -1,42 +1,7 @@ -using Microsoft.Extensions.DependencyInjection; -using SIL.ServiceToolkit.Services; - namespace Serval.Translation.Services; public class BuildCleanupService( IServiceProvider services, ILogger logger, TimeSpan? timeout = null -) : RecurrentTask("Build Cleanup Service", services, RefreshPeriod, logger) -{ - private readonly ILogger _logger = logger; - private readonly TimeSpan _timeout = timeout ?? TimeSpan.FromMinutes(2); - private static readonly TimeSpan RefreshPeriod = TimeSpan.FromDays(1); - - protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken) - { - _logger.LogInformation("Running build cleanup job"); - var builds = scope.ServiceProvider.GetRequiredService>(); - await CheckBuildsAsync(builds, cancellationToken); - } - - public async Task CheckBuildsAsync(IRepository builds, CancellationToken cancellationToken) - { - IReadOnlyList allBuilds = await builds.GetAllAsync(cancellationToken); - IEnumerable notStartedBuilds = allBuilds.Where(b => !b.SuccessfullyStarted); - await Task.Delay(_timeout, cancellationToken); //Make sure the builds are not midway through starting - foreach ( - Build build in await builds.GetAllAsync( - b => notStartedBuilds.Select(c => c.Id).Contains(b.Id), - cancellationToken - ) - ) - { - if (!build.SuccessfullyStarted) - { - _logger.LogInformation("Deleting build {id} because it was never successfully started", build.Id); - await builds.DeleteAsync(build, cancellationToken); - } - } - } -} +) : UninitializedCleanupService(services, logger, timeout) { } diff --git a/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs b/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs index 7d502c89..bd3cc8ef 100644 --- a/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs +++ b/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs @@ -1,5 +1,4 @@ using Microsoft.Extensions.DependencyInjection; -using SIL.ServiceToolkit.Services; namespace Serval.Translation.Services; @@ -7,41 +6,24 @@ public class EngineCleanupService( IServiceProvider services, ILogger logger, TimeSpan? timeout = null -) : RecurrentTask("Engine Cleanup Service", services, RefreshPeriod, logger) +) : UninitializedCleanupService(services, logger, timeout) { - private readonly ILogger _logger = logger; - private readonly TimeSpan _timeout = timeout ?? TimeSpan.FromMinutes(2); - private static readonly TimeSpan RefreshPeriod = TimeSpan.FromDays(1); + public EngineService? EngineService { get; set; } protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken) { - _logger.LogInformation("Running engine cleanup job"); - var engines = scope.ServiceProvider.GetRequiredService>(); - var engineService = scope.ServiceProvider.GetRequiredService(); - await CheckEnginesAsync(engines, engineService, cancellationToken); + EngineService = scope.ServiceProvider.GetRequiredService(); + await base.DoWorkAsync(scope, cancellationToken); } - public async Task CheckEnginesAsync( + protected override async Task DeleteEntityAsync( IRepository engines, - EngineService engineService, + Engine engine, CancellationToken cancellationToken ) { - IReadOnlyList allEngines = await engines.GetAllAsync(cancellationToken); - IEnumerable notCreatedEngines = allEngines.Where(e => !e.SuccessfullyCreated); - await Task.Delay(_timeout, cancellationToken); //Make sure the engines are not midway through being created - foreach ( - Engine engine in await engines.GetAllAsync( - e => notCreatedEngines.Select(f => f.Id).Contains(e.Id), - cancellationToken - ) - ) - { - if (!engine.SuccessfullyCreated) - { - _logger.LogInformation("Deleting engine {id} because it was never successfully created", engine.Id); - await engineService.DeleteAsync(engine.Id, cancellationToken); - } - } + if (EngineService == null) + return; + await EngineService.DeleteAsync(engine.Id, cancellationToken); } } diff --git a/src/Serval/src/Serval.Translation/Services/EngineService.cs b/src/Serval/src/Serval.Translation/Services/EngineService.cs index 42daad0a..02e1aee5 100644 --- a/src/Serval/src/Serval.Translation/Services/EngineService.cs +++ b/src/Serval/src/Serval.Translation/Services/EngineService.cs @@ -120,9 +120,9 @@ await client.TrainSegmentPairAsync( public override async Task CreateAsync(Engine engine, CancellationToken cancellationToken = default) { - bool updateIsModelPersisted = engine.IsModelPersisted is null; try { + engine.DateCreated = DateTime.UtcNow; await Entities.InsertAsync(engine, cancellationToken); TranslationEngineApi.TranslationEngineApiClient? client = _grpcClientFactory.CreateClient(engine.Type); @@ -148,7 +148,11 @@ public override async Task CreateAsync(Engine engine, CancellationToken }; await Entities.UpdateAsync( engine, - u => u.Set(e => e.SuccessfullyCreated, true), + u => + { + u.Set(e => e.IsInitialized, true); + u.Set(e => e.IsModelPersisted, engine.IsModelPersisted); + }, cancellationToken: cancellationToken ); } @@ -169,14 +173,6 @@ await Entities.UpdateAsync( await Entities.DeleteAsync(engine, CancellationToken.None); throw; } - if (updateIsModelPersisted) - { - await Entities.UpdateAsync( - engine, - u => u.Set(e => e.IsModelPersisted, engine.IsModelPersisted), - cancellationToken: cancellationToken - ); - } return engine; } @@ -221,6 +217,7 @@ private Dictionary> GetChapters(string fileLocation, string sc public async Task StartBuildAsync(Build build, CancellationToken cancellationToken = default) { + build.DateCreated = DateTime.UtcNow; Engine engine = await GetAsync(build.EngineRef, cancellationToken); await _builds.InsertAsync(build, cancellationToken); @@ -298,9 +295,9 @@ public async Task StartBuildAsync(Build build, CancellationToken cancellationTok } await client.StartBuildAsync(request, cancellationToken: cancellationToken); await _builds.UpdateAsync( - build.Id, - u => u.Set(e => e.SuccessfullyStarted, true), - cancellationToken: cancellationToken + b => b.Id == build.Id, + u => u.Set(e => e.IsInitialized, true), + cancellationToken: CancellationToken.None ); } catch diff --git a/src/Serval/src/Serval.Translation/Services/UnitializedEntityCleanupService.cs b/src/Serval/src/Serval.Translation/Services/UnitializedEntityCleanupService.cs new file mode 100644 index 00000000..2730a08c --- /dev/null +++ b/src/Serval/src/Serval.Translation/Services/UnitializedEntityCleanupService.cs @@ -0,0 +1,50 @@ +using Microsoft.Extensions.DependencyInjection; +using SIL.ServiceToolkit.Services; + +namespace Serval.Translation.Services; + +public abstract class UninitializedCleanupService( + IServiceProvider services, + ILogger> logger, + TimeSpan? timeout = null +) : RecurrentTask($"{typeof(T)} Cleanup Service", services, RefreshPeriod, logger) + where T : IInitializableEntity +{ + private readonly ILogger> _logger = logger; + private readonly TimeSpan _timeout = timeout ?? TimeSpan.FromMinutes(2); + private static readonly TimeSpan RefreshPeriod = TimeSpan.FromDays(1); + + protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken) + { + _logger.LogInformation("Running build cleanup job"); + var entities = scope.ServiceProvider.GetRequiredService>(); + await CheckEntitiesAsync(entities, cancellationToken); + } + + public async Task CheckEntitiesAsync(IRepository entities, CancellationToken cancellationToken) + { + IReadOnlyList allEntities = await entities.GetAllAsync(cancellationToken); + var now = DateTime.UtcNow; + IEnumerable uninitializedEntities = allEntities.Where(b => + !(b.IsInitialized ?? true) && (now - (b.DateCreated ?? DateTime.UtcNow)) > _timeout + ); + foreach (T entity in uninitializedEntities) + { + _logger.LogInformation( + "Deleting {type} {id} because it was never successfully started", + typeof(T), + entity.Id + ); + await DeleteEntityAsync(entities, entity, cancellationToken); + } + } + + protected virtual async Task DeleteEntityAsync( + IRepository entities, + T entity, + CancellationToken cancellationToken + ) + { + await entities.DeleteAsync(e => e.Id == entity.Id, cancellationToken); + } +} diff --git a/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs index ea13b5b7..fd2ab34d 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs @@ -25,7 +25,8 @@ public TestEnvironment() { Id = "build1", EngineRef = "engine1", - SuccessfullyStarted = false + IsInitialized = false, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) } ); Builds.Add( @@ -33,7 +34,8 @@ public TestEnvironment() { Id = "build2", EngineRef = "engine2", - SuccessfullyStarted = true + IsInitialized = true, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) } ); @@ -48,7 +50,7 @@ public TestEnvironment() public async Task CheckBuildsAsync() { - await Service.CheckBuildsAsync(Builds, CancellationToken.None); + await Service.CheckEntitiesAsync(Builds, CancellationToken.None); } } } diff --git a/src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs index bb95dbf4..39848ba0 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs @@ -32,7 +32,8 @@ public TestEnvironment() TargetLanguage = "es", Type = "Nmt", Owner = "client1", - SuccessfullyCreated = false + IsInitialized = false, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) } ); Engines.Add( @@ -43,7 +44,8 @@ public TestEnvironment() TargetLanguage = "es", Type = "Nmt", Owner = "client1", - SuccessfullyCreated = true + IsInitialized = true, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) } ); @@ -61,7 +63,7 @@ public TestEnvironment() .CreateClient("Nmt") .Returns(translationServiceClient); - _engineService = new EngineService( + var engineService = new EngineService( Engines, new MemoryRepository(), new MemoryRepository(), @@ -72,15 +74,15 @@ public TestEnvironment() new LoggerFactory(), Substitute.For() ); + + Service.EngineService = engineService; } public EngineCleanupService Service { get; } - private readonly EngineService _engineService; - public async Task CheckEnginesAsync() { - await Service.CheckEnginesAsync(Engines, _engineService, CancellationToken.None); + await Service.CheckEntitiesAsync(Engines, CancellationToken.None); } private static AsyncUnaryCall CreateAsyncUnaryCall(TResponse response)