Skip to content

[No QA] Prevent sticky disks from growing unboundedly - #98422

Merged
mountiny merged 10 commits into
Expensify:mainfrom
software-mansion-labs:collectioneur/test-sticky-disk
Aug 19, 2026
Merged

[No QA] Prevent sticky disks from growing unboundedly#98422
mountiny merged 10 commits into
Expensify:mainfrom
software-mansion-labs:collectioneur/test-sticky-disk

Conversation

@collectioneur

@collectioneur collectioneur commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

The problem

A Blacksmith sticky disk is not a tarball of files like actions/cache - it is an ext4 block device stored in Blacksmith's Ceph cluster. When a job mounts one, the last committed snapshot is cloned and mounted at the given path, when the job finishes, the clone is committed back and becomes the new snapshot.

The important part is when that commit happens: the sticky disk action commits in its post-job phase, so the snapshot always captures the state of the mounted path at the very end of the job. There is no API to commit at the end of a specific step - we cannot say "snapshot node_modules right after npm ci and ignore everything written afterwards". Everything a job writes into the mount before it ends is part of the snapshot.

That matters because plenty of tooling writes into node_modules during a job:

  • ESLint, Babel, rspack and other tools default their caches to node_modules/.cache.
  • Android builds compile native React Native libraries from source, and Gradle/CMake write their outputs straight into the packages: node_modules/<lib>/android/build, .cxx object files per ABI, generated codegen sources.

Because every job committed (commit defaults to true), each run layered its own generated files into the snapshot, and the next run started from that fatter snapshot and added more. It compounds, and on the block level it is worse than the file listing suggests: deleting or rewriting a file inside the mount frees ext4 blocks, but the freed blocks are never returned to the storage layer - fstrim on a sticky disk mount fails with the discard operation is not supported. The billed size therefore tracks every block ever written, not the blocks currently in use.

The result: node_modules that is 3.8 GB after a clean npm ci grew into disks reporting 30 GB of live files and 103 GB of allocated blocks in the Blacksmith dashboard after ~3 weeks on the same key. Inspecting one of those disks showed exactly the expected culprits - react-native-reanimated at 4.0 GB, react-native-worklets at 3.2 GB, react-native-quick-crypto at 2.9 GB, and so on for every library with native code, against ~50 MB each when freshly installed. Sticky disk storage is billed at $0.50/GB/month, so a single such key cost ~$50/month on its own.

The fix

Separate "who produces the snapshot" from "who consumes it". setupNode takes a new SEED_STICKY_DISKS input that controls the commit mode of the node_modules disks:

  • The new seedStickyDisks.yml workflow passes SEED_STICKY_DISKS: 'true' and gets commit: if-missing. It installs dependencies and does nothing else, so the snapshot it writes is a clean npm ci result by construction. It runs on every push to main, once a key has a snapshot, the seed is a no-op that mounts it and exits.
  • Every other workflow keeps the default and mounts with commit: false. Jobs still read node_modules from the snapshot and may write whatever they like into their own clone - caches, Gradle output, .cxx and all of it is discarded when the clone is unmounted instead of being committed. Disks stay at the size of a clean install permanently.
  • The ~/.npm disk keeps committing, with commit: on-change. Its key has no hash in it, so it has to keep absorbing new package tarballs across lockfile bumps, and it does not suffer the same growth: tarballs are content-addressed, it sits at ~6 GB, and on-change skips the commit on the majority of runs that never touch it.

Cache-hit detection is unchanged: the node_modules/.stickydisk-installed marker is written at the end of setupNode, so it lands in the seed's snapshot and consumers skip npm ci when they see it.

Fixed Issues

$ #98593
$ #98563

Tests

No QA

Offline tests

N/A

QA Steps

No QA

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

@collectioneur collectioneur changed the title Test Sticky Disks [No QA] Prevent sticky disks from growing unboundedly Aug 13, 2026
@collectioneur
collectioneur marked this pull request as ready for review August 13, 2026 11:13
@collectioneur
collectioneur requested a review from a team as a code owner August 13, 2026 11:13
@melvin-bot
melvin-bot Bot requested review from gijoe0295 and removed request for a team August 13, 2026 11:13
@melvin-bot

melvin-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

