From 350b60cf5ccc2ea66e46760ee0dbb42256c20352 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Thu, 12 Mar 2026 17:38:11 -0600 Subject: [PATCH 1/2] Batch and deduplicate action resolution across composite depths Thread a cache through PrepareActionsRecursiveAsync so the same action is resolved at most once regardless of depth. Collect sub-actions from all sibling composites and resolve them in one API call instead of one per composite. ~30-composite internal workflow went from ~20 resolve calls to 3-4. Fixes https://github.com/actions/runner/issues/3731 Co-Authored-By: Claude Opus 4.6 --- src/Runner.Worker/ActionManager.cs | 102 +++- src/Test/L0/Worker/ActionManagerL0.cs | 638 ++++++++++++++++++++++++++ 2 files changed, 728 insertions(+), 12 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 38c2ab8b320..f1fd97e0edb 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -79,6 +79,9 @@ public sealed class ActionManager : RunnerService, IActionManager PreStepTracker = new Dictionary() }; var containerSetupSteps = new List(); + // Stack-local cache: same action (owner/repo@ref) is resolved only once, + // even if it appears at multiple depths in a composite tree. + var resolvedDownloadInfos = new Dictionary(StringComparer.OrdinalIgnoreCase); var depth = 0; // We are running at the start of a job if (rootStepId == default(Guid)) @@ -105,7 +108,7 @@ public sealed class ActionManager : RunnerService, IActionManager PrepareActionsState result = new PrepareActionsState(); try { - result = await PrepareActionsRecursiveAsync(executionContext, state, actions, depth, rootStepId); + result = await PrepareActionsRecursiveAsync(executionContext, state, actions, resolvedDownloadInfos, depth, rootStepId); } catch (FailedToResolveActionDownloadInfoException ex) { @@ -161,13 +164,14 @@ public sealed class ActionManager : RunnerService, IActionManager return new PrepareResult(containerSetupSteps, result.PreStepTracker); } - private async Task PrepareActionsRecursiveAsync(IExecutionContext executionContext, PrepareActionsState state, IEnumerable actions, Int32 depth = 0, Guid parentStepId = default(Guid)) + private async Task PrepareActionsRecursiveAsync(IExecutionContext executionContext, PrepareActionsState state, IEnumerable actions, Dictionary resolvedDownloadInfos, Int32 depth = 0, Guid parentStepId = default(Guid)) { ArgUtil.NotNull(executionContext, nameof(executionContext)); if (depth > Constants.CompositeActionsMaxDepth) { throw new Exception($"Composite action depth exceeded max depth {Constants.CompositeActionsMaxDepth}"); } + var repositoryActions = new List(); foreach (var action in actions) @@ -195,27 +199,32 @@ public sealed class ActionManager : RunnerService, IActionManager if (repositoryActions.Count > 0) { - // Get the download info - var downloadInfos = await GetDownloadInfoAsync(executionContext, repositoryActions); + // Resolve download info, skipping any actions already cached. + await ResolveNewActionsAsync(executionContext, repositoryActions, resolvedDownloadInfos); - // Download each action + // Download each action (in parallel when multiple unique actions exist). + var downloadKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var downloadTasks = new List(); foreach (var action in repositoryActions) { var lookupKey = GetDownloadInfoLookupKey(action); - if (string.IsNullOrEmpty(lookupKey)) + if (string.IsNullOrEmpty(lookupKey) || !downloadKeys.Add(lookupKey)) { continue; } - - if (!downloadInfos.TryGetValue(lookupKey, out var downloadInfo)) + if (!resolvedDownloadInfos.TryGetValue(lookupKey, out var downloadInfo)) { throw new Exception($"Missing download info for {lookupKey}"); } - - await DownloadRepositoryActionAsync(executionContext, downloadInfo); + downloadTasks.Add(DownloadRepositoryActionAsync(executionContext, downloadInfo)); } + await Task.WhenAll(downloadTasks); + + // Parse action.yml and collect composite sub-actions for batched + // resolution below. Pre/post step registration is deferred until + // after recursion so that HasPre/HasPost reflect the full subtree. + var nextLevel = new List<(Pipelines.ActionStep action, Guid parentId)>(); - // More preparation based on content in the repository (action.yml) foreach (var action in repositoryActions) { var setupInfo = PrepareRepositoryActionAsync(executionContext, action); @@ -247,8 +256,50 @@ public sealed class ActionManager : RunnerService, IActionManager } else if (setupInfo != null && setupInfo.Steps != null && setupInfo.Steps.Count > 0) { - state = await PrepareActionsRecursiveAsync(executionContext, state, setupInfo.Steps, depth + 1, action.Id); + foreach (var step in setupInfo.Steps) + { + nextLevel.Add((step, action.Id)); + } } + } + + // Resolve all next-level sub-actions in one batch API call, + // then recurse per parent (which hits the cache, not the API). + if (nextLevel.Count > 0) + { + var nextLevelRepoActions = nextLevel + .Where(x => x.action.Reference.Type == Pipelines.ActionSourceType.Repository) + .Select(x => x.action) + .ToList(); + await ResolveNewActionsAsync(executionContext, nextLevelRepoActions, resolvedDownloadInfos); + + // Pre-download next-level actions in parallel so that the + // recursive calls below find watermarks on disk and skip + // redundant downloads. + var preDownloadKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var preDownloadTasks = new List(); + foreach (var action in nextLevelRepoActions) + { + var lookupKey = GetDownloadInfoLookupKey(action); + if (!string.IsNullOrEmpty(lookupKey) && preDownloadKeys.Add(lookupKey) && resolvedDownloadInfos.TryGetValue(lookupKey, out var downloadInfo)) + { + preDownloadTasks.Add(DownloadRepositoryActionAsync(executionContext, downloadInfo)); + } + } + await Task.WhenAll(preDownloadTasks); + + foreach (var group in nextLevel.GroupBy(x => x.parentId)) + { + var groupActions = group.Select(x => x.action).ToList(); + state = await PrepareActionsRecursiveAsync(executionContext, state, groupActions, resolvedDownloadInfos, depth + 1, group.Key); + } + } + + // Register pre/post steps after recursion so that HasPre/HasPost + // are correct (they depend on _cachedEmbeddedPreSteps/PostSteps + // being populated by the recursive calls above). + foreach (var action in repositoryActions) + { var repoAction = action.Reference as Pipelines.RepositoryPathReference; if (repoAction.RepositoryType != Pipelines.PipelineConstants.SelfAlias) { @@ -754,6 +805,33 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, return actionDownloadInfos.Actions; } + /// + /// Only resolves actions not already in resolvedDownloadInfos. + /// Results are cached for reuse at deeper recursion levels. + /// + private async Task ResolveNewActionsAsync(IExecutionContext executionContext, List actions, Dictionary resolvedDownloadInfos) + { + var actionsToResolve = new List(); + var pendingKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var action in actions) + { + var lookupKey = GetDownloadInfoLookupKey(action); + if (!string.IsNullOrEmpty(lookupKey) && !resolvedDownloadInfos.ContainsKey(lookupKey) && pendingKeys.Add(lookupKey)) + { + actionsToResolve.Add(action); + } + } + + if (actionsToResolve.Count > 0) + { + var downloadInfos = await GetDownloadInfoAsync(executionContext, actionsToResolve); + foreach (var kvp in downloadInfos) + { + resolvedDownloadInfos[kvp.Key] = kvp.Value; + } + } + } + private async Task DownloadRepositoryActionAsync(IExecutionContext executionContext, WebApi.ActionDownloadInfo downloadInfo) { Trace.Entering(); diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 5aa1f2dbc20..b6983082494 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.IO; using System.IO.Compression; using System.Net; @@ -1253,6 +1254,643 @@ public async void PrepareActions_CompositeActionWithActionfile_CompositeContaine } #endif + // ================================================================= + // Tests for batched action resolution optimization + // ================================================================= + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_BatchesResolutionAcrossCompositeActions() + { + // Verifies that when multiple composite actions at the same depth + // reference sub-actions, those sub-actions are resolved in a single + // batched API call rather than one call per composite. + // + // Action tree: + // CompositePrestep (composite) → [Node action, CompositePrestep2 (composite)] + // CompositePrestep2 (composite) → [Node action, Docker action] + // + // Without batching: 3 API calls (depth 0, depth 1 for CompositePrestep, depth 2 for CompositePrestep2) + // With batching: still 3 calls at most, but the key is that depth-1 + // sub-actions from all composites at depth 0 are batched into 1 call. + // And the same action appearing at multiple depths triggers only 1 resolve. + try + { + //Arrange + Setup(); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + var resolveCallCount = 0; + var resolvedActions = new List(); + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + resolveCallCount++; + resolvedActions.Add(actions); + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actionId = Guid.NewGuid(); + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositePrestep", + RepositoryType = "GitHub" + } + } + }; + + //Act + var result = await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // The composite tree is: + // depth 0: CompositePrestep + // depth 1: Node@RepositoryActionWithWrapperActionfile_Node + CompositePrestep2 + // depth 2: Node@RepositoryActionWithWrapperActionfile_Node + Docker@RepositoryActionWithWrapperActionfile_Docker + // + // With batching: + // Call 1 (depth 0, resolve): CompositePrestep + // Call 2 (depth 0→1, pre-resolve): Node + CompositePrestep2 in one batch + // Call 3 (depth 1→2, pre-resolve): Docker only (Node already cached from call 2) + Assert.Equal(3, resolveCallCount); + + // Call 1: depth 0 resolve — just the top-level composite + var call1Keys = resolvedActions[0].Actions.Select(a => $"{a.NameWithOwner}@{a.Ref}").OrderBy(k => k).ToList(); + Assert.Equal(new[] { "TingluoHuang/runner_L0@CompositePrestep" }, call1Keys); + + // Call 2: depth 0→1 pre-resolve — batch both children of CompositePrestep + var call2Keys = resolvedActions[1].Actions.Select(a => $"{a.NameWithOwner}@{a.Ref}").OrderBy(k => k).ToList(); + Assert.Equal(new[] { "TingluoHuang/runner_L0@CompositePrestep2", "TingluoHuang/runner_L0@RepositoryActionWithWrapperActionfile_Node" }, call2Keys); + + // Call 3: depth 1→2 pre-resolve — only Docker (Node was cached in call 2) + var call3Keys = resolvedActions[2].Actions.Select(a => $"{a.NameWithOwner}@{a.Ref}").OrderBy(k => k).ToList(); + Assert.Equal(new[] { "TingluoHuang/runner_L0@RepositoryActionWithWrapperActionfile_Docker" }, call3Keys); + + // Verify all actions were downloaded + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "CompositePrestep.completed"))); + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"))); + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "CompositePrestep2.completed"))); + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"))); + + // Verify pre-step tracking still works correctly + Assert.Equal(1, result.PreStepTracker.Count); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_DeduplicatesResolutionAcrossDepthLevels() + { + // Verifies that an action appearing at multiple depths in the + // composite tree is only resolved once (not re-resolved at each level). + // + // CompositePrestep uses Node action at depth 1. + // CompositePrestep2 (also at depth 1) uses the SAME Node action at depth 2. + // The Node action should only be resolved once total. + try + { + //Arrange + Setup(); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + var allResolvedKeys = new List(); + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + allResolvedKeys.Add(key); + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actionId = Guid.NewGuid(); + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositePrestep", + RepositoryType = "GitHub" + } + } + }; + + //Act + await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // TingluoHuang/runner_L0@RepositoryActionWithWrapperActionfile_Node appears + // at both depth 1 (sub-step of CompositePrestep) and depth 2 (sub-step of + // CompositePrestep2). With deduplication it should only be resolved once. + var nodeActionKey = "TingluoHuang/runner_L0@RepositoryActionWithWrapperActionfile_Node"; + var nodeResolveCount = allResolvedKeys.FindAll(k => k == nodeActionKey).Count; + Assert.Equal(1, nodeResolveCount); + + // Verify the total number of unique actions resolved matches the tree + var uniqueKeys = new HashSet(allResolvedKeys); + // Expected unique actions: CompositePrestep, Node, CompositePrestep2, Docker = 4 + Assert.Equal(4, uniqueKeys.Count); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_MultipleTopLevelActions_BatchesResolution() + { + // Verifies that multiple independent actions at depth 0 are + // resolved in a single API call. + try + { + //Arrange + Setup(); + // Node action has pre+post, needs IActionRunner instances + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + var resolveCallCount = 0; + var firstCallActionCount = 0; + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + resolveCallCount++; + if (resolveCallCount == 1) + { + firstCallActionCount = actions.Actions.Count; + } + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action1", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action2", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Docker", + RepositoryType = "GitHub" + } + } + }; + + //Act + await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // Both actions are at depth 0 — should be resolved in a single batch call + Assert.Equal(1, resolveCallCount); + Assert.Equal(2, firstCallActionCount); + + // Verify both were downloaded + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"))); + Assert.True(File.Exists(Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"))); + } + finally + { + Teardown(); + } + } + +#if OS_LINUX + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_NestedCompositeContainers_BatchedResolution() + { + // Verifies batching with nested composite actions that reference + // container actions (Linux-only since containers require Linux). + // + // CompositeContainerNested (composite): + // → repositoryactionwithdockerfile (Dockerfile) + // → CompositeContainerNested2 (composite): + // → repositoryactionwithdockerfile (Dockerfile, same as above) + // → notpullorbuildimagesmultipletimes1 (Dockerfile) + try + { + //Arrange + Setup(); + + var resolveCallCount = 0; + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + resolveCallCount++; + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actionId = Guid.NewGuid(); + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositeContainerNested", + RepositoryType = "GitHub" + } + } + }; + + //Act + var result = await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // Tree has 3 depth levels with 5 unique actions. + // With batching, should need at most 3 resolve calls (one per depth level). + Assert.True(resolveCallCount <= 3, $"Expected at most 3 resolve calls but got {resolveCallCount}"); + + // repositoryactionwithdockerfile appears at both depth 1 and depth 2. + // Container setup should still work correctly — 2 unique Docker images. + Assert.Equal(2, result.ContainerSetupSteps.Count); + } + finally + { + Teardown(); + } + } +#endif + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_ParallelDownloads_MultipleUniqueActions() + { + // Verifies that multiple unique top-level actions are downloaded via + // DownloadActionsInParallelAsync (the parallel code path), and that + // all actions are correctly resolved and downloaded. + try + { + //Arrange + Setup(); + // Node action (top-level + inside composites) and Docker action + // may register pre steps — enqueue enough runners for all of them + for (int i = 0; i < 5; i++) + { + _hc.EnqueueInstance(new Mock().Object); + } + + var resolveCallCount = 0; + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + Interlocked.Increment(ref resolveCallCount); + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action1", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action2", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Docker", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action3", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositePrestep", + RepositoryType = "GitHub" + } + } + }; + + //Act + await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // 3 unique actions at depth 0 → triggers DownloadActionsInParallelAsync + // (parallel path used when uniqueDownloads.Count > 1) + var nodeCompleted = Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"); + var dockerCompleted = Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"); + var compositeCompleted = Path.Combine(_hc.GetDirectory(WellKnownDirectory.Actions), "TingluoHuang/runner_L0", "CompositePrestep.completed"); + + Assert.True(File.Exists(nodeCompleted), $"Expected watermark at {nodeCompleted}"); + Assert.True(File.Exists(dockerCompleted), $"Expected watermark at {dockerCompleted}"); + Assert.True(File.Exists(compositeCompleted), $"Expected watermark at {compositeCompleted}"); + + // All depth-0 actions resolved in a single batch call. + // Composite sub-actions may add 1-2 more calls. + Assert.True(resolveCallCount >= 1, "Expected at least 1 resolve call"); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_PreDownloadsNextLevelActions() + { + // Verifies that after pre-resolving next-level sub-actions, + // they are also pre-downloaded in parallel BEFORE recursion. + // This means the recursive call should find watermarks already + // on disk and skip redundant downloads. + // + // Action tree: + // CompositePrestep (composite) → [Node, CompositePrestep2 (composite)] + // CompositePrestep2 (composite) → [Node, Docker] + // + // Without pre-download: downloads happen during recursion (serial per depth) + // With pre-download: depth 1 actions (Node + CompositePrestep2) are + // downloaded in parallel before recursing, so recursion is a no-op + // for downloads. + try + { + //Arrange + Setup(); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + // Track watermark state at the time of each resolve call. + // If pre-download works, when the 3rd resolve fires (depth 2 + // pre-resolve for Docker), the depth-1 actions (Node + + // CompositePrestep2) should already have watermarks on disk. + var resolveCallCount = 0; + var watermarksAtResolve3 = new Dictionary(); + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + resolveCallCount++; + if (resolveCallCount == 3) + { + // At the time of the 3rd resolve, check if depth-1 actions + // are already downloaded (pre-download should have done this) + var actionsDir2 = _hc.GetDirectory(WellKnownDirectory.Actions); + watermarksAtResolve3["Node"] = File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed")); + watermarksAtResolve3["CompositePrestep2"] = File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "CompositePrestep2.completed")); + } + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actionId = Guid.NewGuid(); + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositePrestep", + RepositoryType = "GitHub" + } + } + }; + + //Act + var result = await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // All actions should be downloaded (watermarks exist) + var actionsDir = _hc.GetDirectory(WellKnownDirectory.Actions); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "CompositePrestep.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "CompositePrestep2.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"))); + + // 3 resolve calls total + Assert.Equal(3, resolveCallCount); + + // The key assertion: at the time of the 3rd resolve call + // (pre-resolve for depth 2), the depth-1 actions should + // ALREADY be downloaded thanks to pre-download. + // Without pre-download, these watermarks wouldn't exist yet + // because depth-1 downloads would only happen during recursion. + Assert.True(watermarksAtResolve3["Node"], + "Node action should be pre-downloaded before depth 2 pre-resolve"); + Assert.True(watermarksAtResolve3["CompositePrestep2"], + "CompositePrestep2 should be pre-downloaded before depth 2 pre-resolve"); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_ParallelDownloadsAtSameDepth() + { + // Verifies that multiple unique actions at the same depth are + // downloaded concurrently (Task.WhenAll) rather than sequentially. + // We detect this by checking that all watermarks exist after a + // single PrepareActionsAsync call with multiple top-level actions. + try + { + //Arrange + Setup(); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action1", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action2", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Docker", + RepositoryType = "GitHub" + } + } + }; + + //Act + await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert - both downloaded (parallel path used when > 1 unique download) + var actionsDir = _hc.GetDirectory(WellKnownDirectory.Actions); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"))); + } + finally + { + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] From bc068a465c651d89184472ae0c1d2f911df642c3 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Thu, 12 Mar 2026 22:18:41 -0600 Subject: [PATCH 2/2] Eager BFS resolution: pre-resolve and pre-download all reachable actions upfront Before the recursive composite walk begins, a BFS loop iteratively: 1. Resolves pending actions in one batch API call 2. Downloads all tarballs in parallel 3. Scans downloaded action.ymls for composite sub-action references 4. Feeds newly discovered actions back into the loop This collapses all network I/O into a tight loop so the recursive walk is purely local (zero API calls, zero downloads). Key design detail: uses separate tracking for downloads (keyed by owner/repo@ref) vs scans (keyed by owner/repo/path@ref) so that different sub-paths within the same repo tarball are each scanned for their composite children. Co-Authored-By: Claude Opus 4.6 --- src/Runner.Worker/ActionManager.cs | 184 ++++++++++++++++++++++++++ src/Test/L0/Worker/ActionManagerL0.cs | 107 +++++++++++++++ 2 files changed, 291 insertions(+) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index f1fd97e0edb..825dfcf66e6 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -105,6 +105,15 @@ public sealed class ActionManager : RunnerService, IActionManager } IEnumerable actions = steps.OfType(); executionContext.Output("Prepare all required actions"); + + // Eagerly discover, resolve, and download all reachable actions + // via BFS before the recursive walk. This collapses all network + // I/O into a tight loop so the recursion is purely local. + if (rootStepId == default(Guid)) + { + await EagerlyResolveAllReachableActionsAsync(executionContext, actions.ToList(), resolvedDownloadInfos); + } + PrepareActionsState result = new PrepareActionsState(); try { @@ -832,6 +841,181 @@ private async Task ResolveNewActionsAsync(IExecutionContext executionContext, Li } } + /// + /// BFS discovery loop: resolve and download all reachable actions upfront + /// so the recursive walk makes zero network calls. + /// + private async Task EagerlyResolveAllReachableActionsAsync( + IExecutionContext executionContext, + List initialActions, + Dictionary resolvedDownloadInfos) + { + // downloadedKeys tracks repo downloads (owner/repo@ref — no path). + // scannedKeys tracks which sub-path action.ymls have been scanned + // (owner/repo/path@ref) so we don't re-scan the same composite. + var downloadedKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var scannedKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var pending = new List(initialActions); + + while (pending.Count > 0) + { + // Collect repo actions we haven't scanned yet + var repoActions = new List(); + foreach (var action in pending) + { + if (action.Reference.Type != Pipelines.ActionSourceType.Repository) + { + continue; + } + var scanKey = GetScanKey(action); + if (!string.IsNullOrEmpty(scanKey) && scannedKeys.Add(scanKey)) + { + repoActions.Add(action); + } + } + pending.Clear(); + + if (repoActions.Count == 0) + { + break; + } + + // Resolve only repos not yet in the cache + var toResolve = repoActions + .Where(a => + { + var key = GetDownloadInfoLookupKey(a); + return !string.IsNullOrEmpty(key) && !downloadedKeys.Contains(key); + }) + .ToList(); + + if (toResolve.Count > 0) + { + await ResolveNewActionsAsync(executionContext, toResolve, resolvedDownloadInfos); + + // Download in parallel + var downloadTasks = new List(); + foreach (var action in toResolve) + { + var key = GetDownloadInfoLookupKey(action); + if (downloadedKeys.Add(key) && resolvedDownloadInfos.TryGetValue(key, out var info)) + { + downloadTasks.Add(DownloadRepositoryActionAsync(executionContext, info)); + } + } + await Task.WhenAll(downloadTasks); + } + + // Scan ALL actions for composite sub-references (including + // sub-paths of already-downloaded repos). + foreach (var action in repoActions) + { + var subRefs = ScanForRepoReferences(executionContext, action); + if (subRefs != null) + { + pending.AddRange(subRefs); + } + } + } + } + + /// + /// Returns a scan key that includes the sub-path, so that different + /// sub-paths within the same repo are scanned independently. + /// Format: "owner/repo/path@ref" or "owner/repo@ref" when no path. + /// + private static string GetScanKey(Pipelines.ActionStep action) + { + if (action.Reference.Type != Pipelines.ActionSourceType.Repository) + { + return null; + } + + var repoRef = action.Reference as Pipelines.RepositoryPathReference; + if (repoRef == null || + string.Equals(repoRef.RepositoryType, Pipelines.PipelineConstants.SelfAlias, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + if (!string.IsNullOrEmpty(repoRef.Path)) + { + return $"{repoRef.Name}/{repoRef.Path}@{repoRef.Ref}"; + } + return $"{repoRef.Name}@{repoRef.Ref}"; + } + + /// + /// Lightweight scan: load an action's manifest and return any repository + /// references from composite steps, without side effects (no GUID + /// assignment, no step caching). + /// + private List ScanForRepoReferences( + IExecutionContext executionContext, + Pipelines.ActionStep action) + { + var repoRef = action.Reference as Pipelines.RepositoryPathReference; + if (repoRef == null || + string.Equals(repoRef.RepositoryType, Pipelines.PipelineConstants.SelfAlias, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + string destDirectory = Path.Combine( + HostContext.GetDirectory(WellKnownDirectory.Actions), + repoRef.Name.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar), + repoRef.Ref); + string actionEntryDirectory = destDirectory; + if (!string.IsNullOrEmpty(repoRef.Path)) + { + actionEntryDirectory = Path.Combine(destDirectory, repoRef.Path); + } + + var actionManifest = Path.Combine(actionEntryDirectory, Constants.Path.ActionManifestYmlFile); + var actionManifestYaml = Path.Combine(actionEntryDirectory, Constants.Path.ActionManifestYamlFile); + + ActionDefinitionData actionDef = null; + try + { + var manifestManager = HostContext.GetService(); + if (File.Exists(actionManifest)) + { + actionDef = manifestManager.Load(executionContext, actionManifest); + } + else if (File.Exists(actionManifestYaml)) + { + actionDef = manifestManager.Load(executionContext, actionManifestYaml); + } + } + catch (Exception ex) + { + Trace.Warning($"Failed to scan action manifest for {repoRef.Name}: {ex.Message}"); + return null; + } + + if (actionDef?.Execution?.ExecutionType != ActionExecutionType.Composite) + { + return null; + } + + var compositeAction = actionDef.Execution as CompositeActionExecutionData; + if (compositeAction?.Steps == null) + { + return null; + } + + var result = new List(); + foreach (var step in compositeAction.Steps) + { + if (step is Pipelines.ActionStep actionStep && + actionStep.Reference?.Type == Pipelines.ActionSourceType.Repository) + { + result.Add(actionStep); + } + } + return result.Count > 0 ? result : null; + } + private async Task DownloadRepositoryActionAsync(IExecutionContext executionContext, WebApi.ActionDownloadInfo downloadInfo) { Trace.Entering(); diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index b6983082494..657bb51e4d4 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1891,6 +1891,113 @@ public async void PrepareActions_ParallelDownloadsAtSameDepth() } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_EagerResolution_AllDownloadsBeforeRecursion() + { + // Verifies that the eager BFS resolution loop resolves and + // downloads ALL reachable actions before the recursive walk + // begins. We detect this by checking that all watermarks exist + // at the time of the FIRST resolve callback that the recursion + // would normally trigger (i.e. the eager loop should have + // already completed all downloads by then). + // + // Action tree (3 levels): + // CompositePrestep → [Node, CompositePrestep2] + // CompositePrestep2 → [Node, Docker] + // + // With eager BFS all 4 watermarks should exist after the BFS + // loop, before recursion. We verify by recording watermark + // state during the very last resolve call. + try + { + //Arrange + Setup(); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + _hc.EnqueueInstance(new Mock().Object); + + var resolveCallCount = 0; + var allWatermarksAtLastResolve = new Dictionary(); + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + resolveCallCount++; + // On each resolve call, snapshot which watermarks exist. + // After the final call, all earlier downloads should be done. + var actionsDir = _hc.GetDirectory(WellKnownDirectory.Actions); + allWatermarksAtLastResolve["CompositePrestep"] = File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "CompositePrestep.completed")); + allWatermarksAtLastResolve["Node"] = File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed")); + allWatermarksAtLastResolve["CompositePrestep2"] = File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "CompositePrestep2.completed")); + allWatermarksAtLastResolve["Docker"] = File.Exists(Path.Combine(actionsDir, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed")); + + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "CompositePrestep", + RepositoryType = "GitHub" + } + } + }; + + //Act + var result = await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // All resolve calls should happen during the eager BFS loop + // (3 calls: one per BFS wavefront). + Assert.Equal(3, resolveCallCount); + + // At the time of the LAST resolve call (call 3 for Docker), + // the first two wavefronts should already be fully downloaded: + Assert.True(allWatermarksAtLastResolve["CompositePrestep"], + "CompositePrestep should be downloaded before last resolve call"); + Assert.True(allWatermarksAtLastResolve["Node"], + "Node should be downloaded before last resolve call"); + Assert.True(allWatermarksAtLastResolve["CompositePrestep2"], + "CompositePrestep2 should be downloaded before last resolve call"); + // Docker is resolved in the last call, so it's NOT yet downloaded + // at that point — that's expected. + Assert.False(allWatermarksAtLastResolve["Docker"], + "Docker should NOT yet be downloaded during its own resolve call"); + + // After PrepareActionsAsync completes, ALL should be downloaded + var actionsDir2 = _hc.GetDirectory(WellKnownDirectory.Actions); + Assert.True(File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "CompositePrestep.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Node.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "CompositePrestep2.completed"))); + Assert.True(File.Exists(Path.Combine(actionsDir2, "TingluoHuang/runner_L0", "RepositoryActionWithWrapperActionfile_Docker.completed"))); + } + finally + { + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")]