diff --git a/docs/docs/how-to/timeouts.md b/docs/docs/how-to/timeouts.md index 133e45763b4..55aae7d6ff4 100644 --- a/docs/docs/how-to/timeouts.md +++ b/docs/docs/how-to/timeouts.md @@ -22,6 +22,19 @@ builder.ConfigurePipelineOptions(options => options with You can override the pipeline default for one module using `Configure()`. Bear in mind some build runners, like GitHub Actions, have their own timeouts, so extending past these won't help. +`AlwaysRun` teardown has a separate 30-second scheduler-progress watchdog. This prevents a +constraint-deferred `AlwaysRun` module from waiting indefinitely for a hung active module, even +when ordinary module timeouts are disabled. Configure it independently when needed: + +```csharp +builder.ConfigurePipelineOptions(options => options with +{ + AlwaysRunProgressTimeout = TimeSpan.FromMinutes(1), +}); +``` + +Set `AlwaysRunProgressTimeout` to `TimeSpan.Zero` only when an unlimited teardown wait is intentional. + ## Using ModuleConfiguration ```csharp diff --git a/src/ModularPipelines/Engine/Execution/AlwaysRunHandler.cs b/src/ModularPipelines/Engine/Execution/AlwaysRunHandler.cs index 5db7e2f1da3..348977bedb6 100644 --- a/src/ModularPipelines/Engine/Execution/AlwaysRunHandler.cs +++ b/src/ModularPipelines/Engine/Execution/AlwaysRunHandler.cs @@ -21,7 +21,7 @@ internal class AlwaysRunHandler( { private readonly IModuleRunner _moduleRunner = moduleRunner; private readonly IParallelLimitProvider _parallelLimitProvider = parallelLimitProvider; - private readonly TimeSpan _schedulerProgressTimeout = pipelineOptions.Value.DefaultModuleTimeout; + private readonly TimeSpan _schedulerProgressTimeout = pipelineOptions.Value.AlwaysRunProgressTimeout; private readonly ILogger _logger = logger; private readonly TimeProvider _timeProvider = timeProvider; diff --git a/src/ModularPipelines/Options/PipelineOptions.cs b/src/ModularPipelines/Options/PipelineOptions.cs index 24f433a2d9f..063df831bc8 100644 --- a/src/ModularPipelines/Options/PipelineOptions.cs +++ b/src/ModularPipelines/Options/PipelineOptions.cs @@ -93,6 +93,12 @@ public record PipelineOptions /// public TimeSpan DefaultModuleTimeout { get; init; } = TimeSpan.FromMinutes(30); + /// + /// Gets the maximum cumulative time to wait for scheduler progress before retrying deferred + /// AlwaysRun modules. Set to to disable this watchdog. + /// + public TimeSpan AlwaysRunProgressTimeout { get; init; } = TimeSpan.FromSeconds(30); + /// /// Gets the collection of module categories to run exclusively, matched case-insensitively. /// If specified, only modules in these categories will run. diff --git a/src/ModularPipelines/Validation/OptionsValidator.cs b/src/ModularPipelines/Validation/OptionsValidator.cs index d4f095d73c5..bcd0ab21061 100644 --- a/src/ModularPipelines/Validation/OptionsValidator.cs +++ b/src/ModularPipelines/Validation/OptionsValidator.cs @@ -48,6 +48,13 @@ public ValidationResult ValidateOptions(PipelineOptions options) $"DefaultModuleTimeout cannot be negative. Current value: {options.DefaultModuleTimeout}")); } + if (options.AlwaysRunProgressTimeout < TimeSpan.Zero) + { + result.AddError(new ValidationError( + ValidationErrorCategory.Options, + $"AlwaysRunProgressTimeout cannot be negative. Current value: {options.AlwaysRunProgressTimeout}")); + } + var consoleOptions = options.Console; if (consoleOptions.ModuleOutputFlushInterval < TimeSpan.Zero) { diff --git a/test/ModularPipelines.UnitTests/Engine/Execution/AlwaysRunHandlerTests.cs b/test/ModularPipelines.UnitTests/Engine/Execution/AlwaysRunHandlerTests.cs index 7995cdb1c10..e8767962a7d 100644 --- a/test/ModularPipelines.UnitTests/Engine/Execution/AlwaysRunHandlerTests.cs +++ b/test/ModularPipelines.UnitTests/Engine/Execution/AlwaysRunHandlerTests.cs @@ -332,7 +332,7 @@ await Assert.That(prerequisiteStateWhenDependentStarted) } [Test] - public async Task WaitForAlwaysRunModulesAsync_TimesOutWhenSchedulerCannotMakeProgress() + public async Task WaitForAlwaysRunModulesAsync_UsesDedicatedProgressTimeoutWhenModuleTimeoutsAreDisabled() { var timeProvider = TestPipelineBuilder.CreateFakeTimeProvider(); var module = new FirstAlwaysRunModule(); @@ -358,14 +358,15 @@ public async Task WaitForAlwaysRunModulesAsync_TimesOutWhenSchedulerCannotMakePr CancellationToken.None)) .Returns(Task.CompletedTask); - var handler = CreateHandler( - moduleRunner.Object, - TimeSpan.FromMilliseconds(50), - timeProvider); + var pipelineOptions = new PipelineOptions + { + DefaultModuleTimeout = TimeSpan.Zero, + }; + var handler = CreateHandler(moduleRunner.Object, pipelineOptions, timeProvider); var handlerTask = handler.WaitForAlwaysRunModulesAsync(scheduler.Object, [module, blocker]); await progressWaitObserved.Task.WaitAsync(TimeSpan.FromSeconds(2)); - timeProvider.Advance(TimeSpan.FromMilliseconds(50)); + timeProvider.Advance(TimeSpan.FromSeconds(30)); var exception = await Assert.ThrowsAsync(() => handlerTask); await Assert.That(exception!.InnerExceptions).Contains(x => x is TimeoutException); @@ -418,7 +419,10 @@ public async Task WaitForAlwaysRunModulesAsync_UsesCumulativeSchedulerProgressTi var handler = CreateHandler( moduleRunner.Object, - TimeSpan.FromMilliseconds(200), + new PipelineOptions + { + AlwaysRunProgressTimeout = TimeSpan.FromMilliseconds(200), + }, timeProvider); var handlerTask = handler.WaitForAlwaysRunModulesAsync( scheduler.Object, @@ -460,7 +464,7 @@ private static Mock CreateScheduler(params ModuleState[] modul private static AlwaysRunHandler CreateHandler( IModuleRunner moduleRunner, - TimeSpan? schedulerProgressTimeout = null, + PipelineOptions? pipelineOptions = null, TimeProvider? timeProvider = null) { var parallelLimitProvider = new Mock(); @@ -471,10 +475,7 @@ private static AlwaysRunHandler CreateHandler( return new AlwaysRunHandler( moduleRunner, parallelLimitProvider.Object, - Microsoft.Extensions.Options.Options.Create(new PipelineOptions - { - DefaultModuleTimeout = schedulerProgressTimeout ?? TimeSpan.FromSeconds(2), - }), + Microsoft.Extensions.Options.Options.Create(pipelineOptions ?? new PipelineOptions()), NullLogger.Instance, timeProvider ?? TimeProvider.System); } diff --git a/test/ModularPipelines.UnitTests/Execution/ModuleTimeoutTests.cs b/test/ModularPipelines.UnitTests/Execution/ModuleTimeoutTests.cs index 38d068dc979..5c7868db2e7 100644 --- a/test/ModularPipelines.UnitTests/Execution/ModuleTimeoutTests.cs +++ b/test/ModularPipelines.UnitTests/Execution/ModuleTimeoutTests.cs @@ -72,6 +72,14 @@ public async Task Default_Module_Timeout_Is_Thirty_Minutes() await Assert.That(options.DefaultModuleTimeout).IsEqualTo(TimeSpan.FromMinutes(30)); } + [Test] + public async Task Default_AlwaysRun_Progress_Timeout_Is_Thirty_Seconds() + { + var options = new PipelineOptions(); + + await Assert.That(options.AlwaysRunProgressTimeout).IsEqualTo(TimeSpan.FromSeconds(30)); + } + [Test] public async Task Pipeline_Default_Module_Timeout_Is_Applied() { diff --git a/test/ModularPipelines.UnitTests/Validation/ValidationTests.cs b/test/ModularPipelines.UnitTests/Validation/ValidationTests.cs index 077bfe94972..d56ac743c15 100644 --- a/test/ModularPipelines.UnitTests/Validation/ValidationTests.cs +++ b/test/ModularPipelines.UnitTests/Validation/ValidationTests.cs @@ -518,6 +518,24 @@ await Assert.That(result.Errors.Any(e => e.Message.Contains("DefaultModuleTimeout"))).IsTrue(); } + [Test] + public async Task ValidateAsync_WithNegativeAlwaysRunProgressTimeout_ReturnsError() + { + var builder = Pipeline.CreateBuilder(); + builder.AddModule(); + builder.ConfigurePipelineOptions(options => options with + { + AlwaysRunProgressTimeout = TimeSpan.FromSeconds(-1), + }); + + var result = await builder.ValidateAsync(); + + await Assert.That(result.HasErrors).IsTrue(); + await Assert.That(result.Errors.Any(e => + e.Category == ValidationErrorCategory.Options && + e.Message.Contains("AlwaysRunProgressTimeout"))).IsTrue(); + } + [Test] public async Task ValidateAsync_WithNegativeModuleOutputFlushInterval_ReturnsError() {