@gijoe0295 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b01a28887a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/seedStickyDisks.yml Outdated
with:
key: ${{ format('{0}-{1}-{2}-node-modules-{3}', github.repository, runner.os, runner.arch, inputs.IS_HYBRID_BUILD == 'true' && hashFiles('normalized-package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**') || hashFiles('normalized-package-lock.json', 'patches/**')) }}
path: node_modules
commit: ${{ inputs.SEED_STICKY_DISKS == 'true' && 'if-missing' || 'false' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Rotate keys or replace existing snapshots

The pinned stickydisk action defines if-missing as committing only when no prior snapshot existed, so any already-bloated node_modules disk for the unchanged key will be mounted, treated as a hit via .stickydisk-installed, and then skipped by the seed instead of being replaced. Because regular consumers now use commit: false, those 30–100GB snapshots remain active as long as CI keeps mounting them; use a new key/version or a one-time delete/reseed path so existing contaminated disks are actually replaced.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, we need to remove all the previous sticky disks using the Blacksmith panel when this PR hits main, since these disks are bloated and some of them reach 80 GB
сс: @mountiny

Screenshot 2026-08-13 at 13 42 12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok I can do that when we are ready

@collectioneur

Copy link
Copy Markdown
Contributor Author

@mountiny all comments are resolved. Could you please take a look? 🙂

@mountiny

Copy link
Copy Markdown
Contributor

@MelvinBot review

@MelvinBot

Copy link
Copy Markdown
Contributor

Review — looks solid ✅

The core mechanism is right: separating producer (SEED_STICKY_DISKS: 'true'commit: if-missing) from consumer (commit: false) is exactly what caps the growth, and on-change for ~/.npm is the correct exception since that key is unhashed and content-addressed. Nice, well-documented change. A few things worth confirming before merge — none are blockers.

1. PR body no longer matches the workflow trigger

The description says the seed "runs on every pull request and every push to main", but seedStickyDisks.yml only triggers on push to main (the pull_request trigger was removed in a later commit). Worth updating the body so the caching reasoning stays accurate — this directly affects point 2.

2. Dependency-bump PRs lose the cache for their whole lifetime

Because the seed only runs on main, a PR that changes package-lock.json or patches/** produces a new key that has no seeded snapshot. With consumers now mounting commit: false, every job in that PR runs a full uncached npm ci (and none commit), for the entire life of the PR — only after merge does the main push seed that key. Previously the first job would commit and later jobs/re-runs hit the cache. This is a reasonable tradeoff (dep-bump PRs are relatively rare), but please confirm it's acceptable — on a fan-out PR with many parallel setupNode jobs it means N × npm ci instead of 1.

3. if-missing won't reclaim the existing bloat

For keys that already have a snapshot today (the 30 GB / 103 GB ones), the seed's if-missing will skip the commit since the snapshot exists. So growth stops immediately, but the current bloated snapshots persist until the next lockfile bump rotates the key. If you want the savings now rather than "eventually," you may want a one-time manual purge of the current sticky disks in the Blacksmith dashboard.

Minor / non-blocking
  • Good call removing the pull_request trigger — seedHybrid checks out submodules with secrets.OS_BOTIFY_TOKEN, and keeping the seed on push-to-main only keeps that secret away from fork PRs.
  • seedStandalone and seedHybrid run in parallel and both mount the same ~/.npm key with commit: on-change, so they can race on that commit. Harmless (content-addressed, last-writer-wins), just noting it.
  • No breakage for consumer workflows that relied on cross-job persistence: setupNode always runs npm ci when the .stickydisk-installed marker is absent, so each consumer job is self-sufficient off the base snapshot.

CI-only change with no runtime/app surface, so there's nothing to exercise in-app here.

@collectioneur

Copy link
Copy Markdown
Contributor Author
  1. Changed the description ✅
  2. We discussed this with Cole in this thread. We agreed to try enabling sticky disks for dependency-changing PRs too, but first, he will ask the Blacksmith team if they already have a solution for this or if it's on their roadmap.
  3. All old disks will be deleted, so there’s no problem here 👌

@mountiny

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Android: mWeb Chrome
iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / Safari

@mountiny
mountiny merged commit 309fdc8 into Expensify:main Aug 19, 2026
16 of 18 checks passed
@mountiny

Copy link
Copy Markdown
Contributor

Deleted the sticky disks
image

@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.57-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/Beamanator in version: 9.4.57-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

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.

5 participants