diff --git a/storage/posix/files.go b/storage/posix/files.go index 3ad1e366f..a60a768ac 100644 --- a/storage/posix/files.go +++ b/storage/posix/files.go @@ -1388,7 +1388,10 @@ func (m *MirrorWriter) UpdateCheckpoint(ctx context.Context, fn func(old []byte) // If an implied partial resource is not already present, this function will attempt to create // it from a strictly larger resource whose presence is implied by treeSize. func (m *MirrorWriter) ensureGeometry(ctx context.Context, cpSize, treeSize uint64) error { - if cpSize == 0 { + // If cpSize is zero then no tree exists. + // If cpSize == treeSize the resources are guaranteed present by integration. + // In both cases there's nothing to do. + if cpSize == 0 || cpSize == treeSize { return nil } if cpSize > treeSize { @@ -1399,6 +1402,12 @@ func (m *MirrorWriter) ensureGeometry(ctx context.Context, cpSize, treeSize uint for l := uint64(0); l <= uint64(ml); l, idx = l+1, idx>>layout.TileHeight { treeP := layout.PartialTileSize(l, idx, treeSize) cpP := layout.PartialTileSize(l, idx, cpSize) + + if cpP == treeP { + // Nothing to be done at this level. + continue + } + if l == 0 { if err := m.ensurePartialBundle(ctx, idx, cpP, treeP); err != nil { return err @@ -1425,7 +1434,7 @@ func maxLevel(sz uint64) int { // If the implied partial entry bundle is not already present, this function will attempt to create // it from the entry bundle implied by treeSize. func (m *MirrorWriter) ensurePartialBundle(ctx context.Context, idx uint64, cpP, treeP uint8) error { - if cpP == treeP { + if cpP == 0 { return nil } @@ -1451,12 +1460,7 @@ func (m *MirrorWriter) ensurePartialBundle(ctx context.Context, idx uint64, cpP, return fmt.Errorf("failed to unmarshal entry bundle @%d.%d: %v", idx, treeP, err) } - // Then trim it down to the size implied by the checkpoint, and write it out. - // Handle cpP == 0 where a full-bundle is implied - we should never actually hit this case since cpP must - // equal treeP in this case, but it doesn't hurt to be defensive. - if cpP > 0 { - eb.Entries = eb.Entries[:cpP] - } + eb.Entries = eb.Entries[:cpP] d, err = marshalTlogEntryBundle(eb) if err != nil { return fmt.Errorf("failed to marshal entry bundle @%d.%d: %v", idx, cpP, err) @@ -1473,7 +1477,7 @@ func (m *MirrorWriter) ensurePartialBundle(ctx context.Context, idx uint64, cpP, // If the implied partial tile is not already present, this function will attempt to create // it from the tile implied by treeSize. func (m *MirrorWriter) ensurePartialTile(ctx context.Context, l uint64, idx uint64, cpP, treeP uint8) error { - if cpP == treeP { + if cpP == 0 { return nil } @@ -1499,12 +1503,7 @@ func (m *MirrorWriter) ensurePartialTile(ctx context.Context, l uint64, idx uint return fmt.Errorf("failed to unmarshal tile @%d/%d.%d: %v", l, idx, treeP, err) } - // Then trim it down to the size implied by the checkpoint, and write it out. - // Handle cpP == 0 where a full-tile is implied - we should never actually hit this case since cpP must - // equal treeP in this case, but it doesn't hurt to be defensive. - if cpP > 0 { - t.Nodes = t.Nodes[:cpP] - } + t.Nodes = t.Nodes[:cpP] d, err = t.MarshalText() if err != nil { return fmt.Errorf("failed to marshal tile @%d/%d.%d: %v", l, idx, cpP, err) diff --git a/storage/posix/files_test.go b/storage/posix/files_test.go index 8f54495b1..0ba1c6b1e 100644 --- a/storage/posix/files_test.go +++ b/storage/posix/files_test.go @@ -20,10 +20,10 @@ import ( "encoding/base64" "errors" "fmt" + "iter" "net/http" "os" "path/filepath" - "slices" "strings" "testing" "time" @@ -802,48 +802,33 @@ func TestMirrorWriter_UpdateCheckpointGeometry(t *testing.T) { t.Fatalf("MirrorWriter: %v", err) } - // Create 600 entries to span multiple tile levels. - entries := make([][]byte, 600) - for i := range 600 { - entries[i] = fmt.Appendf(nil, "entry %d", i) - } - - bundles := []*api.EntryBundle{ - {Entries: entries[0:256]}, - {Entries: entries[256:512]}, - {Entries: entries[512:600]}, - } - bundlesIter := func(yield func(*api.EntryBundle, error) bool) { - for _, b := range bundles { - if !yield(b, nil) { - return - } - } - } - - // Integrate to size 600. - // Tiles layout should then be: - // Level 1: [2] - // Level 0: [256] [256] [88] - // Entries: [256] [256] [88] - size, _, err := mw.IntegrateBundles(ctx, 0, bundlesIter) + // Create 66000 entries to span multiple tile levels and multiple tiles at level 1. + const treeSize = 66000 + size, _, err := mw.IntegrateBundles(ctx, 0, generateBundles(t, treeSize)) if err != nil { t.Fatalf("IntegrateBundles: %v", err) } - if size != 600 { - t.Fatalf("expected integrated size 600, got %d", size) + if size != treeSize { + t.Fatalf("expected integrated size %d, got %d", treeSize, size) } for _, test := range []struct { - size uint64 + size uint64 + // wantRHSizes[l] is the expected node count for the rightmost tile at level l: + // - 1..255: partial tile with that many nodes (read via .p/N) + // - 256: full tile (read via p=0, asserting 256 nodes) + // - 0: no tile exists at this coordinate for this checkpoint (asserts full tile file is absent) wantRHSizes []int }{ {0, []int{}}, {1, []int{1}}, {129, []int{129}}, {256, []int{256, 1}}, - {300, []int{44, 2}}, + {300, []int{44, 1}}, {600, []int{88, 2}}, + {65536, []int{256, 256, 1}}, + {65537, []int{1, 0, 1}}, + {66000, []int{208, 1, 1}}, } { t.Run(fmt.Sprintf("size_%d", test.size), func(t *testing.T) { h := make([]byte, 32) @@ -864,14 +849,38 @@ func TestMirrorWriter_UpdateCheckpointGeometry(t *testing.T) { if l := len(eb.Entries); l != test.wantRHSizes[0] { t.Fatalf("expected %d entries in partial bundle %d.%d, got %d", test.wantRHSizes[0], idx, p, l) } - if got, want := eb.Entries, bundles[idx].Entries[:test.wantRHSizes[0]]; !slices.EqualFunc(got, want, bytes.Equal) { - t.Errorf("entrybundle doesn't match:\ngot %v\nwant %v", got, want) - } - // Verify the existence/correctness of the partial tiles for the given size. - for level, p := range test.wantRHSizes { - tile := mustReadTile(t, lr, uint64(level), idx, uint8(p)) - if len(tile.Nodes) != p { - t.Errorf("expected %d nodes in partial tile %d/%d.%d, got %d", p, level, idx, p, len(tile.Nodes)) + + // Verify the existence/correctness of the partial tiles for the given size, + // and ensure that full tiles whose range extends beyond the tree are not created. + for level, wantNodes := range test.wantRHSizes { + fullTilePath := layout.TilePath(uint64(level), idx, 0) + treeP := layout.PartialTileSize(uint64(level), idx, treeSize) + + if wantNodes == 0 { + // Tile does not exist at this checkpoint size. + // A full tile _may_ legitimately exist in the tree if it's large enough. + // However, since our treeSize is 66000 we know that one cannot be present for + // these test cases, so we'll assert that it doesn't exist in order to verify that we + // haven't incorrectly created one. + if fi, err := os.Stat(filepath.Join(s.cfg.Path, fullTilePath)); err == nil { + t.Fatalf("Level %d tile %d: full tile %s should not exist for size %d (found %d bytes)", level, idx, fullTilePath, test.size, fi.Size()) + } else if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("Failed to stat %s: %v", fullTilePath, err) + } + } else { + tile := mustReadTile(t, lr, uint64(level), idx, uint8(wantNodes)) + if len(tile.Nodes) != wantNodes { + t.Errorf("Expected %d nodes in partial tile %d/%d.%d, got %d", wantNodes, level, idx, uint8(wantNodes), len(tile.Nodes)) + } + // If the integrated tree does not have a full tile at this position (treeP > 0), + // no full tile file should have been written. + if treeP > 0 { + if fi, err := os.Stat(filepath.Join(s.cfg.Path, fullTilePath)); err == nil { + t.Errorf("Level %d tile %d: full tile %s should not exist for size %d (found %d bytes)", level, idx, fullTilePath, test.size, fi.Size()) + } else if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("Failed to stat %s: %v", fullTilePath, err) + } + } } idx >>= layout.TileHeight } @@ -928,3 +937,24 @@ func BenchmarkMarshalTlogEntryBundle(b *testing.B) { } } +func generateBundles(t *testing.T, totalEntries int) iter.Seq2[*api.EntryBundle, error] { + t.Helper() + + return func(yield func(*api.EntryBundle, error) bool) { + remaining := totalEntries + + N := 0 + for remaining > 0 { + count := min(remaining, layout.EntryBundleWidth) + entries := make([][]byte, count) + for i := range entries { + entries[i] = fmt.Appendf(nil, "entry %d", N+i) + } + if !yield(&api.EntryBundle{Entries: entries}, nil) { + return + } + remaining -= count + N += count + } + } +}