[rush] Avoid redundant downloads when local processes race for the same build cache entry#5883
Open
iclanton wants to merge 3 commits into
Open
[rush] Avoid redundant downloads when local processes race for the same build cache entry#5883iclanton wants to merge 3 commits into
iclanton wants to merge 3 commits into
Conversation
…d cache entry When useDirectFileTransfersForBuildCache is enabled, multiple local Rush processes (e.g. parallel "rush build" invocations on the same machine or CI agent) restoring the same cache entry would each independently download it from the cloud cache provider. Use LockFile to have later processes wait for the first one to finish downloading, mirroring the pattern already used for installing the package manager (see InstallHelpers.ensureLocalPackageManagerAsync). After acquiring the lock (or failing/timing out), recheck whether the final cache entry path already exists before downloading, so a process that waited can skip the network round-trip entirely if a peer already populated it. This is strictly a best-effort optimization: if the lock cannot be acquired (for example, it's held by a process on a different machine sharing a network cache folder, where LockFile's stale-holder detection cannot see across machines) we fall back to downloading independently, exactly as before. Correctness never depends on the lock, since downloads always land in a uniquely-named temp file that is atomically renamed into place. The lock's resource name is derived by hashing the cache ID (sha1-hex), since cache IDs can contain characters (e.g. "+", "_", "/") that LockFile.getLockFilePath's resource name validation rejects, depending on the configured cacheEntryNamePattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d807794-b652-4bf8-bc4b-8ee4b2039f92
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d807794-b652-4bf8-bc4b-8ee4b2039f92
iclanton
requested review from
apostolisms,
bmiddha,
dmichon-msft,
jxanthony and
octogonz
as code owners
July 18, 2026 00:52
…me build cache entry
iclanton
enabled auto-merge (squash)
July 18, 2026 01:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #5746. When
useDirectFileTransfersForBuildCacheis enabled, multiple local Rush processes (e.g. parallelrush buildinvocations on the same machine or CI agent) restoring the same cache entry would each independently download it from the cloud cache provider, wasting bandwidth for potentially large cache entries.Details
OperationBuildCachenow attempts to acquire aLockFilescoped to the cache entry (mirroring the pattern already used for installing the package manager — seeInstallHelpers.ensureLocalPackageManagerAsync), so later processes wait for the first one to finish instead of racing.LockFile's same-machine stale-holder detection can't see across machines), we simply fall back to downloading independently, exactly as before. Correctness never depends on the lock, since downloads always land in a uniquely-named temp file that is atomically renamed into place (from the prior fix in Add file-based transfer APIs to build cache provider interface and all cache plugins #5746).+,_,/) thatLockFile.getLockFilePath's resource name validation rejects, depending on the configuredcacheEntryNamePattern. (Rush's own recommended default pattern produces IDs containing both+and_.)How it was tested
OperationBuildCache.test.tscovering: lock acquired/released around a normal download, download skipped when another process already populated the entry while waiting for the lock, and fallback to independent download when the lock cannot be acquired.OperationBuildCachetests continue to pass (11/11).rush build(typecheck, lint, API Extractor) passes;OperationBuildCacheis@internal, so there is no public API surface change.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com