Nmt download (#164)

* Presigned URL code cleaning script IsModelPersisted nullable Return IsModelPersistedState to Serval Check for modelRevision + 1 in cleanup and just delete without keeping internal states. * Reviewer comments * update to most recent api
sillsdev · Feb 9, 2024 · 525950f · 525950f
1 parent 32a8471
commit 525950f
Show file tree

Hide file tree

Showing 24 changed files with 437 additions and 105 deletions.
diff --git a/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs b/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs
@@ -151,6 +151,21 @@ private static IMachineBuilder AddHangfireBuildJobRunner(this IMachineBuilder bu
         return builder;
     }
 
+    private static MongoStorageOptions GetMongoStorageOptions()
+    {
+        var mongoStorageOptions = new MongoStorageOptions
+        {
+            MigrationOptions = new MongoMigrationOptions
+            {
+                MigrationStrategy = new MigrateMongoMigrationStrategy(),
+                BackupStrategy = new CollectionMongoBackupStrategy()
+            },
+            CheckConnection = true,
+            CheckQueuedJobsStrategy = CheckQueuedJobsStrategy.TailNotificationsCollection,
+        };
+        return mongoStorageOptions;
+    }
+
     public static IMachineBuilder AddMongoHangfireJobClient(
         this IMachineBuilder builder,
         string? connectionString = null
@@ -164,19 +179,7 @@ public static IMachineBuilder AddMongoHangfireJobClient(
             c.SetDataCompatibilityLevel(CompatibilityLevel.Version_170)
                 .UseSimpleAssemblyNameTypeSerializer()
                 .UseRecommendedSerializerSettings()
-                .UseMongoStorage(
-                    connectionString,
-                    new MongoStorageOptions
-                    {
-                        MigrationOptions = new MongoMigrationOptions
-                        {
-                            MigrationStrategy = new MigrateMongoMigrationStrategy(),
-                            BackupStrategy = new CollectionMongoBackupStrategy()
-                        },
-                        CheckConnection = true,
-                        CheckQueuedJobsStrategy = CheckQueuedJobsStrategy.TailNotificationsCollection,
-                    }
-                )
+                .UseMongoStorage(connectionString, GetMongoStorageOptions())
                 .UseFilter(new AutomaticRetryAttribute { Attempts = 0 })
         );
         builder.Services.AddHealthChecks().AddCheck<HangfireHealthCheck>(name: "Hangfire");
@@ -402,6 +405,12 @@ public static IMachineBuilder AddBuildJobService(this IMachineBuilder builder)
         return builder;
     }
 
+    public static IMachineBuilder AddModelCleanupService(this IMachineBuilder builder)
+    {
+        builder.Services.AddHostedService<ModelCleanupService>();
+        return builder;
+    }
+
     private static IMachineBuilder AddBuildJobService(this IMachineBuilder builder, BuildJobOptions options)
     {
         builder.Services.AddScoped<IBuildJobService, BuildJobService>();

diff --git a/src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs b/src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs
@@ -0,0 +1,11 @@
+using System;
+
+namespace SIL.Machine.AspNetCore.Models
+{
+    public class ModelDownloadUrl
+    {
+        public string Url { get; set; } = default!;
+        public int ModelRevision { get; set; } = default!;
+        public DateTime ExipiresAt { get; set; } = default!;
+    }
+}
diff --git a/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs b/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs
@@ -7,6 +7,7 @@ public class TranslationEngine : IEntity
     public string EngineId { get; set; } = default!;
     public string SourceLanguage { get; set; } = default!;
     public string TargetLanguage { get; set; } = default!;
+    public bool IsModelPersisted { get; set; }
     public int BuildRevision { get; set; }
     public Build? CurrentBuild { get; set; }
 }
diff --git a/src/SIL.Machine.AspNetCore/SIL.Machine.AspNetCore.csproj b/src/SIL.Machine.AspNetCore/SIL.Machine.AspNetCore.csproj
@@ -35,7 +35,7 @@
 		<PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="6.0.16" />
 		<PackageReference Include="Microsoft.Extensions.Http.Polly" Version="6.0.14" />
 		<PackageReference Include="Python.Included" Version="3.11.4" />
-		<PackageReference Include="Serval.Grpc" Version="0.15.0" Condition="!Exists('..\..\..\serval\src\Serval.Grpc\Serval.Grpc.csproj')" />
+		<PackageReference Include="Serval.Grpc" Version="0.16.0" Condition="!Exists('..\..\..\serval\src\Serval.Grpc\Serval.Grpc.csproj')" />
 		<PackageReference Include="SIL.DataAccess" Version="0.5.2" Condition="!Exists('..\..\..\serval\src\SIL.DataAccess\SIL.DataAccess.csproj')" />
 		<PackageReference Include="SIL.WritingSystems" Version="12.0.1" />
 		<PackageReference Include="System.Linq.Async" Version="6.0.1" />

diff --git a/src/SIL.Machine.AspNetCore/Services/HangfireHealthCheck.cs b/src/SIL.Machine.AspNetCore/Services/HangfireHealthCheck.cs
@@ -1,15 +1,9 @@
 namespace SIL.Machine.AspNetCore.Services;
 
-public class HangfireHealthCheck : IHealthCheck
+public class HangfireHealthCheck(JobStorage jobStorage, IOptions<BackgroundJobServerOptions> options) : IHealthCheck
 {
-    private readonly JobStorage _jobStorage;
-    private readonly IOptions<BackgroundJobServerOptions> _options;
-
-    public HangfireHealthCheck(JobStorage jobStorage, IOptions<BackgroundJobServerOptions> options)
-    {
-        _jobStorage = jobStorage;
-        _options = options;
-    }
+    private readonly JobStorage _jobStorage = jobStorage;
+    private readonly IOptions<BackgroundJobServerOptions> _options = options;
 
     public Task<HealthCheckResult> CheckHealthAsync(
         HealthCheckContext context,

diff --git a/src/SIL.Machine.AspNetCore/Services/IFileStorage.cs b/src/SIL.Machine.AspNetCore/Services/IFileStorage.cs
@@ -14,5 +14,7 @@ Task<IReadOnlyCollection<string>> ListFilesAsync(
 
     Task<Stream> OpenWriteAsync(string path, CancellationToken cancellationToken = default);
 
+    Task<string> GetDownloadUrlAsync(string path, DateTime expiresAt, CancellationToken cancellationToken = default);
+
     Task DeleteAsync(string path, bool recurse = false, CancellationToken cancellationToken = default);
 }
diff --git a/src/SIL.Machine.AspNetCore/Services/ISharedFileService.cs b/src/SIL.Machine.AspNetCore/Services/ISharedFileService.cs
@@ -6,6 +6,14 @@ public interface ISharedFileService
 
     Uri GetResolvedUri(string path);
 
+    Task<string> GetDownloadUrlAsync(string path, DateTime expiresAt);
+
+    Task<IReadOnlyCollection<string>> ListFilesAsync(
+        string path,
+        bool recurse = false,
+        CancellationToken cancellationToken = default
+    );
+
     Task<Stream> OpenReadAsync(string path, CancellationToken cancellationToken = default);
 
     Task<Stream> OpenWriteAsync(string path, CancellationToken cancellationToken = default);

diff --git a/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs b/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs
@@ -4,11 +4,12 @@ public interface ITranslationEngineService
 {
     TranslationEngineType Type { get; }
 
-    Task CreateAsync(
+    Task<TranslationEngine> CreateAsync(
         string engineId,
         string? engineName,
         string sourceLanguage,
         string targetLanguage,
+        bool? isModelPersisted = null,
         CancellationToken cancellationToken = default
     );
     Task DeleteAsync(string engineId, CancellationToken cancellationToken = default);
@@ -40,6 +41,8 @@ Task StartBuildAsync(
 
     Task CancelBuildAsync(string engineId, CancellationToken cancellationToken = default);
 
+    Task<ModelDownloadUrl> GetModelDownloadUrlAsync(string engineId, CancellationToken cancellationToken = default);
+
     Task<int> GetQueueSizeAsync(CancellationToken cancellationToken = default);
 
     bool IsLanguageNativeToModel(string language, out string internalCode);

diff --git a/src/SIL.Machine.AspNetCore/Services/InMemoryStorage.cs b/src/SIL.Machine.AspNetCore/Services/InMemoryStorage.cs
@@ -96,6 +96,15 @@ public Task<IReadOnlyCollection<string>> ListFilesAsync(
         );
     }
 
+    public Task<string> GetDownloadUrlAsync(
+        string path,
+        DateTime expiresAt,
+        CancellationToken cancellationToken = default
+    )
+    {
+        throw new NotSupportedException();
+    }
+
     public Task<Stream> OpenReadAsync(string path, CancellationToken cancellationToken = default)
     {
         if (!_memoryStreams.TryGetValue(Normalize(path), out Entry? ret))

diff --git a/src/SIL.Machine.AspNetCore/Services/LocalStorage.cs b/src/SIL.Machine.AspNetCore/Services/LocalStorage.cs
@@ -36,6 +36,15 @@ public Task<IReadOnlyCollection<string>> ListFilesAsync(
         );
     }
 
+    public Task<string> GetDownloadUrlAsync(
+        string path,
+        DateTime expiresAt,
+        CancellationToken cancellationToken = default
+    )
+    {
+        throw new NotSupportedException();
+    }
+
     public Task<Stream> OpenReadAsync(string path, CancellationToken cancellationToken = default)
     {
         Uri pathUri = new(_basePath, Normalize(path));

diff --git a/src/SIL.Machine.AspNetCore/Services/ModelCleanupService.cs b/src/SIL.Machine.AspNetCore/Services/ModelCleanupService.cs
@@ -0,0 +1,59 @@
+namespace SIL.Machine.AspNetCore.Services;
+
+public class ModelCleanupService(
+    IServiceProvider services,
+    ISharedFileService sharedFileService,
+    IRepository<TranslationEngine> engines,
+    ILogger<ModelCleanupService> logger
+) : RecurrentTask("Model Cleanup Service", services, RefreshPeriod, logger)
+{
+    private readonly ISharedFileService _sharedFileService = sharedFileService;
+    private readonly ILogger<ModelCleanupService> _logger = logger;
+    private readonly IRepository<TranslationEngine> _engines = engines;
+    private static readonly TimeSpan RefreshPeriod = TimeSpan.FromDays(1);
+
+    protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken)
+    {
+        await CheckModelsAsync(cancellationToken);
+    }
+
+    private async Task CheckModelsAsync(CancellationToken cancellationToken)
+    {
+        _logger.LogInformation("Running model cleanup job");
+        IReadOnlyCollection<string> paths = await _sharedFileService.ListFilesAsync(
+            NmtEngineService.ModelDirectory,
+            cancellationToken: cancellationToken
+        );
+        // Get all engine ids from the database
+        IReadOnlyList<TranslationEngine>? allEngines = await _engines.GetAllAsync(cancellationToken: cancellationToken);
+        IEnumerable<string> validFilenames = allEngines.Select(e =>
+            NmtEngineService.GetModelPath(e.EngineId, e.BuildRevision)
+        );
+        // If there is a currently running build that creates and pushes a new file, but the database has not
+        // updated yet, don't delete the new file.
+        IEnumerable<string> validFilenamesForNextBuild = allEngines.Select(e =>
+            NmtEngineService.GetModelPath(e.EngineId, e.BuildRevision + 1)
+        );
+        HashSet<string> filenameFilter = validFilenames.Concat(validFilenamesForNextBuild).ToHashSet();
+
+        foreach (string path in paths)
+        {
+            if (!filenameFilter.Contains(path))
+            {
+                await DeleteFileAsync(
+                    path,
+                    $"file in S3 bucket not found in database.  It may be an old rev, etc.",
+                    cancellationToken
+                );
+            }
+        }
+    }
+
+    private async Task DeleteFileAsync(string path, string message, CancellationToken cancellationToken = default)
+    {
+        // This may delete a file while it is being downloaded, but the chance is rare
+        // enough and the solution easy enough (just download again) to just live with it.
+        _logger.LogInformation("Deleting old model file {filename}: {message}", path, message);
+        await _sharedFileService.DeleteAsync(path, cancellationToken);
+    }
+}
diff --git a/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs b/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs
@@ -1,24 +1,16 @@
 namespace SIL.Machine.AspNetCore.Services;
 
-public class NmtClearMLBuildJobFactory : IClearMLBuildJobFactory
+public class NmtClearMLBuildJobFactory(
+    ISharedFileService sharedFileService,
+    ILanguageTagService languageTagService,
+    IRepository<TranslationEngine> engines,
+    IOptionsMonitor<ClearMLOptions> options
+) : IClearMLBuildJobFactory
 {
-    private readonly ISharedFileService _sharedFileService;
-    private readonly ILanguageTagService _languageTagService;
-    private readonly IRepository<TranslationEngine> _engines;
-    private readonly IOptionsMonitor<ClearMLOptions> _options;
-
-    public NmtClearMLBuildJobFactory(
-        ISharedFileService sharedFileService,
-        ILanguageTagService languageTagService,
-        IRepository<TranslationEngine> engines,
-        IOptionsMonitor<ClearMLOptions> options
-    )
-    {
-        _sharedFileService = sharedFileService;
-        _languageTagService = languageTagService;
-        _engines = engines;
-        _options = options;
-    }
+    private readonly ISharedFileService _sharedFileService = sharedFileService;
+    private readonly ILanguageTagService _languageTagService = languageTagService;
+    private readonly IRepository<TranslationEngine> _engines = engines;
+    private readonly IOptionsMonitor<ClearMLOptions> _options = options;
 
     public TranslationEngineType EngineType => TranslationEngineType.Nmt;
 
@@ -52,6 +44,9 @@ public async Task<string> CreateJobScriptAsync(
                 + $"    'shared_file_uri': '{baseUri}',\n"
                 + $"    'shared_file_folder': '{folder}',\n"
                 + (buildOptions is not null ? $"    'build_options': '''{buildOptions}''',\n" : "")
+                // buildRevision + 1 because the build revision is incremented after the build job
+                // is finished successfully but the file should be saved with the new revision number
+                + (engine.IsModelPersisted ? $"    'save_model': '{engineId}_{engine.BuildRevision + 1}',\n" : $"")
                 + $"    'clearml': True\n"
                 + "}\n"
                 + "run(args)\n";