-
Notifications
You must be signed in to change notification settings - Fork 731
Improve pipeline concurrency with readiness-based scheduler #12059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0dfbabc
a950d0e
2ab0b84
4a27085
4cccccb
644f85b
6306b3a
08550b6
ce4697a
dc903e0
e64e04f
68fa8fe
f869798
4fb390f
7e65d7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -113,39 +113,10 @@ public async Task ExecuteAsync(DeployingContext context) | |||||||||||
|
|
||||||||||||
| ValidateSteps(allSteps); | ||||||||||||
|
|
||||||||||||
| var stepsByName = allSteps.ToDictionary(s => s.Name); | ||||||||||||
| var stepsByName = allSteps.ToDictionary(s => s.Name, StringComparer.Ordinal); | ||||||||||||
|
|
||||||||||||
| var levels = ResolveDependencies(allSteps, stepsByName); | ||||||||||||
|
|
||||||||||||
| foreach (var level in levels) | ||||||||||||
| { | ||||||||||||
| var tasks = level.Select(step => ExecuteStepAsync(step, context)).ToList(); | ||||||||||||
| try | ||||||||||||
| { | ||||||||||||
| await Task.WhenAll(tasks).ConfigureAwait(false); | ||||||||||||
| } | ||||||||||||
| catch | ||||||||||||
| { | ||||||||||||
| // Collect all exceptions from failed tasks | ||||||||||||
| var exceptions = tasks | ||||||||||||
| .Where(t => t.IsFaulted) | ||||||||||||
| .SelectMany(t => t.Exception?.InnerExceptions ?? Enumerable.Empty<Exception>()) | ||||||||||||
| .ToList(); | ||||||||||||
|
|
||||||||||||
| if (exceptions.Count == 1) | ||||||||||||
| { | ||||||||||||
| ExceptionDispatchInfo.Capture(exceptions[0]).Throw(); | ||||||||||||
| } | ||||||||||||
| else if (exceptions.Count > 1) | ||||||||||||
| { | ||||||||||||
| throw new AggregateException( | ||||||||||||
| $"Multiple pipeline steps failed at the same level: {string.Join(", ", exceptions.OfType<InvalidOperationException>().Select(e => e.Message))}", | ||||||||||||
| exceptions); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| throw; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| // Build dependency graph and execute with readiness-based scheduler | ||||||||||||
| await ExecuteStepsAsTaskDag(allSteps, stepsByName, context).ConfigureAwait(false); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private static IEnumerable<PipelineStep> CollectStepsFromAnnotations(DeployingContext context) | ||||||||||||
|
|
@@ -167,7 +138,7 @@ private static IEnumerable<PipelineStep> CollectStepsFromAnnotations(DeployingCo | |||||||||||
|
|
||||||||||||
| private static void ValidateSteps(IEnumerable<PipelineStep> steps) | ||||||||||||
| { | ||||||||||||
| var stepNames = new HashSet<string>(); | ||||||||||||
| var stepNames = new HashSet<string>(StringComparer.Ordinal); | ||||||||||||
|
|
||||||||||||
| foreach (var step in steps) | ||||||||||||
| { | ||||||||||||
|
|
@@ -201,126 +172,213 @@ private static void ValidateSteps(IEnumerable<PipelineStep> steps) | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /// <summary> | ||||||||||||
| /// Resolves the dependencies among the steps and organizes them into levels for execution. | ||||||||||||
| /// Executes pipeline steps by building a Task DAG where each step waits on its dependencies. | ||||||||||||
| /// Uses CancellationToken to stop remaining work when any step fails. | ||||||||||||
| /// </summary> | ||||||||||||
| /// <param name="steps">The complete set of pipeline steps populated from annotations and the builder</param> | ||||||||||||
| /// <param name="stepsByName">A dictionary mapping step names to their corresponding step objects</param> | ||||||||||||
| /// <returns>A list of lists where each list contains the steps to be executed at the same level</returns> | ||||||||||||
| private static List<List<PipelineStep>> ResolveDependencies( | ||||||||||||
| IEnumerable<PipelineStep> steps, | ||||||||||||
| Dictionary<string, PipelineStep> stepsByName) | ||||||||||||
| private static async Task ExecuteStepsAsTaskDag( | ||||||||||||
| List<PipelineStep> steps, | ||||||||||||
| Dictionary<string, PipelineStep> stepsByName, | ||||||||||||
| DeployingContext context) | ||||||||||||
| { | ||||||||||||
| // Initial a graph that represents a step and its dependencies | ||||||||||||
| // and an inDegree map to count the number of dependencies that | ||||||||||||
| // each step has. | ||||||||||||
| var graph = new Dictionary<string, List<string>>(); | ||||||||||||
| var inDegree = new Dictionary<string, int>(); | ||||||||||||
| // Validate no cycles exist in the dependency graph | ||||||||||||
| ValidateDependencyGraph(steps, stepsByName); | ||||||||||||
|
|
||||||||||||
| // Create a TaskCompletionSource for each step | ||||||||||||
| var stepCompletions = new Dictionary<string, TaskCompletionSource>(steps.Count, StringComparer.Ordinal); | ||||||||||||
| foreach (var step in steps) | ||||||||||||
| { | ||||||||||||
| graph[step.Name] = []; | ||||||||||||
| inDegree[step.Name] = 0; | ||||||||||||
| stepCompletions[step.Name] = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Process all the `RequiredBy` relationships in the graph and adds | ||||||||||||
| // the each `RequiredBy` step to the DependsOn list of the step that requires it. | ||||||||||||
| foreach (var step in steps) | ||||||||||||
| // Execute a step after its dependencies complete | ||||||||||||
| async Task ExecuteStepWithDependencies(PipelineStep step) | ||||||||||||
| { | ||||||||||||
| foreach (var requiredByStep in step.RequiredBySteps) | ||||||||||||
| var stepTcs = stepCompletions[step.Name]; | ||||||||||||
|
|
||||||||||||
| // Wait for all dependencies to complete (will throw if any dependency failed) | ||||||||||||
| if (step.DependsOnSteps.Count > 0) | ||||||||||||
| { | ||||||||||||
| if (!graph.ContainsKey(requiredByStep)) | ||||||||||||
| try | ||||||||||||
| { | ||||||||||||
| throw new InvalidOperationException( | ||||||||||||
| $"Step '{step.Name}' is required by unknown step '{requiredByStep}'"); | ||||||||||||
| var depTasks = step.DependsOnSteps.Select(depName => stepCompletions[depName].Task); | ||||||||||||
| await Task.WhenAll(depTasks).ConfigureAwait(false); | ||||||||||||
| } | ||||||||||||
| catch (Exception ex) | ||||||||||||
| { | ||||||||||||
| // Find all dependencies that failed | ||||||||||||
| var failedDeps = step.DependsOnSteps | ||||||||||||
| .Where(depName => stepCompletions[depName].Task.IsFaulted) | ||||||||||||
| .ToList(); | ||||||||||||
|
|
||||||||||||
| var message = failedDeps.Count > 0 | ||||||||||||
| ? $"Step '{step.Name}' cannot run because {(failedDeps.Count == 1 ? "dependency" : "dependencies")} {string.Join(", ", failedDeps.Select(d => $"'{d}'"))} failed" | ||||||||||||
| : $"Step '{step.Name}' cannot run because a dependency failed"; | ||||||||||||
|
|
||||||||||||
| // Wrap the dependency failure with context about this step | ||||||||||||
| var wrappedException = new InvalidOperationException(message, ex); | ||||||||||||
| stepTcs.TrySetException(wrappedException); | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| try | ||||||||||||
| { | ||||||||||||
| await ExecuteStepAsync(step, context).ConfigureAwait(false); | ||||||||||||
|
|
||||||||||||
| stepTcs.TrySetResult(); | ||||||||||||
| } | ||||||||||||
| catch (Exception ex) | ||||||||||||
| { | ||||||||||||
| // Execution failure - mark as failed and re-throw so it's counted | ||||||||||||
| stepTcs.TrySetException(ex); | ||||||||||||
| throw; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Start all steps (they'll wait on their dependencies internally) | ||||||||||||
| var allStepTasks = new Task[steps.Count]; | ||||||||||||
| for (var i = 0; i < steps.Count; i++) | ||||||||||||
| { | ||||||||||||
| var step = steps[i]; | ||||||||||||
| allStepTasks[i] = Task.Run(() => ExecuteStepWithDependencies(step)); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (stepsByName.TryGetValue(requiredByStep, out var requiredByStepObj) && | ||||||||||||
| !requiredByStepObj.DependsOnSteps.Contains(step.Name)) | ||||||||||||
| // Wait for all steps to complete (or fail) | ||||||||||||
| try | ||||||||||||
| { | ||||||||||||
| await Task.WhenAll(allStepTasks).ConfigureAwait(false); | ||||||||||||
| } | ||||||||||||
| catch | ||||||||||||
| { | ||||||||||||
| // Collect all failed steps and their names | ||||||||||||
| var failures = allStepTasks | ||||||||||||
| .Where(t => t.IsFaulted) | ||||||||||||
| .Select(t => t.Exception!) | ||||||||||||
| .SelectMany(ae => ae.InnerExceptions) | ||||||||||||
| .ToList(); | ||||||||||||
|
|
||||||||||||
| if (failures.Count > 1) | ||||||||||||
| { | ||||||||||||
| // Match failures to steps to get their names | ||||||||||||
| var failedStepNames = new List<string>(); | ||||||||||||
| for (var i = 0; i < allStepTasks.Length; i++) | ||||||||||||
| { | ||||||||||||
| requiredByStepObj.DependsOnSteps.Add(step.Name); | ||||||||||||
| if (allStepTasks[i].IsFaulted) | ||||||||||||
| { | ||||||||||||
| failedStepNames.Add(steps[i].Name); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var message = failedStepNames.Count > 0 | ||||||||||||
| ? $"Multiple pipeline steps failed: {string.Join(", ", failedStepNames.Distinct())}" | ||||||||||||
| : "Multiple pipeline steps failed."; | ||||||||||||
|
|
||||||||||||
| throw new AggregateException(message, failures); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Single failure - just rethrow | ||||||||||||
| throw; | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same theme: Wrap exception with an exception with message that includes name? |
||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /// <summary> | ||||||||||||
| /// Represents the visitation state of a step during cycle detection. | ||||||||||||
| /// </summary> | ||||||||||||
| private enum VisitState | ||||||||||||
| { | ||||||||||||
| /// <summary> | ||||||||||||
| /// The step has not been visited yet. | ||||||||||||
| /// </summary> | ||||||||||||
| Unvisited, | ||||||||||||
|
|
||||||||||||
| /// <summary> | ||||||||||||
| /// The step is currently being visited (on the current DFS path). | ||||||||||||
| /// </summary> | ||||||||||||
| Visiting, | ||||||||||||
|
|
||||||||||||
| /// <summary> | ||||||||||||
| /// The step has been fully visited (all descendants explored). | ||||||||||||
| /// </summary> | ||||||||||||
| Visited | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Now that the `DependsOn` lists are fully populated, we can build the graph | ||||||||||||
| // and the inDegree map based only on the DependOnSteps list. | ||||||||||||
| /// <summary> | ||||||||||||
| /// Validates that the pipeline steps form a directed acyclic graph (DAG) with no circular dependencies. | ||||||||||||
| /// </summary> | ||||||||||||
| /// <remarks> | ||||||||||||
| /// Uses depth-first search (DFS) to detect cycles. A cycle exists if we encounter a node that is | ||||||||||||
| /// currently being visited (in the Visiting state), meaning we've found a back edge in the graph. | ||||||||||||
| /// | ||||||||||||
| /// Example: A → B → C is valid (no cycle) | ||||||||||||
| /// Example: A → B → C → A is invalid (cycle detected) | ||||||||||||
| /// Example: A → B, A → C, B → D, C → D is valid (diamond dependency, no cycle) | ||||||||||||
| /// </remarks> | ||||||||||||
| private static void ValidateDependencyGraph( | ||||||||||||
| List<PipelineStep> steps, | ||||||||||||
| Dictionary<string, PipelineStep> stepsByName) | ||||||||||||
| { | ||||||||||||
| // Process all RequiredBy relationships and normalize to DependsOn | ||||||||||||
| foreach (var step in steps) | ||||||||||||
| { | ||||||||||||
| foreach (var dependency in step.DependsOnSteps) | ||||||||||||
| foreach (var requiredByStep in step.RequiredBySteps) | ||||||||||||
| { | ||||||||||||
| if (!graph.TryGetValue(dependency, out var dependents)) | ||||||||||||
| if (!stepsByName.TryGetValue(requiredByStep, out var requiredByStepObj)) | ||||||||||||
| { | ||||||||||||
| throw new InvalidOperationException( | ||||||||||||
| $"Step '{step.Name}' depends on unknown step '{dependency}'"); | ||||||||||||
| $"Step '{step.Name}' is required by unknown step '{requiredByStep}'"); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| dependents.Add(step.Name); | ||||||||||||
| inDegree[step.Name]++; | ||||||||||||
| requiredByStepObj.DependsOnSteps.Add(step.Name); | ||||||||||||
|
||||||||||||
| requiredByStepObj.DependsOnSteps.Add(step.Name); | |
| if (!requiredByStepObj.DependsOnSteps.Contains(step.Name)) | |
| { | |
| requiredByStepObj.DependsOnSteps.Add(step.Name); | |
| } |
Uh oh!
There was an error while loading. Please reload this page.