[magma-beef]

In the post step, run fstrim on the buildkit 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. Setup reads the typed field (@buf/blacksmith_vm-agent.bufbuild_es regenerated from fa after FastActions/fa#6600) and stores it in action state; 0/absent disables the trim.
• Post trims only when the budget is positive and the run is still on track to commit (expose id present, no early deny, no SIGKILL, no failed previous steps). The trim runs after buildkitd shutdown and sync, before the usage sample and umount.
• Best-effort: timeout <budget> sudo fstrim -v; a missing binary, unsupported discard, or timeout only logs and never fails the step.
• The trim result (outcome ok/timeout/unsupported/failed/skipped, duration, fstrim -v bytes) is recorded on the in-memory JobLifecycle. It is sent in DockerJobLifecycle once the proto package is regenerated from FastActions/fa#6812 (fields 28–30); until then shipTeardownReport omits it.

Start here: src/main.ts (post step, trimFilesystem), then src/setup_builder.ts (parseFstrimTimeoutSecs, fstrimFailureOutcome, parseFstrimBytes). dist/ is regenerated.

Checks: npm run lint, npx tsc --noEmit -p ., npx vitest run, 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

---

^{Need help on this PR? Tag @codesmith-bot with what you need. (Staging)}

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[devin-ai-integration]

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

[devin-ai-integration]

✅ Devin Review: No Issues Found

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

[blacksmith-staging]

Post step trims freed blocks before unmount

After the change the post step runs fstrim before unmount and the backing image drops from 578M to 66M; before, it stays at 578M.

_Merge-base

_{This PR 48d373d0}

Also checked · 5 match

Budget read from field 8 over gRPC
No trim when previous steps failed
Trim failure never fails the step
fstrim result reaches the lifecycle report
Host minimum extent passed as fstrim -m

_{Proof on 48d373d0 (from 44137f89)}

View full Proof report

[cursor]

This PR is too large for Bugbot to review. It changes 9 lines and 1,709,694 characters. Split the change into smaller pull requests to get a review.

[devin-ai-integration]

✅ Devin addressed Devin Review findings on 48d373d

No findings needed changes.

View all findings in Devin Review

[piob-io]

Drop reading the unknown field 8. No hacks like that. We should get fresh proto definition (fa 6600 is merged now), and use it here.

[piob-io]

The trim result is only core.info'd; nothing lands in DockerJobLifecycle, so we can't see what a budget costs or reclaims before widening it. Please add fstrim_duration_ms, an outcome (ok / timeout / unsupported / skipped) and bytes trimmed (from fstrim -v) next to buildkitd_shutdown_duration_ms — proto + CH column in fa, then populate them in shipTeardownReport here.

[piob-io]

Should we also control how big blocks we want fstrim to communicate, if there is such setting?

[devin-ai-integration]

Drop reading the unknown field 8. No hacks like that. We should get fresh proto definition (fa 6600 is merged now), and use it here.

Done in cf89a22: the unknown-field decoding is gone. @buf/blacksmith_vm-agent.bufbuild_es is bumped to 2.16.0-20261022235850-0cca692559b2.1 (generated from fa main after #6600), which exposes GetStickyDiskResponse.fstrimTimeoutSecs, and setup now reads parseFstrimTimeoutSecs(response.fstrimTimeoutSecs). @bufbuild/protobuf moved to ^2.16.0 for the generated package's peer dep. Same cleanup pushed to stickydisk #82 (b19c797) and checkout #63 (2a8ace2).

[devin-ai-integration]

The trim result is only core.info'd; nothing lands in DockerJobLifecycle … Please add fstrim_duration_ms, an outcome (ok / timeout / unsupported / skipped) and bytes trimmed (from fstrim -v) next to buildkitd_shutdown_duration_ms — proto + CH column in fa, then populate them in shipTeardownReport here.

Agreed. Split across three PRs because the proto has to merge before this action can serialize it:

• FastActions/fa#6812 — DockerJobLifecycle.fstrim_duration_ms / fstrim_outcome / fstrim_bytes (fields 28–30, outcome adds failed for errors that are neither timeout nor EOPNOTSUPP/missing binary) → docker_build_summary.fstrim_ms / fstrim_outcome / fstrim_bytes next to buildkitd_shutdown_ms.
• useblacksmith/web#19044 — the ClickHouse migration (summary columns + docker_build_daily sums and fstrim_outcome_map). Must land before the agent rolls.
• here, 44137f8 — trimFilesystem returns {outcome, durationMs, bytes} (bytes parsed from fstrim -v), the post step stores them on the JobLifecycle. The three reportDockerBuild lines that put them in the message come in one more commit after #6812 merges and the @buf package is regenerated — that is also why Proof currently flags "fstrim result reaches the lifecycle report" as differing. I'll push that as soon as the package exists.

[devin-ai-integration]

Should we also control how big blocks we want fstrim to communicate, if there is such setting?

There is one: fstrim -m <bytes> (--minimum) skips free extents smaller than that; -o/-l bound the range. There is no "block size" knob — the kernel splits each free extent into discards according to what the device advertises, and storageproxy advertises discard_sector_alignment = 1 with max_discard_sectors = u32::MAX (transport/vhost_user.rs), so the guest sends ext4's free-extent map as-is, 4 KiB-granular.

What happens to a small extent downstream: storageproxy passes each guest discard range 1:1 to rbd_aio_discard on the clone (transport/backend.rs → remote/librbd.rs), and librbd with its default rbd_skip_partial_discard=true / rbd_discard_granularity_bytes=64K drops pieces under 64 KiB, turns the rest of a partial object into a zero/truncate op (BlueStore frees at min_alloc_size, the object stays), and only a range covering a whole 4 MiB object deletes the object. So extents < 64 KiB are pure overhead, and the bulk of the byte reclaim comes from the ≥ 4 MiB extents. A floor like -m 1M would cut the RADOS op count on a fragmented disk a lot while giving up little.

I would not hard-code it in the action though, for the same reason the timeout isn't: make it a second agent-sent value next to the budget (GetStickyDiskResponse.fstrim_min_extent_bytes, 0 = fstrim default, from a guest_fstrim_min_extent_bytes live-config key), and pick the number from data — the duration/bytes telemetry this PR now records, plus storageproxy's discard counters, will show whether small extents are actually eating the budget. Happy to add that field in the fa follow-up now if you'd rather have the knob in place before the first rollout; otherwise I'd leave it until the first week of fstrim_ms numbers says it's needed.

[piob-io]

Do we need to advertise sectors at the ceph object granularity, to actually free bytes at ceph level? Or we somehow combine this on the proxy side?

Additionally, it does feel like it would be useful to also have a live config knob to skip free extents smaller than configured threshold.

[devin-ai-integration]

Do we need to advertise sectors at the ceph object granularity, to actually free bytes at ceph level? Or we somehow combine this on the proxy side?

No to both, as far as the code goes. The proxy does not coalesce: each guest discard range is one plan_guest_discard → rbd_aio_discard(offset, len) call (transport/backend.rs, drive.rs), and it advertises discard_sector_alignment = 1, max_discard_sectors = u32::MAX, so the guest can send a range of any size. Whether bytes come back depends on what librbd makes of each range:

• Range covers whole 4 MiB objects → those objects are deleted; the object is freed in full.
• Range covers part of an object → librbd issues a zero/truncate on that object; BlueStore punches the hole and releases the allocation at min_alloc_size granularity (4 KiB on SSD OSDs), so bytes are reclaimed, just not the whole object.
• Partial-object ranges smaller than rbd_discard_granularity_bytes (64 KiB default) are dropped by librbd when rbd_skip_partial_discard is on (the default) — those cost a round trip and free nothing.

So a 4 MiB alignment is not needed to free bytes; the trade-off is per-request overhead (one RADOS op per object touched) vs. bytes. ext4's FITRIM already emits one discard per contiguous free extent (bounded by the 128 MiB block group), so a disk that freed a few big layers sends few, large ranges; the pathological case is many small holes, which is exactly what the minimum-extent knob below is for. The one real gap on the proxy side is unrelated to granularity: before promotion a discard is LocalOnly and is never replayed to Ceph after the clone is promoted (tracked as the follow-up from fa#6600).

it does feel like it would be useful to also have a live config knob to skip free extents smaller than configured threshold.

Agreed, added:

• FastActions/fa#6823 — live config guest_fstrim_min_extent_bytes → GetStickyDiskResponse.fstrim_min_extent_bytes (int64, field 9); 0/absent = fstrim default.
• useblacksmith/web#18629 — now accepts both fstrim keys (int, 0..1 GiB for the new one).
• This PR, stickydisk#82 and checkout#63 will pass fstrim -m <bytes> in the same follow-up commit that serializes the telemetry fields, once the @buf package is regenerated from fa main with fields 9 and 28–30.

Suggested first value once enabled: 64 KiB (librbd's partial-discard floor), raise toward 4 MiB only if fstrim_ms shows the per-request cost dominating.

[piob-io]

I still think that we should force the 4MiB alignment, to avoid wasting time and resources on ops that just get ignored. The knob is different - it's a trade-off decision, between deleting more data and spending less time on fstrims. And while closely related, these two settings apply at different levels, no?

[devin-ai-integration]

Agreed they are different levels: the knob is a policy choice (bytes vs. trim time) per host, the device limit is what the guest kernel is allowed to send. Two facts on what the device limit actually does before we pick the value:

1. discard_sector_alignment becomes the guest's queue/discard_granularity (virtio_blk sets q->limits.discard_granularity from it). ext4's ext4_trim_fs clamps minlen up to that granularity, so it acts as a hard floor on free-extent size — but it does not align ranges to boundaries: a 6 MiB free extent starting 1 MiB into an object is still one discard, and librbd splits it into one full-object delete plus two partial-object zeroes. So a 4 MiB alignment removes the small extents but not the partial-object ops at the ends of big ones.
2. The ops that are really ignored are partials < rbd_discard_granularity_bytes (64 KiB); 64 KiB..4 MiB partials do free bytes at BlueStore's alloc granularity, they just cost a RADOS op each.

So, concretely:

• Advertise discard_sector_alignment = 64 KiB (128 sectors) in storageproxy: no guest can emit an op librbd drops, zero bytes forfeited. I'd do this unconditionally.
• If you want whole-object-only, the place that can enforce it is plan_guest_discard in storageproxy: round each range inward to 4 MiB object boundaries and skip empties, with the device advertising 4 MiB granularity so ext4 does not bother with sub-object extents either. That trades the partial-object bytes for fewer ops per trim; the knob then only matters above 4 MiB.

Both are proxy-side changes and inert until Firecracker negotiates VIRTIO_BLK_F_DISCARD, so they can ship ahead. Which do you want — 64 KiB floor only, or 4 MiB floor + inward rounding? I'll open the fa PR accordingly.

[piob-io]

Why not discard_sector_alignment = 4MiB ?

[devin-ai-integration]

Only because of what the kernel does with it: discard_sector_alignment lands as queue/discard_granularity, which ext4 uses as a floor on free-extent size (ext4_trim_fs clamps minlen up to it) — it does not snap ranges to boundaries. So 4 MiB alone (a) drops every free extent under 4 MiB, including the 64 KiB–4 MiB partials that do free bytes today, and (b) still sends partial-object ops for the unaligned head/tail of every larger extent. Neither is a correctness problem; (b) just means it doesn't fully remove the ops you want gone.

If the goal is "no partial-object ops, ever", the combination is: advertise 4 MiB and round each range inward to object boundaries in plan_guest_discard, dropping empties. That gives exactly whole-object deletes and nothing else, and the -m knob is then only meaningful above 4 MiB. I'll open that as a small fa PR (inert until Firecracker negotiates VIRTIO_BLK_F_DISCARD) and link it here.

[devin-ai-integration]

Done in FastActions/fa#6827 (draft pending sbauer's nod): discard_sector_alignment = 4 MiB plus inward object rounding in plan_guest_discard, so Ceph only ever sees whole-object deletes. The -m knob (#6823) stays as the per-host policy above that.