From 2f8f44b008e7c35add7e46a86d322b4871392f1f Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 4 Sep 2024 17:23:56 -0400 Subject: [PATCH] Have true RequestTimeouts and directly encode default lock time. --- .../Configuration/ServiceOptions.cs | 1 - .../Services/DistributedReaderWriterLock.cs | 11 +++-------- .../Services/DistributedReaderWriterLockFactory.cs | 8 +------- .../Services/SmtTransferBuildJob.cs | 4 +++- src/Serval/src/Serval.ApiServer/Startup.cs | 6 ++++++ src/Serval/src/Serval.ApiServer/Usings.cs | 1 + .../Controllers/AssessmentEnginesController.cs | 1 + src/Serval/src/Serval.Assessment/Usings.cs | 1 + .../Controllers/DataFilesController.cs | 1 + src/Serval/src/Serval.DataFiles/Usings.cs | 1 + .../Controllers/TranslationEnginesController.cs | 1 + src/Serval/src/Serval.Translation/Usings.cs | 1 + .../Serval.Webhooks/Controllers/WebhooksController.cs | 1 + src/Serval/src/Serval.Webhooks/Usings.cs | 1 + 14 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/Machine/src/Serval.Machine.Shared/Configuration/ServiceOptions.cs b/src/Machine/src/Serval.Machine.Shared/Configuration/ServiceOptions.cs index c41aa098..8011e7b7 100644 --- a/src/Machine/src/Serval.Machine.Shared/Configuration/ServiceOptions.cs +++ b/src/Machine/src/Serval.Machine.Shared/Configuration/ServiceOptions.cs @@ -5,5 +5,4 @@ public class ServiceOptions public const string Key = "Service"; public string ServiceId { get; set; } = "machine_api"; - public TimeSpan ReadWriteLockTimeout { get; set; } = TimeSpan.FromSeconds(55); } diff --git a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs index 0e6a6395..aa320bc7 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLock.cs @@ -1,18 +1,13 @@ namespace Serval.Machine.Shared.Services; -public class DistributedReaderWriterLock( - string hostId, - IRepository locks, - IIdGenerator idGenerator, - string id, - TimeSpan defaultLifetime -) : IDistributedReaderWriterLock +public class DistributedReaderWriterLock(string hostId, IRepository locks, IIdGenerator idGenerator, string id) + : IDistributedReaderWriterLock { private readonly string _hostId = hostId; private readonly IRepository _locks = locks; private readonly IIdGenerator _idGenerator = idGenerator; private readonly string _id = id; - private readonly TimeSpan _defaultLifetime = defaultLifetime; + private readonly TimeSpan _defaultLifetime = TimeSpan.FromSeconds(10); public async Task ReaderLockAsync( TimeSpan? lifetime = default, diff --git a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs index acc9e38c..81810fb1 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/DistributedReaderWriterLockFactory.cs @@ -39,13 +39,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, - _serviceOptions.ReadWriteLockTimeout - ); + return new DistributedReaderWriterLock(_serviceOptions.ServiceId, _locks, _idGenerator, id); } public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) diff --git a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs index c83f0703..45db348b 100644 --- a/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs +++ b/src/Machine/src/Serval.Machine.Shared/Services/SmtTransferBuildJob.cs @@ -110,7 +110,9 @@ CancellationToken cancellationToken if (engine is null) throw new OperationCanceledException(); - await using (await @lock.WriterLockAsync(cancellationToken: cancellationToken)) + await using ( + await @lock.WriterLockAsync(lifetime: TimeSpan.FromMinutes(5), cancellationToken: cancellationToken) + ) { cancellationToken.ThrowIfCancellationRequested(); await smtModelTrainer.SaveAsync(CancellationToken.None); diff --git a/src/Serval/src/Serval.ApiServer/Startup.cs b/src/Serval/src/Serval.ApiServer/Startup.cs index 27e142e2..12ae3afb 100644 --- a/src/Serval/src/Serval.ApiServer/Startup.cs +++ b/src/Serval/src/Serval.ApiServer/Startup.cs @@ -10,6 +10,11 @@ public void ConfigureServices(IServiceCollection services) { services.AddFeatureManagement(); services.AddRouting(o => o.LowercaseUrls = true); + services.AddRequestTimeouts(o => + { + o.DefaultPolicy = new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(10) }; + o.AddPolicy("LongRequest", TimeSpan.FromSeconds(55)); + }); services.AddOutputCache(options => { options.DefaultExpirationTimeSpan = TimeSpan.FromSeconds(10); @@ -215,6 +220,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..64a3068f 100644 --- a/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs +++ b/src/Serval/src/Serval.Assessment/Controllers/AssessmentEnginesController.cs @@ -32,6 +32,7 @@ IUrlService urlService /// A necessary service is currently unavailable. Check `/health` for more details. [Authorize(Scopes.ReadAssessmentEngines)] [HttpGet] + [RequestTimeout("LongRequest")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(void), StatusCodes.Status403Forbidden)] diff --git a/src/Serval/src/Serval.Assessment/Usings.cs b/src/Serval/src/Serval.Assessment/Usings.cs index 17020327..d4fac633 100644 --- a/src/Serval/src/Serval.Assessment/Usings.cs +++ b/src/Serval/src/Serval.Assessment/Usings.cs @@ -9,6 +9,7 @@ global using MassTransit; global using Microsoft.AspNetCore.Authorization; global using Microsoft.AspNetCore.Http; +global using Microsoft.AspNetCore.Http.Timeouts; global using Microsoft.AspNetCore.Mvc; global using Microsoft.AspNetCore.Routing; global using Microsoft.Extensions.Configuration; diff --git a/src/Serval/src/Serval.DataFiles/Controllers/DataFilesController.cs b/src/Serval/src/Serval.DataFiles/Controllers/DataFilesController.cs index 32218a68..6333eb7b 100644 --- a/src/Serval/src/Serval.DataFiles/Controllers/DataFilesController.cs +++ b/src/Serval/src/Serval.DataFiles/Controllers/DataFilesController.cs @@ -21,6 +21,7 @@ IUrlService urlService /// A necessary service is currently unavailable. Check `/health` for more details. [Authorize(Scopes.ReadFiles)] [HttpGet] + [RequestTimeout("LongRequest")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(void), StatusCodes.Status403Forbidden)] diff --git a/src/Serval/src/Serval.DataFiles/Usings.cs b/src/Serval/src/Serval.DataFiles/Usings.cs index 0d81ccb1..09c6a625 100644 --- a/src/Serval/src/Serval.DataFiles/Usings.cs +++ b/src/Serval/src/Serval.DataFiles/Usings.cs @@ -6,6 +6,7 @@ global using MassTransit.Mediator; global using Microsoft.AspNetCore.Authorization; global using Microsoft.AspNetCore.Http; +global using Microsoft.AspNetCore.Http.Timeouts; global using Microsoft.AspNetCore.Mvc; global using Microsoft.AspNetCore.Mvc.ModelBinding; global using Microsoft.AspNetCore.Routing; diff --git a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs index d11d8679..8bd1fe4e 100644 --- a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs +++ b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs @@ -31,6 +31,7 @@ IUrlService urlService /// A necessary service is currently unavailable. Check `/health` for more details. [Authorize(Scopes.ReadTranslationEngines)] [HttpGet] + [RequestTimeout("LongRequest")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(void), StatusCodes.Status403Forbidden)] diff --git a/src/Serval/src/Serval.Translation/Usings.cs b/src/Serval/src/Serval.Translation/Usings.cs index 77bb4439..7892ede9 100644 --- a/src/Serval/src/Serval.Translation/Usings.cs +++ b/src/Serval/src/Serval.Translation/Usings.cs @@ -9,6 +9,7 @@ global using MassTransit; global using Microsoft.AspNetCore.Authorization; global using Microsoft.AspNetCore.Http; +global using Microsoft.AspNetCore.Http.Timeouts; global using Microsoft.AspNetCore.Mvc; global using Microsoft.AspNetCore.Routing; global using Microsoft.Extensions.Configuration; diff --git a/src/Serval/src/Serval.Webhooks/Controllers/WebhooksController.cs b/src/Serval/src/Serval.Webhooks/Controllers/WebhooksController.cs index 8758faca..e88b09f3 100644 --- a/src/Serval/src/Serval.Webhooks/Controllers/WebhooksController.cs +++ b/src/Serval/src/Serval.Webhooks/Controllers/WebhooksController.cs @@ -15,6 +15,7 @@ public class WebhooksController(IAuthorizationService authService, IWebhookServi /// A necessary service is currently unavailable. Check `/health` for more details. [Authorize(Scopes.ReadHooks)] [HttpGet] + [RequestTimeout("LongRequest")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status503ServiceUnavailable)] public async Task> GetAllAsync(CancellationToken cancellationToken) diff --git a/src/Serval/src/Serval.Webhooks/Usings.cs b/src/Serval/src/Serval.Webhooks/Usings.cs index f68d9a61..1b8b8e26 100644 --- a/src/Serval/src/Serval.Webhooks/Usings.cs +++ b/src/Serval/src/Serval.Webhooks/Usings.cs @@ -8,6 +8,7 @@ global using MassTransit; global using Microsoft.AspNetCore.Authorization; global using Microsoft.AspNetCore.Http; +global using Microsoft.AspNetCore.Http.Timeouts; global using Microsoft.AspNetCore.Mvc; global using Microsoft.AspNetCore.Routing; global using Microsoft.Extensions.Options;