From 2fa330d522cf70e77ffc1126849fa7a25d0cc815 Mon Sep 17 00:00:00 2001 From: "Eli C. Lowry" <83078660+Enkidu93@users.noreply.github.com> Date: Wed, 4 Dec 2024 07:52:46 -0500 Subject: [PATCH 1/2] Add cleanup services (#510) Create parent class and initializable interface Collapse query Move cleanup service to serval.shared --- .../Models/IInitializableEntity.cs | 7 ++ .../Services/EntityServiceBase.cs | 2 +- .../UnitializedEntityCleanupService.cs | 55 ++++++++++++++++ .../IMongoDataAccessConfiguratorExtensions.cs | 14 +++- .../Configuration/IServalBuilderExtensions.cs | 3 + .../TranslationEnginesController.cs | 6 +- .../src/Serval.Translation/Models/Build.cs | 4 +- .../src/Serval.Translation/Models/Engine.cs | 4 +- .../Services/BuildCleanupService.cs | 7 ++ .../Services/BuildService.cs | 32 +++++++-- .../Services/EngineCleanupService.cs | 7 ++ .../Services/EngineService.cs | 66 ++++++++++++++----- .../Services/BuildCleanupServiceTests.cs | 56 ++++++++++++++++ .../Services/EngineCleanupServiceTests.cs | 62 +++++++++++++++++ 14 files changed, 298 insertions(+), 27 deletions(-) create mode 100644 src/Serval/src/Serval.Shared/Models/IInitializableEntity.cs create mode 100644 src/Serval/src/Serval.Shared/Services/UnitializedEntityCleanupService.cs create mode 100644 src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs create mode 100644 src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs create mode 100644 src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs create mode 100644 src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs 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.Shared/Services/EntityServiceBase.cs b/src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs index e506b402..b46aa87f 100644 --- a/src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs +++ b/src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs @@ -5,7 +5,7 @@ public abstract class EntityServiceBase(IRepository entities) { protected IRepository Entities { get; } = entities; - public async Task GetAsync(string id, CancellationToken cancellationToken = default) + public virtual async Task GetAsync(string id, CancellationToken cancellationToken = default) { T? entity = await Entities.GetAsync(id, cancellationToken); if (entity is null) diff --git a/src/Serval/src/Serval.Shared/Services/UnitializedEntityCleanupService.cs b/src/Serval/src/Serval.Shared/Services/UnitializedEntityCleanupService.cs new file mode 100644 index 00000000..2dc54e93 --- /dev/null +++ b/src/Serval/src/Serval.Shared/Services/UnitializedEntityCleanupService.cs @@ -0,0 +1,55 @@ +using Microsoft.Extensions.DependencyInjection; +using SIL.ServiceToolkit.Services; + +namespace Serval.Shared.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) + { + var now = DateTime.UtcNow; + IEnumerable uninitializedEntities = await entities.GetAllAsync( + e => + e.DateCreated != null + && e.DateCreated < now - _timeout + && e.IsInitialized != null + && !e.IsInitialized.Value, + cancellationToken + ); + + foreach (T entity in uninitializedEntities) + { + _logger.LogInformation( + "Deleting {type} {id} because it was never successfully initialized.", + 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/src/Serval.Translation/Configuration/IMongoDataAccessConfiguratorExtensions.cs b/src/Serval/src/Serval.Translation/Configuration/IMongoDataAccessConfiguratorExtensions.cs index ea016c0e..d0092c0b 100644 --- a/src/Serval/src/Serval.Translation/Configuration/IMongoDataAccessConfiguratorExtensions.cs +++ b/src/Serval/src/Serval.Translation/Configuration/IMongoDataAccessConfiguratorExtensions.cs @@ -15,14 +15,22 @@ this IMongoDataAccessConfigurator configurator await c.Indexes.CreateOrUpdateAsync( new CreateIndexModel(Builders.IndexKeys.Ascending(e => e.Owner)) ); + await c.Indexes.CreateOrUpdateAsync( + new CreateIndexModel(Builders.IndexKeys.Ascending(e => e.DateCreated)) + ); } ); configurator.AddRepository( "translation.builds", - init: c => - c.Indexes.CreateOrUpdateAsync( + init: async c => + { + await c.Indexes.CreateOrUpdateAsync( new CreateIndexModel(Builders.IndexKeys.Ascending(b => b.EngineRef)) - ) + ); + await c.Indexes.CreateOrUpdateAsync( + new CreateIndexModel(Builders.IndexKeys.Ascending(b => b.DateCreated)) + ); + } ); configurator.AddRepository( "translation.pretranslations", diff --git a/src/Serval/src/Serval.Translation/Configuration/IServalBuilderExtensions.cs b/src/Serval/src/Serval.Translation/Configuration/IServalBuilderExtensions.cs index 4e329863..2788ed49 100644 --- a/src/Serval/src/Serval.Translation/Configuration/IServalBuilderExtensions.cs +++ b/src/Serval/src/Serval.Translation/Configuration/IServalBuilderExtensions.cs @@ -14,6 +14,9 @@ public static IServalBuilder AddTranslation(this IServalBuilder builder) builder.Services.AddScoped(); builder.Services.AddScoped(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + var translationOptions = new TranslationOptions(); builder.Configuration.GetSection(TranslationOptions.Key).Bind(translationOptions); diff --git a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs index 9b735a01..1fa13d74 100644 --- a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs +++ b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs @@ -1318,7 +1318,8 @@ private Engine Map(TranslationEngineConfigDto source) Type = source.Type.ToPascalCase(), Owner = Owner, Corpora = [], - IsModelPersisted = source.IsModelPersisted + IsModelPersisted = source.IsModelPersisted, + IsInitialized = false }; } @@ -1331,7 +1332,8 @@ private static Build Map(Engine engine, TranslationBuildConfigDto source, string Pretranslate = Map(engine, source.Pretranslate), TrainOn = Map(engine, source.TrainOn), Options = Map(source.Options), - DeploymentVersion = deploymentVersion + DeploymentVersion = deploymentVersion, + IsInitialized = false }; } diff --git a/src/Serval/src/Serval.Translation/Models/Build.cs b/src/Serval/src/Serval.Translation/Models/Build.cs index 57162048..49168679 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; @@ -16,4 +16,6 @@ public record Build : IEntity public DateTime? DateFinished { get; init; } public IReadOnlyDictionary? Options { get; init; } public string? DeploymentVersion { 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 b4d0f55b..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,4 +16,6 @@ public record Engine : IOwnedEntity public int ModelRevision { get; init; } public double Confidence { get; init; } public int CorpusSize { 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 new file mode 100644 index 00000000..f8fa99b9 --- /dev/null +++ b/src/Serval/src/Serval.Translation/Services/BuildCleanupService.cs @@ -0,0 +1,7 @@ +namespace Serval.Translation.Services; + +public class BuildCleanupService( + IServiceProvider services, + ILogger logger, + TimeSpan? timeout = null +) : UninitializedCleanupService(services, logger, timeout) { } diff --git a/src/Serval/src/Serval.Translation/Services/BuildService.cs b/src/Serval/src/Serval.Translation/Services/BuildService.cs index e3d652d3..85b50909 100644 --- a/src/Serval/src/Serval.Translation/Services/BuildService.cs +++ b/src/Serval/src/Serval.Translation/Services/BuildService.cs @@ -4,13 +4,30 @@ public class BuildService(IRepository builds) : EntityServiceBase( { public async Task> GetAllAsync(string parentId, CancellationToken cancellationToken = default) { - return await Entities.GetAllAsync(e => e.EngineRef == parentId, cancellationToken); + return await Entities.GetAllAsync( + e => e.EngineRef == parentId && (e.IsInitialized == null || e.IsInitialized.Value), + cancellationToken + ); + } + + public override async Task GetAsync(string id, CancellationToken cancellationToken = default) + { + Build? build = await Entities.GetAsync( + e => e.Id == id && (e.IsInitialized == null || e.IsInitialized.Value), + cancellationToken + ); + if (build == null) + throw new EntityNotFoundException($"Could not find the {typeof(Build).Name} '{id}'."); + return build; } public Task GetActiveAsync(string parentId, CancellationToken cancellationToken = default) { return Entities.GetAsync( - b => b.EngineRef == parentId && (b.State == JobState.Active || b.State == JobState.Pending), + b => + b.EngineRef == parentId + && (b.IsInitialized == null || b.IsInitialized.Value) + && (b.State == JobState.Active || b.State == JobState.Pending), cancellationToken ); } @@ -21,7 +38,11 @@ public Task> GetNewerRevisionAsync( CancellationToken cancellationToken = default ) { - return GetNewerRevisionAsync(e => e.Id == id, minRevision, cancellationToken); + return GetNewerRevisionAsync( + e => e.Id == id && (e.IsInitialized == null || e.IsInitialized.Value), + minRevision, + cancellationToken + ); } public Task> GetActiveNewerRevisionAsync( @@ -31,7 +52,10 @@ public Task> GetActiveNewerRevisionAsync( ) { return GetNewerRevisionAsync( - b => b.EngineRef == parentId && (b.State == JobState.Active || b.State == JobState.Pending), + b => + b.EngineRef == parentId + && (b.IsInitialized == null || b.IsInitialized.Value) + && (b.State == JobState.Active || b.State == JobState.Pending), minRevision, cancellationToken ); diff --git a/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs b/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs new file mode 100644 index 00000000..ff4a2e8b --- /dev/null +++ b/src/Serval/src/Serval.Translation/Services/EngineCleanupService.cs @@ -0,0 +1,7 @@ +namespace Serval.Translation.Services; + +public class EngineCleanupService( + IServiceProvider services, + ILogger logger, + TimeSpan? timeout = null +) : UninitializedCleanupService(services, logger, timeout) { } diff --git a/src/Serval/src/Serval.Translation/Services/EngineService.cs b/src/Serval/src/Serval.Translation/Services/EngineService.cs index a8bb3a05..5e653059 100644 --- a/src/Serval/src/Serval.Translation/Services/EngineService.cs +++ b/src/Serval/src/Serval.Translation/Services/EngineService.cs @@ -24,6 +24,25 @@ IScriptureDataFileService scriptureDataFileService private readonly ILogger _logger = loggerFactory.CreateLogger(); private readonly IScriptureDataFileService _scriptureDataFileService = scriptureDataFileService; + public override async Task GetAsync(string id, CancellationToken cancellationToken = default) + { + Engine engine = await base.GetAsync(id, cancellationToken); + if (!(engine.IsInitialized ?? true)) + throw new EntityNotFoundException($"Could not find the {typeof(Engine).Name} '{id}'."); + return engine; + } + + public override async Task> GetAllAsync( + string owner, + CancellationToken cancellationToken = default + ) + { + return await Entities.GetAllAsync( + e => e.Owner == owner && (e.IsInitialized == null || e.IsInitialized.Value), + cancellationToken + ); + } + public async Task TranslateAsync( string engineId, string segment, @@ -120,9 +139,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); @@ -146,6 +165,15 @@ public override async Task CreateAsync(Engine engine, CancellationToken { IsModelPersisted = createResponse.IsModelPersisted }; + await Entities.UpdateAsync( + engine, + u => + { + u.Set(e => e.IsInitialized, true); + u.Set(e => e.IsModelPersisted, engine.IsModelPersisted); + }, + cancellationToken: CancellationToken.None + ); } catch (RpcException rpcex) { @@ -164,14 +192,6 @@ public override async Task CreateAsync(Engine engine, CancellationToken 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; } @@ -216,6 +236,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); @@ -325,6 +346,11 @@ pretranslate is null _logger.LogInformation("{request}", JsonSerializer.Serialize(request)); } await client.StartBuildAsync(request, cancellationToken: cancellationToken); + await _builds.UpdateAsync( + b => b.Id == build.Id, + u => u.Set(e => e.IsInitialized, true), + cancellationToken: CancellationToken.None + ); } catch { @@ -382,7 +408,11 @@ public async Task GetModelDownloadUrlAsync( public Task AddCorpusAsync(string engineId, Models.Corpus corpus, CancellationToken cancellationToken = default) { - return Entities.UpdateAsync(engineId, u => u.Add(e => e.Corpora, corpus), cancellationToken: cancellationToken); + return Entities.UpdateAsync( + e => e.Id == engineId && (e.IsInitialized == null || e.IsInitialized.Value), + u => u.Add(e => e.Corpora, corpus), + cancellationToken: cancellationToken + ); } public async Task UpdateCorpusAsync( @@ -394,7 +424,10 @@ public Task AddCorpusAsync(string engineId, Models.Corpus corpus, CancellationTo ) { Engine? engine = await Entities.UpdateAsync( - e => e.Id == engineId && e.Corpora.Any(c => c.Id == corpusId), + e => + e.Id == engineId + && (e.IsInitialized == null || e.IsInitialized.Value) + && e.Corpora.Any(c => c.Id == corpusId), u => { if (sourceFiles is not null) @@ -421,7 +454,7 @@ await _dataAccessContext.WithTransactionAsync( async (ct) => { originalEngine = await Entities.UpdateAsync( - engineId, + e => e.Id == engineId && (e.IsInitialized == null || e.IsInitialized.Value), u => u.RemoveAll(e => e.Corpora, c => c.Id == corpusId), returnOriginal: true, cancellationToken: ct @@ -456,7 +489,7 @@ public Task AddParallelCorpusAsync( ) { return Entities.UpdateAsync( - engineId, + e => e.Id == engineId && (e.IsInitialized == null || e.IsInitialized.Value), u => u.Add(e => e.ParallelCorpora, corpus), cancellationToken: cancellationToken ); @@ -471,7 +504,10 @@ public Task AddParallelCorpusAsync( ) { Engine? engine = await Entities.UpdateAsync( - e => e.Id == engineId && e.ParallelCorpora.Any(c => c.Id == parallelCorpusId), + e => + e.Id == engineId + && (e.IsInitialized == null || e.IsInitialized.Value) + && e.ParallelCorpora.Any(c => c.Id == parallelCorpusId), u => { if (sourceCorpora is not null) @@ -502,7 +538,7 @@ await _dataAccessContext.WithTransactionAsync( async (ct) => { originalEngine = await Entities.UpdateAsync( - engineId, + e => e.Id == engineId && (e.IsInitialized == null || e.IsInitialized.Value), u => u.RemoveAll(e => e.ParallelCorpora, c => c.Id == parallelCorpusId), returnOriginal: true, cancellationToken: ct diff --git a/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs new file mode 100644 index 00000000..fd2ab34d --- /dev/null +++ b/src/Serval/test/Serval.Translation.Tests/Services/BuildCleanupServiceTests.cs @@ -0,0 +1,56 @@ +namespace Serval.Translation.Services; + +[TestFixture] +public class BuildCleanupServiceTests +{ + [Test] + public async Task CleanupAsync() + { + TestEnvironment env = new(); + Assert.That(env.Builds.Count, Is.EqualTo(2)); + await env.CheckBuildsAsync(); + Assert.That(env.Builds.Count, Is.EqualTo(1)); + Assert.That((await env.Builds.GetAllAsync())[0].Id, Is.EqualTo("build2")); + } + + private class TestEnvironment + { + public MemoryRepository Builds { get; } + + public TestEnvironment() + { + Builds = new MemoryRepository(); + Builds.Add( + new Build + { + Id = "build1", + EngineRef = "engine1", + IsInitialized = false, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) + } + ); + Builds.Add( + new Build + { + Id = "build2", + EngineRef = "engine2", + IsInitialized = true, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) + } + ); + + Service = new BuildCleanupService( + Substitute.For(), + Substitute.For>(), + TimeSpan.Zero + ); + } + + public BuildCleanupService Service { get; } + + public async Task CheckBuildsAsync() + { + 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 new file mode 100644 index 00000000..b4ae29e4 --- /dev/null +++ b/src/Serval/test/Serval.Translation.Tests/Services/EngineCleanupServiceTests.cs @@ -0,0 +1,62 @@ +namespace Serval.Translation.Services; + +[TestFixture] +public class EngineCleanupServiceTests +{ + [Test] + public async Task CleanupAsync() + { + TestEnvironment env = new(); + Assert.That(env.Engines.Count, Is.EqualTo(2)); + await env.CheckEnginesAsync(); + Assert.That(env.Engines.Count, Is.EqualTo(1)); + Assert.That((await env.Engines.GetAllAsync())[0].Id, Is.EqualTo("engine2")); + } + + private class TestEnvironment + { + public MemoryRepository Engines { get; } + + public TestEnvironment() + { + Engines = new MemoryRepository(); + Engines.Add( + new Engine + { + Id = "engine1", + SourceLanguage = "en", + TargetLanguage = "es", + Type = "Nmt", + Owner = "client1", + IsInitialized = false, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) + } + ); + Engines.Add( + new Engine + { + Id = "engine2", + SourceLanguage = "en", + TargetLanguage = "es", + Type = "Nmt", + Owner = "client1", + IsInitialized = true, + DateCreated = DateTime.UtcNow.Subtract(TimeSpan.FromHours(10)) + } + ); + + Service = new EngineCleanupService( + Substitute.For(), + Substitute.For>(), + TimeSpan.Zero + ); + } + + public EngineCleanupService Service { get; } + + public async Task CheckEnginesAsync() + { + await Service.CheckEntitiesAsync(Engines, CancellationToken.None); + } + } +} From 6d2262158112f8585a240e488f8677d8aa98de2b Mon Sep 17 00:00:00 2001 From: John Lambert Date: Thu, 5 Dec 2024 08:56:16 -0500 Subject: [PATCH 2/2] Update email server to Zoho --- deploy/serval/templates/alert-manager-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/serval/templates/alert-manager-config.yaml b/deploy/serval/templates/alert-manager-config.yaml index 2fe6887a..3a675faf 100644 --- a/deploy/serval/templates/alert-manager-config.yaml +++ b/deploy/serval/templates/alert-manager-config.yaml @@ -13,7 +13,7 @@ spec: from: {{ .Values.alertEmail }} requireTLS: true sendResolved: true - smarthost: mail.languagetechnology.org:587 + smarthost: smtppro.zoho.com:587 tlsConfig: {} to: {{ .Values.emailsToAlert }} name: alert-serval