From 7775584bbf0f06d06e98ee78a50ffb8bccb64bd4 Mon Sep 17 00:00:00 2001 From: Jeffrey Ying Date: Fri, 14 Aug 2026 18:03:29 -0400 Subject: [PATCH] ateom-microvm: unlink the merge destination before renaming onto it (#921) `MergeSparseOverlay` ended with a rename onto an existing `outFile`. Renaming over an existing file makes [ext4](https://github.com/torvalds/linux/blob/v6.17/Documentation/admin-guide/ext4.rst#L317-L328) (`data=ordered`) write back the renamed file's dirty pages, and the scratch file carries the whole merged memory image, so the merge pays a full flush of it. `MergeDeltaIntoBase` (see [link](https://github.com/agent-substrate/substrate/blob/a4121a4ca438603481f1799c85928c78a4ba9a6a/cmd/ateom-microvm/internal/ch/merge.go#L150-L158)) already unlinks first for this reason. On the counter demo the merge goes from ~1140 ms to ~115 ms. This is a no-op today, `MergeSparseOverlay` only runs when base and delta land on different filesystems. It becomes the common path in the follow-up that routes hardlinked restores through it, and landing first keeps that change from carrying a 1.1 s suspend regression. --- cmd/ateom-microvm/internal/ch/merge.go | 11 ++++++++ cmd/ateom-microvm/internal/ch/merge_test.go | 31 +++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/cmd/ateom-microvm/internal/ch/merge.go b/cmd/ateom-microvm/internal/ch/merge.go index e20f18ce7..3a0f7b329 100644 --- a/cmd/ateom-microvm/internal/ch/merge.go +++ b/cmd/ateom-microvm/internal/ch/merge.go @@ -84,6 +84,17 @@ func MergeSparseOverlay(ctx context.Context, baseFile, deltaFile, outFile string if err := o.Close(); err != nil { return err } + // Put the merged image at outFile's name. Unlink the old one FIRST, then rename + // onto the now-free name, for the same reason as MergeDeltaIntoBase's final + // rename below: renaming OVER an existing file makes ext4 (data=ordered) + // synchronously write back the renamed file's dirty pages, and `tmp` carries the + // whole merged image. Renaming to a non-existent name skips that flush (the + // dirty pages stay in page cache for atelet to ship), taking the merge + // ~1140ms→~115ms. Unlike there, outFile need not exist: this is also called to + // write a merged image to a fresh path. + if err := os.Remove(outFile); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove old out: %w", err) + } return os.Rename(tmp, outFile) } diff --git a/cmd/ateom-microvm/internal/ch/merge_test.go b/cmd/ateom-microvm/internal/ch/merge_test.go index 43a628d2a..e9376ede3 100644 --- a/cmd/ateom-microvm/internal/ch/merge_test.go +++ b/cmd/ateom-microvm/internal/ch/merge_test.go @@ -152,6 +152,37 @@ func TestMergeDeltaIntoBase(t *testing.T) { } } +// TestMergeSparseOverlayNewOutFile covers the copying merge writing to an outFile +// that does not exist yet: the unlink before its final rename has to tolerate a +// missing destination rather than fail the merge. TestMergeDeltaIntoBase above +// already covers the case where outFile does exist. +func TestMergeSparseOverlayNewOutFile(t *testing.T) { + const size = 8 << 20 // 8 MiB logical + baseRegions := []region{{off: 0, data: fill(1, 4096)}} + deltaRegions := []region{{off: 4 << 20, data: fill(42, 12345)}} + want := make([]byte, size) + copy(want[baseRegions[0].off:], baseRegions[0].data) + copy(want[deltaRegions[0].off:], deltaRegions[0].data) + + dir := t.TempDir() + base := filepath.Join(dir, "base") + delta := filepath.Join(dir, "delta") + out := filepath.Join(dir, "out") // deliberately never created + writeSparse(t, base, size, baseRegions) + writeSparse(t, delta, size, deltaRegions) + + if err := MergeSparseOverlay(context.Background(), base, delta, out); err != nil { + t.Fatalf("MergeSparseOverlay: %v", err) + } + got, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, want) { + t.Fatalf("merged result != expected (len got=%d want=%d)", len(got), len(want)) + } +} + // TestMergeDeltaIntoBaseSizeMismatch verifies a base/delta size mismatch is // refused (misaligned overlay would corrupt the image) rather than silently // producing garbage.