diff --git a/.github/workflows/ci-e2e.yml b/.github/workflows/ci-e2e.yml index ace74939..472e33d0 100644 --- a/.github/workflows/ci-e2e.yml +++ b/.github/workflows/ci-e2e.yml @@ -30,8 +30,15 @@ jobs: - name: Install regctl uses: iarekylew00t/regctl-installer@v1 - - name: Getr Version of Machine.py - run: echo "MACHINE_PY_IMAGE=ghcr.io/sillsdev/machine.py:$(regctl image config ghcr.io/sillsdev/machine.py | jq -r ".config.Labels[\"org.opencontainers.image.version\"]")" >> $GITHUB_ENV + - name: Set proper version of Machine.py + run: | + export MACHINE_PY_IMAGE=ghcr.io/sillsdev/machine.py:$(regctl image config ghcr.io/sillsdev/machine.py | jq -r ".config.Labels[\"org.opencontainers.image.version\"]") && \ + echo "MACHINE_PY_IMAGE=$MACHINE_PY_IMAGE" >> $GITHUB_ENV && \ + echo "MACHINE_PY_CPU_IMAGE=$MACHINE_PY_IMAGE.cpu_only" >> $GITHUB_ENV + + - name: Confirm proper version of Machine.py + run: | + echo $MACHINE_PY_IMAGE $MACHINE_PY_CPU_IMAGE - name: Setup .NET uses: actions/setup-dotnet@v3 @@ -50,6 +57,9 @@ jobs: - name: Test run: dotnet test --no-build --verbosity normal --filter "TestCategory!=slow&TestCategory=E2E" --collect:"Xplat Code Coverage" + - name: Debug network again + run: docker ps -a && docker logs --since 10m serval_cntr && docker logs --since 10m echo_cntr && docker logs --since 10m machine-engine-cntr && docker logs --since 10m serval-mongo-1 && docker logs --since 10m machine-job-cntr + - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v3 env: diff --git a/src/Machine/src/Serval.Machine.Shared/Configuration/BuildJobOptions.cs b/src/Machine/src/Serval.Machine.Shared/Configuration/BuildJobOptions.cs index 547a9dbd..0a7947d5 100644 --- a/src/Machine/src/Serval.Machine.Shared/Configuration/BuildJobOptions.cs +++ b/src/Machine/src/Serval.Machine.Shared/Configuration/BuildJobOptions.cs @@ -5,4 +5,5 @@ public class BuildJobOptions public const string Key = "BuildJob"; public IList ClearML { get; set; } = new List(); + public TimeSpan PostProcessLockLifetime { get; set; } = TimeSpan.FromSeconds(120); } diff --git a/src/Machine/src/Serval.Machine.Shared/Configuration/DistributedReaderWriterLockOptions.cs b/src/Machine/src/Serval.Machine.Shared/Configuration/DistributedReaderWriterLockOptions.cs new file mode 100644 index 00000000..62817dbc --- /dev/null +++ b/src/Machine/src/Serval.Machine.Shared/Configuration/DistributedReaderWriterLockOptions.cs @@ -0,0 +1,8 @@ +namespace Serval.Machine.Shared.Configuration; + +public class DistributedReaderWriterLockOptions +{ + public const string Key = "DistributedReaderWriterLock"; + + public TimeSpan DefaultLifetime { get; set; } = TimeSpan.FromSeconds(56); // must be less than DefaultHttpRequestTimeout +} diff --git a/src/Machine/src/Serval.Machine.Shared/Configuration/IMachineBuilderExtensions.cs b/src/Machine/src/Serval.Machine.Shared/Configuration/IMachineBuilderExtensions.cs index d67afb90..4f60ae90 100644 --- a/src/Machine/src/Serval.Machine.Shared/Configuration/IMachineBuilderExtensions.cs +++ b/src/Machine/src/Serval.Machine.Shared/Configuration/IMachineBuilderExtensions.cs @@ -49,6 +49,24 @@ public static IMachineBuilder AddClearMLOptions(this IMachineBuilder builder, IC return builder; } + public static IMachineBuilder AddDistributedReaderWriterLockOptions( + this IMachineBuilder build, + Action configureOptions + ) + { + build.Services.Configure(configureOptions); + return build; + } + + public static IMachineBuilder AddDistributedReaderWriterLockOptions( + this IMachineBuilder build, + IConfiguration config + ) + { + build.Services.Configure(config); + return build; + } + public static IMachineBuilder AddMessageOutboxOptions( this IMachineBuilder builder, Action configureOptions diff --git a/src/Machine/src/Serval.Machine.Shared/Configuration/IServiceCollectionExtensions.cs b/src/Machine/src/Serval.Machine.Shared/Configuration/IServiceCollectionExtensions.cs index 7463e6ac..9ae176d8 100644 --- a/src/Machine/src/Serval.Machine.Shared/Configuration/IServiceCollectionExtensions.cs +++ b/src/Machine/src/Serval.Machine.Shared/Configuration/IServiceCollectionExtensions.cs @@ -28,6 +28,7 @@ public static IMachineBuilder AddMachine(this IServiceCollection services, IConf builder.AddSharedFileOptions(o => { }); builder.AddSmtTransferEngineOptions(o => { }); builder.AddClearMLOptions(o => { }); + builder.AddDistributedReaderWriterLockOptions(o => { }); builder.AddBuildJobOptions(o => { }); builder.AddMessageOutboxOptions(o => { }); } @@ -37,6 +38,9 @@ public static IMachineBuilder AddMachine(this IServiceCollection services, IConf builder.AddSharedFileOptions(configuration.GetSection(SharedFileOptions.Key)); builder.AddSmtTransferEngineOptions(configuration.GetSection(SmtTransferEngineOptions.Key)); builder.AddClearMLOptions(configuration.GetSection(ClearMLOptions.Key)); + builder.AddDistributedReaderWriterLockOptions( + configuration.GetSection(DistributedReaderWriterLockOptions.Key) + ); builder.AddBuildJobOptions(configuration.GetSection(BuildJobOptions.Key)); builder.AddMessageOutboxOptions(configuration.GetSection(MessageOutboxOptions.Key)); } diff --git a/src/Machine/src/Serval.Machine.Shared/Models/Lock.cs b/src/Machine/src/Serval.Machine.Shared/Models/Lock.cs index 39ceae87..505126d2 100644 --- a/src/Machine/src/Serval.Machine.Shared/Models/Lock.cs +++ b/src/Machine/src/Serval.Machine.Shared/Models/Lock.cs @@ -3,6 +3,7 @@ public record Lock { public required string Id { get; init; } - public DateTime? ExpiresAt { get; init; } + + public DateTime ExpiresAt { get; init; } public required string HostId { get; init; } } diff --git a/src/Machine/src/Serval.Machine.Shared/Models/RWLock.cs b/src/Machine/src/Serval.Machine.Shared/Models/RWLock.cs index 2271aa9b..1a462602 100644 --- a/src/Machine/src/Serval.Machine.Shared/Models/RWLock.cs +++ b/src/Machine/src/Serval.Machine.Shared/Models/RWLock.cs @@ -11,15 +11,14 @@ public record RWLock : IEntity public bool IsAvailableForReading() { var now = DateTime.UtcNow; - return (WriterLock is null || WriterLock.ExpiresAt is not null && WriterLock.ExpiresAt <= now) - && WriterQueue.Count == 0; + return (WriterLock is null || WriterLock.ExpiresAt <= now) && WriterQueue.Count == 0; } public bool IsAvailableForWriting(string? lockId = null) { var now = DateTime.UtcNow; - return (WriterLock is null || WriterLock.ExpiresAt is not null && WriterLock.ExpiresAt <= now) - && !ReaderLocks.Any(l => l.ExpiresAt is null || l.ExpiresAt > now) + return (WriterLock is null || WriterLock.ExpiresAt <= now) + && !ReaderLocks.Any(l => l.ExpiresAt > now) && (lockId is null || WriterQueue.Count > 0 && WriterQueue[0].Id == lockId); } } diff --git a/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj b/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj index 4ea74e68..c24b9f0b 100644 --- a/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj +++ b/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj @@ -46,6 +46,7 @@ + diff --git a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs index 7ea8679f..6dfea687 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs @@ -1,12 +1,18 @@ namespace Serval.Machine.Shared.Services; -public class DistributedReaderWriterLock(string hostId, IRepository locks, IIdGenerator idGenerator, string id) - : IDistributedReaderWriterLock +public class DistributedReaderWriterLock( + string hostId, + IRepository locks, + IIdGenerator idGenerator, + string id, + DistributedReaderWriterLockOptions lockOptions +) : IDistributedReaderWriterLock { private readonly string _hostId = hostId; private readonly IRepository _locks = locks; private readonly IIdGenerator _idGenerator = idGenerator; private readonly string _id = id; + private readonly DistributedReaderWriterLockOptions _lockOptions = lockOptions; public async Task ReaderLockAsync( TimeSpan? lifetime = default, @@ -14,7 +20,8 @@ public async Task ReaderLockAsync( ) { string lockId = _idGenerator.GenerateId(); - if (!await TryAcquireReaderLock(lockId, lifetime, cancellationToken)) + TimeSpan resolvedLifetime = lifetime ?? _lockOptions.DefaultLifetime; + if (!await TryAcquireReaderLock(lockId, resolvedLifetime, cancellationToken)) { using ISubscription sub = await _locks.SubscribeAsync(rwl => rwl.Id == _id, cancellationToken); do @@ -32,7 +39,7 @@ public async Task ReaderLockAsync( if (timeout != TimeSpan.Zero) await sub.WaitForChangeAsync(timeout, cancellationToken); } - } while (!await TryAcquireReaderLock(lockId, lifetime, cancellationToken)); + } while (!await TryAcquireReaderLock(lockId, resolvedLifetime, cancellationToken)); } return new ReaderLockReleaser(this, lockId); } @@ -43,11 +50,12 @@ public async Task WriterLockAsync( ) { string lockId = _idGenerator.GenerateId(); - if (!await TryAcquireWriterLock(lockId, lifetime, cancellationToken)) + TimeSpan resolvedLifetime = lifetime ?? _lockOptions.DefaultLifetime; + if (!await TryAcquireWriterLock(lockId, resolvedLifetime, cancellationToken)) { await _locks.UpdateAsync( _id, - u => u.Add(rwl => rwl.WriterQueue, new Lock { Id = lockId, HostId = _hostId }), + u => u.Add(rwl => rwl.WriterQueue, new Lock { Id = lockId, HostId = _hostId, }), cancellationToken: cancellationToken ); try @@ -58,12 +66,9 @@ await _locks.UpdateAsync( RWLock? rwLock = sub.Change.Entity; if (rwLock is not null && !rwLock.IsAvailableForWriting(lockId)) { - var dateTimes = rwLock - .ReaderLocks.Where(l => l.ExpiresAt.HasValue) - .Select(l => l.ExpiresAt.GetValueOrDefault()) - .ToList(); + var dateTimes = rwLock.ReaderLocks.Select(l => l.ExpiresAt).ToList(); if (rwLock.WriterLock?.ExpiresAt is not null) - dateTimes.Add(rwLock.WriterLock.ExpiresAt.Value); + dateTimes.Add(rwLock.WriterLock.ExpiresAt); TimeSpan? timeout = default; if (dateTimes.Count > 0) { @@ -74,7 +79,7 @@ await _locks.UpdateAsync( if (timeout != TimeSpan.Zero) await sub.WaitForChangeAsync(timeout, cancellationToken); } - } while (!await TryAcquireWriterLock(lockId, lifetime, cancellationToken)); + } while (!await TryAcquireWriterLock(lockId, resolvedLifetime, cancellationToken)); } catch { @@ -89,17 +94,13 @@ await _locks.UpdateAsync( return new WriterLockReleaser(this, lockId); } - private async Task TryAcquireWriterLock( - string lockId, - TimeSpan? lifetime, - CancellationToken cancellationToken - ) + private async Task TryAcquireWriterLock(string lockId, TimeSpan lifetime, CancellationToken cancellationToken) { var now = DateTime.UtcNow; Expression> filter = rwl => rwl.Id == _id - && (rwl.WriterLock == null || rwl.WriterLock.ExpiresAt != null && rwl.WriterLock.ExpiresAt <= now) - && !rwl.ReaderLocks.Any(l => l.ExpiresAt == null || l.ExpiresAt > now) + && (rwl.WriterLock == null || rwl.WriterLock.ExpiresAt <= now) + && !rwl.ReaderLocks.Any(l => l.ExpiresAt > now) && (!rwl.WriterQueue.Any() || rwl.WriterQueue[0].Id == lockId); void Update(IUpdateBuilder u) { @@ -108,7 +109,7 @@ void Update(IUpdateBuilder u) new Lock { Id = lockId, - ExpiresAt = lifetime is null ? null : now + lifetime, + ExpiresAt = now + lifetime, HostId = _hostId } ); @@ -118,17 +119,11 @@ void Update(IUpdateBuilder u) return rwLock is not null; } - private async Task TryAcquireReaderLock( - string lockId, - TimeSpan? lifetime, - CancellationToken cancellationToken - ) + private async Task TryAcquireReaderLock(string lockId, TimeSpan lifetime, CancellationToken cancellationToken) { var now = DateTime.UtcNow; Expression> filter = rwl => - rwl.Id == _id - && (rwl.WriterLock == null || rwl.WriterLock.ExpiresAt != null && rwl.WriterLock.ExpiresAt <= now) - && !rwl.WriterQueue.Any(); + rwl.Id == _id && (rwl.WriterLock == null || rwl.WriterLock.ExpiresAt <= now) && !rwl.WriterQueue.Any(); void Update(IUpdateBuilder u) { u.Add( @@ -136,7 +131,7 @@ void Update(IUpdateBuilder u) new Lock { Id = lockId, - ExpiresAt = lifetime is null ? null : now + lifetime, + ExpiresAt = now + lifetime, HostId = _hostId } ); diff --git a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs index 81810fb1..e0d44795 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs @@ -2,11 +2,13 @@ public class DistributedReaderWriterLockFactory( IOptions serviceOptions, + IOptions lockOptions, IRepository locks, IIdGenerator idGenerator ) : IDistributedReaderWriterLockFactory { private readonly ServiceOptions _serviceOptions = serviceOptions.Value; + private readonly DistributedReaderWriterLockOptions _lockOptions = lockOptions.Value; private readonly IIdGenerator _idGenerator = idGenerator; private readonly IRepository _locks = locks; @@ -39,7 +41,7 @@ await _locks.InsertAsync( // the lock is already made - no new one needs to be made // This is done instead of checking if it exists first to prevent race conditions. } - return new DistributedReaderWriterLock(_serviceOptions.ServiceId, _locks, _idGenerator, id); + return new DistributedReaderWriterLock(_serviceOptions.ServiceId, _locks, _idGenerator, id, _lockOptions); } public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) diff --git a/src/Machine/src/Serval.Machine.Shared/Services/PostprocessBuildJob.cs b/src/Machine/src/Serval.Machine.Shared/Services/PostprocessBuildJob.cs index 25e34892..f208161c 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/PostprocessBuildJob.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/PostprocessBuildJob.cs @@ -7,10 +7,12 @@ public class PostprocessBuildJob( IDataAccessContext dataAccessContext, IBuildJobService buildJobService, ILogger logger, - ISharedFileService sharedFileService + ISharedFileService sharedFileService, + IOptionsMonitor buildJobOptions ) : HangfireBuildJob<(int, double)>(platformService, engines, lockFactory, dataAccessContext, buildJobService, logger) { protected ISharedFileService SharedFileService { get; } = sharedFileService; + private readonly BuildJobOptions _buildJobOptions = buildJobOptions.CurrentValue; protected override async Task DoWorkAsync( string engineId, @@ -33,7 +35,12 @@ CancellationToken cancellationToken await PlatformService.InsertPretranslationsAsync(engineId, pretranslationsStream, cancellationToken); } - await using (await @lock.WriterLockAsync(cancellationToken: CancellationToken.None)) + await using ( + await @lock.WriterLockAsync( + lifetime: _buildJobOptions.PostProcessLockLifetime, + cancellationToken: CancellationToken.None + ) + ) { await DataAccessContext.WithTransactionAsync( async (ct) => diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs index c83f0703..d4ba43ef 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs @@ -10,7 +10,8 @@ public class SmtTransferBuildJob( IRepository trainSegmentPairs, ITruecaserFactory truecaserFactory, ISmtModelFactory smtModelFactory, - ICorpusService corpusService + ICorpusService corpusService, + IOptions buildJobOptions ) : HangfireBuildJob>( platformService, @@ -25,6 +26,7 @@ ICorpusService corpusService private readonly ITruecaserFactory _truecaserFactory = truecaserFactory; private readonly ISmtModelFactory _smtModelFactory = smtModelFactory; private readonly ICorpusService _corpusService = corpusService; + private readonly BuildJobOptions _buildJobOptions = buildJobOptions.Value; protected override Task InitializeAsync( string engineId, @@ -110,7 +112,12 @@ CancellationToken cancellationToken if (engine is null) throw new OperationCanceledException(); - await using (await @lock.WriterLockAsync(cancellationToken: cancellationToken)) + await using ( + await @lock.WriterLockAsync( + lifetime: _buildJobOptions.PostProcessLockLifetime, + cancellationToken: cancellationToken + ) + ) { cancellationToken.ThrowIfCancellationRequested(); await smtModelTrainer.SaveAsync(CancellationToken.None); diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferPostprocessBuildJob.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferPostprocessBuildJob.cs index d0d25fe5..1f8a4d48 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferPostprocessBuildJob.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferPostprocessBuildJob.cs @@ -8,10 +8,11 @@ public class SmtTransferPostprocessBuildJob( IBuildJobService buildJobService, ILogger logger, ISharedFileService sharedFileService, + IOptionsMonitor buildJobOptions, IRepository trainSegmentPairs, ISmtModelFactory smtModelFactory, ITruecaserFactory truecaserFactory, - IOptionsMonitor options + IOptionsMonitor engineOptions ) : PostprocessBuildJob( platformService, @@ -20,13 +21,14 @@ IOptionsMonitor options dataAccessContext, buildJobService, logger, - sharedFileService + sharedFileService, + buildJobOptions ) { private readonly ISmtModelFactory _smtModelFactory = smtModelFactory; private readonly ITruecaserFactory _truecaserFactory = truecaserFactory; private readonly IRepository _trainSegmentPairs = trainSegmentPairs; - private readonly IOptionsMonitor _options = options; + private readonly IOptionsMonitor _engineOptions = engineOptions; protected override async Task SaveModelAsync(string engineId, string buildId) { @@ -38,7 +40,7 @@ protected override async Task SaveModelAsync(string engineId, string buildI ) { await _smtModelFactory.UpdateEngineFromAsync( - Path.Combine(_options.CurrentValue.EnginesDir, engineId), + Path.Combine(_engineOptions.CurrentValue.EnginesDir, engineId), engineStream, CancellationToken.None ); @@ -54,7 +56,7 @@ private async Task TrainOnNewSegmentPairsAsync(string engineId) if (segmentPairs.Count == 0) return segmentPairs.Count; - string engineDir = Path.Combine(_options.CurrentValue.EnginesDir, engineId); + string engineDir = Path.Combine(_engineOptions.CurrentValue.EnginesDir, engineId); var tokenizer = new LatinWordTokenizer(); var detokenizer = new LatinWordDetokenizer(); ITruecaser truecaser = await _truecaserFactory.CreateAsync(engineDir); diff --git a/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockFactoryTests.cs b/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockFactoryTests.cs index d9389a69..84f61fae 100644 --- a/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockFactoryTests.cs +++ b/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockFactoryTests.cs @@ -70,6 +70,7 @@ public TestEnvironment() ServiceOptions serviceOptions = new() { ServiceId = "this_service" }; Factory = new DistributedReaderWriterLockFactory( new OptionsWrapper(serviceOptions), + new OptionsWrapper(new DistributedReaderWriterLockOptions()), Locks, new ObjectIdGenerator() ); diff --git a/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockTests.cs b/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockTests.cs index dae41b35..1b382c37 100644 --- a/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockTests.cs +++ b/src/Machine/test/Serval.Machine.Shared.Tests/Services/DistributedReaderWriterLockTests.cs @@ -394,9 +394,11 @@ public TestEnvironment() { Locks = new MemoryRepository(); var idGenerator = new ObjectIdGenerator(); - var options = Substitute.For>(); - options.Value.Returns(new ServiceOptions { ServiceId = "host" }); - Factory = new DistributedReaderWriterLockFactory(options, Locks, idGenerator); + var serviceOptions = Substitute.For>(); + serviceOptions.Value.Returns(new ServiceOptions { ServiceId = "host" }); + var lockOptions = Substitute.For>(); + lockOptions.Value.Returns(new DistributedReaderWriterLockOptions()); + Factory = new DistributedReaderWriterLockFactory(serviceOptions, lockOptions, Locks, idGenerator); } public DistributedReaderWriterLockFactory Factory { get; } diff --git a/src/Machine/test/Serval.Machine.Shared.Tests/Services/NmtEngineServiceTests.cs b/src/Machine/test/Serval.Machine.Shared.Tests/Services/NmtEngineServiceTests.cs index 5463e613..a36782b2 100644 --- a/src/Machine/test/Serval.Machine.Shared.Tests/Services/NmtEngineServiceTests.cs +++ b/src/Machine/test/Serval.Machine.Shared.Tests/Services/NmtEngineServiceTests.cs @@ -100,6 +100,7 @@ public TestEnvironment() PlatformService = Substitute.For(); _lockFactory = new DistributedReaderWriterLockFactory( new OptionsWrapper(new ServiceOptions { ServiceId = "host" }), + new OptionsWrapper(new DistributedReaderWriterLockOptions()), new MemoryRepository(), new ObjectIdGenerator() ); @@ -307,6 +308,8 @@ public override object ActivateJob(Type jobType) } if (jobType == typeof(PostprocessBuildJob)) { + var buildJobOptions = Substitute.For>(); + buildJobOptions.CurrentValue.Returns(new BuildJobOptions()); return new PostprocessBuildJob( _env.PlatformService, _env.Engines, @@ -314,7 +317,8 @@ public override object ActivateJob(Type jobType) new MemoryDataAccessContext(), _env.BuildJobService, Substitute.For>(), - _env.SharedFileService + _env.SharedFileService, + buildJobOptions ); } return base.ActivateJob(jobType); diff --git a/src/Machine/test/Serval.Machine.Shared.Tests/Services/PreprocessBuildJobTests.cs b/src/Machine/test/Serval.Machine.Shared.Tests/Services/PreprocessBuildJobTests.cs index 7c347603..d387f100 100644 --- a/src/Machine/test/Serval.Machine.Shared.Tests/Services/PreprocessBuildJobTests.cs +++ b/src/Machine/test/Serval.Machine.Shared.Tests/Services/PreprocessBuildJobTests.cs @@ -499,6 +499,7 @@ public TestEnvironment() PlatformService = Substitute.For(); LockFactory = new DistributedReaderWriterLockFactory( new OptionsWrapper(new ServiceOptions { ServiceId = "host" }), + new OptionsWrapper(new DistributedReaderWriterLockOptions()), new MemoryRepository(), new ObjectIdGenerator() ); diff --git a/src/Machine/test/Serval.Machine.Shared.Tests/Services/SmtTransferEngineServiceTests.cs b/src/Machine/test/Serval.Machine.Shared.Tests/Services/SmtTransferEngineServiceTests.cs index 5517aabf..52140ac8 100644 --- a/src/Machine/test/Serval.Machine.Shared.Tests/Services/SmtTransferEngineServiceTests.cs +++ b/src/Machine/test/Serval.Machine.Shared.Tests/Services/SmtTransferEngineServiceTests.cs @@ -247,6 +247,7 @@ public TestEnvironment(BuildJobRunnerType trainJobRunnerType = BuildJobRunnerTyp _truecaserFactory = CreateTruecaserFactory(); _lockFactory = new DistributedReaderWriterLockFactory( new OptionsWrapper(new ServiceOptions { ServiceId = "host" }), + new OptionsWrapper(new DistributedReaderWriterLockOptions()), new MemoryRepository(), new ObjectIdGenerator() ); @@ -702,8 +703,10 @@ public override object ActivateJob(Type jobType) } if (jobType == typeof(SmtTransferPostprocessBuildJob)) { - var options = Substitute.For>(); - options.CurrentValue.Returns(new SmtTransferEngineOptions()); + var engineOptions = Substitute.For>(); + engineOptions.CurrentValue.Returns(new SmtTransferEngineOptions()); + var buildJobOptions = Substitute.For>(); + buildJobOptions.CurrentValue.Returns(new BuildJobOptions()); return new SmtTransferPostprocessBuildJob( _env.PlatformService, _env.Engines, @@ -712,10 +715,11 @@ public override object ActivateJob(Type jobType) _env.BuildJobService, Substitute.For>(), _env.SharedFileService, + buildJobOptions, _env.TrainSegmentPairs, _env.SmtModelFactory, _env._truecaserFactory, - options + engineOptions ); } if (jobType == typeof(SmtTransferTrainBuildJob)) diff --git a/src/Serval/src/Serval.ApiServer/Startup.cs b/src/Serval/src/Serval.ApiServer/Startup.cs index 27e142e2..0831e75c 100644 --- a/src/Serval/src/Serval.ApiServer/Startup.cs +++ b/src/Serval/src/Serval.ApiServer/Startup.cs @@ -10,6 +10,13 @@ public void ConfigureServices(IServiceCollection services) { services.AddFeatureManagement(); services.AddRouting(o => o.LowercaseUrls = true); + + var apiOptions = new ApiOptions(); + Configuration.GetSection(ApiOptions.Key).Bind(apiOptions); + services.AddRequestTimeouts(o => + { + o.DefaultPolicy = new RequestTimeoutPolicy { Timeout = apiOptions.DefaultHttpRequestTimeout }; + }); services.AddOutputCache(options => { options.DefaultExpirationTimeSpan = TimeSpan.FromSeconds(10); @@ -215,6 +222,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) app.UseAuthentication(); app.UseRouting(); + app.UseRequestTimeouts(); app.UseOutputCache(); app.UseAuthorization(); app.UseEndpoints(x => diff --git a/src/Serval/src/Serval.ApiServer/Usings.cs b/src/Serval/src/Serval.ApiServer/Usings.cs index 377cd261..8f4b6446 100644 --- a/src/Serval/src/Serval.ApiServer/Usings.cs +++ b/src/Serval/src/Serval.ApiServer/Usings.cs @@ -10,6 +10,7 @@ global using MassTransit.Mediator; global using Microsoft.AspNetCore.Authentication.JwtBearer; global using Microsoft.AspNetCore.Authorization; +global using Microsoft.AspNetCore.Http.Timeouts; global using Microsoft.AspNetCore.Mvc; global using Microsoft.AspNetCore.OutputCaching; global using Microsoft.Extensions.Diagnostics.HealthChecks; diff --git a/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs b/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs index 3a139a36..17cdf116 100644 --- a/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs +++ b/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs @@ -314,7 +314,7 @@ CancellationToken cancellationToken EntityChange change = await TaskEx.Timeout( ct => _jobService.GetNewerRevisionAsync(jobId, minRevision.Value, ct), _apiOptions.CurrentValue.LongPollTimeout, - cancellationToken + cancellationToken: cancellationToken ); return change.Type switch { diff --git a/src/Serval/src/Serval.Shared/Configuration/ApiOptions.cs b/src/Serval/src/Serval.Shared/Configuration/ApiOptions.cs index b238316c..9a998b72 100644 --- a/src/Serval/src/Serval.Shared/Configuration/ApiOptions.cs +++ b/src/Serval/src/Serval.Shared/Configuration/ApiOptions.cs @@ -4,5 +4,6 @@ public class ApiOptions { public const string Key = "Api"; - public TimeSpan LongPollTimeout { get; set; } = TimeSpan.FromSeconds(40); + public TimeSpan DefaultHttpRequestTimeout { get; set; } = TimeSpan.FromSeconds(58); // must be less than 60 seconds Cloudflare timeout + public TimeSpan LongPollTimeout { get; set; } = TimeSpan.FromSeconds(40); // must be less than DefaultHttpRequestTimeout } diff --git a/src/Serval/src/Serval.Shared/Controllers/OperationCancelledExceptionFilter.cs b/src/Serval/src/Serval.Shared/Controllers/OperationCancelledExceptionFilter.cs index 7fc82635..40b494d1 100644 --- a/src/Serval/src/Serval.Shared/Controllers/OperationCancelledExceptionFilter.cs +++ b/src/Serval/src/Serval.Shared/Controllers/OperationCancelledExceptionFilter.cs @@ -11,7 +11,11 @@ context.Exception is OperationCanceledException || context.Exception is RpcException rpcEx && rpcEx.StatusCode == StatusCode.Cancelled ) { - _logger.LogInformation("Request was cancelled"); + _logger.LogInformation( + "Request {RequestMethod}:{RequestPath} was cancelled", + context.HttpContext.Request.Method, + context.HttpContext.Request.Path + ); context.ExceptionHandled = true; context.Result = new StatusCodeResult(499); } diff --git a/src/Serval/src/Serval.Shared/Utils/TaskEx.cs b/src/Serval/src/Serval.Shared/Utils/TaskEx.cs index a9dd7cba..edceaa93 100644 --- a/src/Serval/src/Serval.Shared/Utils/TaskEx.cs +++ b/src/Serval/src/Serval.Shared/Utils/TaskEx.cs @@ -13,8 +13,9 @@ public static class TaskEx var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); Task task = action(cts.Token); - var completedTask = await Task.WhenAny(task, Delay(timeout, cancellationToken)); - if (task != completedTask) + Task delayTask = Delay(timeout, cancellationToken); + var completedTask = await Task.WhenAny(task, delayTask); + if (delayTask.Status == TaskStatus.RanToCompletion) cts.Cancel(); return await completedTask; } @@ -33,8 +34,9 @@ public static async Task Timeout( { var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); Task task = action(cts.Token); - Task completedTask = await Task.WhenAny(task, Task.Delay(timeout, cancellationToken)); - if (task != completedTask) + Task delayTask = Task.Delay(timeout, cancellationToken); + var completedTask = await Task.WhenAny(task, delayTask); + if (delayTask.Status == TaskStatus.RanToCompletion) cts.Cancel(); await completedTask; } diff --git a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs index d11d8679..9b13f1f3 100644 --- a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs +++ b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs @@ -757,7 +757,7 @@ CancellationToken cancellationToken EntityChange change = await TaskEx.Timeout( ct => _buildService.GetNewerRevisionAsync(buildId, minRevision.Value, ct), _apiOptions.CurrentValue.LongPollTimeout, - cancellationToken + cancellationToken: cancellationToken ); return change.Type switch { @@ -867,7 +867,7 @@ CancellationToken cancellationToken EntityChange change = await TaskEx.Timeout( ct => _buildService.GetActiveNewerRevisionAsync(id, minRevision.Value, ct), _apiOptions.CurrentValue.LongPollTimeout, - cancellationToken + cancellationToken: cancellationToken ); return change.Type switch {