mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-11-13 10:43:28 +08:00
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.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user