Skip to content

git-mirror: fstrim the mirror sticky disk before unmount at commit - #63

Open
magma-beef wants to merge 9 commits into
mainfrom
devin/1792451273-git-mirror-fstrim
Open

magma-beef wants to merge 9 commits into
mainfrom
devin/1792451273-git-mirror-fstrim

Conversation

@magma-beef

@magma-beef magma-beef commented Oct 20, 2026 •

Copy link
Copy Markdown

In git-mirror cleanup, run fstrim on the mirror sticky disk before it is unmounted and committed, so freed blocks are discarded from the backing image.

  • The agent sends a trim budget in GetStickyDiskResponse.fstrim_timeout_secs and a minimum free-extent size in fstrim_min_extent_bytes. Mount stores both in action state; a 0/absent budget disables the trim.
  • Cleanup trims only when the budget is positive and the mirror will be committed (shouldCommit), after sync/maintenance and before umount.
  • Best-effort: sudo timeout <budget> fstrim -v [-m <bytes>]; timeout runs under sudo so it can stop root's fstrim when the budget expires. The result is logged in one line as ok / timeout / unsupported (discard not negotiated, or no fstrim binary) / failed, the same vocabulary as setup-docker-builder and the agent's FstrimOutcome, and never fails the step.

Start here: src/blacksmith-cache.ts (trimFilesystem, the trim in cleanup); src/state-helper.ts, src/git-source-provider.ts, src/main.ts only carry the values from mount to cleanup. dist/ is regenerated.

Checks: npm run lint, npx tsc --noEmit -p ., npm test, npm run build.

Link to Devin session: https://app.devin.ai/sessions/2e17815c306b4c3cb7af4f6bce008a66
Open in Devin Desktop: https://app.devin.ai/desktop/session/2e17815c306b4c3cb7af4f6bce008a66?variant=devin
Requested by: @magma-beef


🟢 Devin Review found nothing to fix · View findings · Follow Devin's work

Devin Review


View with [code]smith Generate Proof with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


View full Proof
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need. (Staging)

magma-beef and others added 3 commits October 19, 2026 23:07
Deleted packs keep their RBD extents allocated on the backing store until
the guest discards them. Run a best-effort fstrim on the mirror mount
after sync and before unmount, only when the run will commit and only
when the host enabled it: the VM agent returns
GetStickyDiskResponse.fstrim_timeout_secs (fa live config
guest_fstrim_timeout_secs), saved in action state at mount time; 0 means
no trim. Any failure or timeout only logs.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…oto package predates it

The bundled GetStickyDiskResponse has no fstrim_timeout_secs yet, and protobuf-es keeps fields it does not know in the message's unknown-field list (the gRPC transport is binary), so decode field 8 from there when the typed field is absent. Releases built before the field existed therefore honour the host budget once the agent sends it.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@blacksmith-staging

blacksmith-staging Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Matches Post step trims mirror disk before unmount

With a 30s budget and a committing job, the post step ran fstrim (logged ok) after sync and before umount, and the backing image dropped from 515M to 2.7M; before the PR it stayed at 515M.

Merge-base

Before: 515M kept

This PR 5580ba7a

After: trimmed to 2.7M

Also checked · 3 match

Matches Agent budget reaches the post step
Matches No trim when disabled or not committing
Matches Trim failure or timeout never fails the step

Proof on 5580ba7a (from dcf05dcc) · mocked: Blacksmith VM agent (StickyDiskService g... (unavailable), Mirror sticky disk (unavailable) · Did Proof get it right? Yes · No

View full Proof report

magma-beef and others added 5 commits October 23, 2026 00:08
Bump @buf/blacksmith_vm-agent.bufbuild_es to a build that includes
GetStickyDiskResponse.fstrim_timeout_secs (fa #6600) and drop the
unknown-field-8 decoding.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ackages

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
GetStickyDiskResponse.fstrim_min_extent_bytes (fa #6823) is saved as action
state next to the timeout budget and, when positive, becomes
`fstrim -m <bytes>` so free extents below the host's threshold are not
discarded. Zero or absent leaves fstrim at its default. Bumps the @buf
vm-agent packages to the fa main build that carries field 9 and regenerates
their licensed records.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
An unprivileged timeout cannot signal the root-owned sudo/fstrim it
spawned, so the host budget never bounded the trim and fstrim could
outlive it and keep the mirror mount busy. sudo timeout <secs> fstrim
gives timeout the privilege to kill fstrim; exit 124 still maps to the
timeout outcome.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

Same vocabulary as setup-docker-builder and the agent's FstrimOutcome, so
logs across the actions read alike and a discard that the host has not
negotiated yet is not mixed in with real failures.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.

1 participant