Skip to content

CLDSRV-955: Cache node_modules instead of yarn download cache#6227

Open
tcarmet wants to merge 1 commit into
development/9.3from
improvement/CLDSRV-955/node-modules-cache
Open

CLDSRV-955: Cache node_modules instead of yarn download cache#6227
tcarmet wants to merge 1 commit into
development/9.3from
improvement/CLDSRV-955/node-modules-cache

Conversation

@tcarmet

@tcarmet tcarmet commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Speeds up CI by caching the installed dependency tree between runs instead of only the package-manager download cache. Because several dependencies compile TypeScript at install time, previous runs re-cloned, recompiled and relinked them on every job despite a "cache hit"; now a warm cache skips that work entirely and jobs reuse the prior install. This is the first of two changes tracked under CLDSRV-955 to reduce the tests workflow wall time; the second parallelizes the image builds.

Replace `cache: yarn` (which only caches the yarn download dir) with an
actions/cache on node_modules keyed by the yarn.lock hash, and make the
yarn install / typescript steps conditional on a cache miss. Because the
git dependencies compile TypeScript at install time, a warm cache now
skips the clone, tsc build and relink entirely.

Applied to the setup-ci composite action (used by all functional jobs)
and the lint, async-migration-report, unit-tests and cleanup-gcp-buckets
jobs.
@bert-e

bert-e commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Hello tcarmet,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval
/bypass_build_status Bypass the build and test status
/bypass_commit_size Bypass the check on the size of the changeset TBA
/bypass_incompatible_branch Bypass the check on the source branch prefix
/bypass_jira_check Bypass the Jira issue check
/bypass_peer_approval Bypass the pull request peers' approval
/bypass_leader_approval Bypass the pull request leaders' approval
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request TBA
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e

bert-e commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Incorrect fix version

The Fix Version/s in issue CLDSRV-955 contains:

  • None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

  • 9.3.14

  • 9.4.0

Please check the Fix Version/s of CLDSRV-955, or the target
branch of this pull request.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
8490 2 8488 0
View the top 1 failed test(s) by shortest run time
should log correct bucketGetPolicy operation with all required fields::Server Access Logs - File Output With default signature should log correct bucketGetPolicy operation with all required fields
Stack Traces | 0.031s run time
Expected 3 log entries, got 4

4 !== 3
View the full list of 3 ❄️ flaky test(s)
"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With default signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 192 times)

Stack Traces | 0.014s run time
We encountered an internal error. Please try again.
"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With v4 signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 228 times)

Stack Traces | 0.013s run time
We encountered an internal error. Please try again.
should overwrite an existing part by copying a part::Object Part Copy With v4 signature When copy source was put by MPU should overwrite an existing part by copying a part

Flake rate in main: 13.81% (Passed 668 times, Failed 107 times)

Stack Traces | 30s run time
Connection timed out after 30000 ms

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@tcarmet

tcarmet commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

node_modules cache - CI timing impact

Measured on a warm run of this branch (run 29055930710, the fully-warm attempt) vs a representative successful development/9.3 run (28062248343). The branch's own first (cold) run is excluded on purpose - it only populates the cache and carries the change, so it is not a fair baseline.

Job dev/9.3 dependency setup this PR (warm) setup saved job wall (dev → PR)
lint 61s (ts 2s + install 59s) 9s (restore 8s, install skipped) −52s 116s → 49s
unit-tests 54s 10s −44s 114s → 64s
file-ft-tests* 84s (Setup CI environment) 41s (Setup CI environment) −43s 750s → 722s

* Functional jobs install via the setup-ci composite action, which also does non-yarn work (hosts, credentials, python, telemetry). Only the yarn portion is skipped on a cache hit, hence the ~41s residual.

Takeaways (honest)

  • Dependency setup drops ~43–52s per job; the cache-restore step itself costs ~8–9s and is already netted into the numbers above.
  • This applies to ~15 jobs (4 inline + ~11 functional via setup-ci). Since they run in parallel, the end-to-end wall-time win is bounded by the critical path (~30–40s on the slowest functional job). The larger effect is total CI compute, which drops by roughly 15 × ~45s ≈ ~10 min per run.
  • Short jobs benefit most in wall time: lint 116→49s, unit-tests 114→64s (>2× faster feedback).
  • Cache key is ${{ runner.os }}-node-modules-${{ hashFiles('yarn.lock') }}; a lockfile bump invalidates it and the next run repopulates.

Note: the functional suite is independently flaky on the shared MPU abort-race teardown (test/object/mpuVersion.js, CLDSRV-938) - unrelated to this change; this run needed one rerun of that single test to go green.

@tcarmet tcarmet marked this pull request as ready for review July 10, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants