[POSIX] Fix incorrect mirror partial tile reconstruction

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)

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
+		}
+	}
+}