Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CI: try to make caching more reliable #25259

Merged
merged 11 commits into from
Sep 14, 2022
Merged

CI: try to make caching more reliable #25259

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ccab23d
CI: updated caching strategy
kassens Sep 13, 2022
0268eb2
add missing node_modules
kassens Sep 13, 2022
565d8df
always cache yarn cache instead of partially node_modules
kassens Sep 13, 2022
9e0d024
Revert "always cache yarn cache instead of partially node_modules"
kassens Sep 14, 2022
3db7f85
bump node_modules cache to v2
kassens Sep 14, 2022
8377a51
include epoch in node_modules cache
kassens Sep 14, 2022
8866f9f
use revision instead of epoch
kassens Sep 14, 2022
af45c32
also update key for restore step
kassens Sep 14, 2022
8fc2ca6
remove prepare_node_modules_cache_key
kassens Sep 14, 2022
58f3e66
add retry
kassens Sep 14, 2022
92d0e84
add comment about cache key per revision
kassens Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 90 additions & 64 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,93 @@ aliases:
- &restore_yarn_cache
restore_cache:
name: Restore yarn cache
key: v2-node-{{ arch }}-{{ checksum "yarn.lock" }}-yarn
keys:
- v1-yarn_cache-{{ arch }}-{{ checksum "yarn.lock" }}
- v1-yarn_cache-{{ arch }}-
- v1-yarn_cache-

- &yarn_install
run:
name: Install dependencies
command: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn

- &yarn_install_retry
run:
name: Install dependencies (retry)
when: on_fail
command: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn

- &save_yarn_cache
save_cache:
name: Save yarn cache
key: v1-yarn_cache-{{ arch }}-{{ checksum "yarn.lock" }}
paths:
- ~/.cache/yarn

- &restore_yarn_cache_fixtures_dom
restore_cache:
name: Restore yarn cache for fixtures/dom
keys:
- v1-yarn_cache-{{ arch }}-{{ checksum "yarn.lock" }}-fixtures/dom
- v1-yarn_cache-{{ arch }}-{{ checksum "yarn.lock" }}
- v1-yarn_cache-{{ arch }}-
- v1-yarn_cache-

- &yarn_install_fixtures_dom
run:
name: Install dependencies in fixtures/dom
working_directory: fixtures/dom
command: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn

- &prepare_node_modules_cache_key
- &yarn_install_fixtures_dom_retry
run:
name: Preparing node_modules cache key
command: yarn workspaces info | head -n -1 > workspace_info.txt
name: Install dependencies in fixtures/dom (retry)
when: on_fail
working_directory: fixtures/dom
command: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn

- &save_yarn_cache_fixtures_dom
save_cache:
name: Save yarn cache for fixtures/dom
key: v1-yarn_cache-{{ arch }}-{{ checksum "yarn.lock" }}-fixtures/dom
paths:
- ~/.cache/yarn

- &save_node_modules
save_cache:
name: Save node_modules cache
# Cache only for the current revision to prevent cache injections from
# malicious PRs.
key: v1-node_modules-{{ arch }}-{{ .Revision }}
paths:
- node_modules
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just node_modules is actually insufficient as yarn workspaces create node_modules directories in each project.

This feels tricky to get stable. Maybe we should include the yarn install, but restore the yarn cache so the yarn install can make use of that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that that was what we do?

- packages/eslint-plugin-react-hooks/node_modules
- packages/react-art/node_modules
- packages/react-client/node_modules
- packages/react-devtools-core/node_modules
- packages/react-devtools-extensions/node_modules
- packages/react-devtools-inline/node_modules
- packages/react-devtools-shared/node_modules
- packages/react-devtools-shell/node_modules
- packages/react-devtools-timeline/node_modules
- packages/react-devtools/node_modules
- packages/react-dom/node_modules
- packages/react-interactions/node_modules
- packages/react-native-renderer/node_modules
- packages/react-reconciler/node_modules
- packages/react-server-dom-relay/node_modules
- packages/react-server-dom-webpack/node_modules
- packages/react-server-native-relay/node_modules
- packages/react-server/node_modules
- packages/react-test-renderer/node_modules
- packages/react/node_modules
- packages/scheduler/node_modules

- &restore_node_modules
restore_cache:
name: Restore node_modules cache
keys:
- v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }}-{{ checksum "workspace_info.txt" }}-node-modules
- v1-node_modules-{{ arch }}-{{ .Revision }}

- &TEST_PARALLELISM 20

Expand Down Expand Up @@ -49,35 +124,18 @@ jobs:
name: Nodejs Version
command: node --version
- *restore_yarn_cache
- run:
name: Install Packages
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run: yarn workspaces info | head -n -1 > workspace_info.txt
- save_cache:
# Store the yarn cache globally for all lock files with this same
# checksum. This will speed up the setup job for all PRs where the
# lockfile is the same.
name: Save yarn cache for future installs
key: v2-node-{{ arch }}-{{ checksum "yarn.lock" }}-yarn
paths:
- ~/.cache/yarn
- save_cache:
# Store node_modules for all jobs in this workflow so that they don't
# need to each run a yarn install for each job. This will speed up
# all jobs run on this branch with the same lockfile.
name: Save node_modules cache
# This cache key is per branch, a yarn install in setup is required.
key: v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }}-{{ checksum "workspace_info.txt" }}-node-modules
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why we would include the branch name in the caching. This seems to just needlessly split the cache keys (wastes cache storage and more cache misses)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you add a new dependency in a branch? Doesn't the cache need to break so you install the dependency?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answer: In that case the yarn.lock would change for that branch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that doesn't have anything todo with the branch (name?) right? We might install a new dependency on the main branch or re-use a branch name across multiple PRs.

My thinking is that yarn.lock + workspace_info should capture everything relevant.

If we're feeling unsure about re-using the node_modules cache across PRs, we could also use the git revision for the key (I saw that's available in the docs)

paths:
- node_modules
- *restore_node_modules
- *yarn_install
- *yarn_install_retry
- *save_yarn_cache
- *save_node_modules

yarn_lint:
docker: *docker
environment: *environment

steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: node ./scripts/prettier/index
- run: node ./scripts/tasks/eslint
Expand All @@ -92,7 +150,6 @@ jobs:

steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: node ./scripts/tasks/flow-ci

Expand All @@ -102,7 +159,6 @@ jobs:

steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
command: |
Expand All @@ -119,7 +175,6 @@ jobs:
parallelism: 40
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: yarn build-combined
- persist_to_workspace:
Expand All @@ -135,7 +190,6 @@ jobs:
type: string
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Download artifacts for revision
Expand All @@ -153,7 +207,6 @@ jobs:
environment: *environment
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Download artifacts for base revision
Expand Down Expand Up @@ -182,7 +235,6 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: echo "<< pipeline.git.revision >>" >> build/COMMIT_SHA
# Compress build directory into a single tarball for easy download
Expand All @@ -202,7 +254,6 @@ jobs:
- attach_workspace:
at: .
- run: echo "<< pipeline.git.revision >>" >> build/COMMIT_SHA
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
command: node ./scripts/tasks/danger
Expand All @@ -214,11 +265,7 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Install Packages
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run:
environment:
RELEASE_CHANNEL: experimental
Expand All @@ -233,11 +280,7 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Install Packages
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run:
name: Playwright install deps
command: |
Expand All @@ -259,11 +302,7 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Install nested packages from Yarn cache
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run: ./scripts/circleci/download_devtools_regression_build.js << parameters.version >> --replaceBuild
- run: node ./scripts/jest/jest-cli.js --build --project devtools --release-channel=experimental --reactVersion << parameters.version >> --ci

Expand All @@ -278,11 +317,7 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Install nested packages from Yarn cache
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run:
name: Playwright install deps
command: |
Expand All @@ -306,7 +341,6 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: yarn lint-build

Expand All @@ -317,7 +351,6 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: yarn check-release-dependencies

Expand All @@ -328,7 +361,6 @@ jobs:
steps:
- checkout
- attach_workspace: *attach_workspace
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Search build artifacts for unminified errors
Expand All @@ -345,7 +377,6 @@ jobs:
type: string
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run: yarn test <<parameters.args>> --ci

Expand All @@ -360,11 +391,7 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Install nested packages from Yarn cache
command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- run: yarn test --build <<parameters.args>> --ci

RELEASE_CHANNEL_stable_yarn_test_dom_fixtures:
Expand All @@ -374,15 +401,17 @@ jobs:
- checkout
- attach_workspace:
at: .
- *prepare_node_modules_cache_key
- *restore_node_modules
- *restore_yarn_cache_fixtures_dom
- *yarn_install_fixtures_dom
- *yarn_install_fixtures_dom_retry
- *save_yarn_cache_fixtures_dom
- run:
name: Run DOM fixture tests
environment:
RELEASE_CHANNEL: stable
working_directory: fixtures/dom
command: |
cd fixtures/dom
yarn --frozen-lockfile
yarn prestart
yarn test --maxWorkers=2

Expand All @@ -391,7 +420,6 @@ jobs:
environment: *environment
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Run fuzz tests
Expand All @@ -411,7 +439,6 @@ jobs:
environment: *environment
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Run publish script
Expand All @@ -430,7 +457,6 @@ jobs:
environment: *environment
steps:
- checkout
- *prepare_node_modules_cache_key
- *restore_node_modules
- run:
name: Fetch revisions that contain an intentional fork
Expand Down