feat(filer.sync.verify): reclassify chunk-slice-order ETag diffs as CHUNK_REORDER (#10177)

* feat(filer.sync.verify): reclassify chunk-slice-order ETag diffs as CHUNK_REORDER filer.ETagChunks concatenates per-chunk MD5s in stored slice order without normalising by offset, so byte-identical content written by two different paths (S3 multipart part-completion order on the source vs filer.backup replication arrival order on the destination) yields different file ETags. filer.sync.verify reported these as ETAG_MISMATCH even though the files are equal. Add a second-pass check on every ETAG_MISMATCH: when both sides derive their ETag from chunks (no attr.Md5) and hold a manifest-free, non-overlapping chunk set that, once sorted by offset, matches element-wise on (offset, size, ETag), classify the file as CHUNK_REORDER. Such files are content-equal, so they are not counted as errors and do not affect the exit code; they are listed only at higher verbosity (weed -v=1), while the summary always shows their count. The check stays conservative: a stored attr.Md5 (order-independent content hash), a differing chunk count, an overlapping/duplicate offset (whose visible bytes are resolved by timestamp), or a manifest chunk all remain ETAG_MISMATCH. * filer.sync.verify: decline chunk-reorder fast path on empty per-chunk ETag An empty or undecodable per-chunk ETag is not a content fingerprint, so element-wise (offset, size, ETag) equality can't prove the bytes match. Treating "" == "" as content-equal could reclassify a genuine divergence as CHUNK_REORDER and drop it from the error count. Decline such chunk sets so they stay ETAG_MISMATCH. * filer.sync.verify: emit CHUNK_REORDER in JSON output regardless of -v The -v=1 gate belongs to the human text report only. Applying it before the jsonOutput branch dropped the per-file CHUNK_REORDER records from NDJSON while the summary still counted them, so a machine consumer saw a non-zero count with no records to reconcile it. Gate the text path only; JSON always emits. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>
2026-10-13 17:48:29 +02:00 · 2026-07-01 10:38:21 -07:00
1 parent e6441d84e0
commit ebeab4b6ec
@@ -4,11 +4,14 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"os"
+	"strings"
 	"sync/atomic"
 	"testing"
 	"time"

 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
+	"github.com/seaweedfs/seaweedfs/weed/util"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
 )
@@ -101,6 +104,34 @@ func verifyDirEntry(name string) *filer_pb.Entry {
 	return &filer_pb.Entry{Name: name, IsDirectory: true}
 }

+// verifyChunk builds a FileChunk whose ETag is the base64 MD5 of seed, so
+// filer.ETagChunks (which base64-decodes each chunk ETag) can hash it. Distinct
+// seeds yield distinct per-chunk MD5s.
+func verifyChunk(offset int64, size uint64, seed string) *filer_pb.FileChunk {
+	return &filer_pb.FileChunk{
+		Offset: offset,
+		Size:   size,
+		ETag:   util.Base64Encode(util.Md5([]byte(seed))),
+	}
+}
+
+// verifyChunkedEntry builds a chunk-backed entry with no stored attr.Md5, so
+// its file ETag is derived from ETagChunks (the order-sensitive path). FileSize
+// is the max chunk end, matching filer.FileSize, so size comparison passes.
+func verifyChunkedEntry(name string, chunks []*filer_pb.FileChunk) *filer_pb.Entry {
+	var total uint64
+	for _, c := range chunks {
+		if end := uint64(c.Offset) + c.Size; end > total {
+			total = end
+		}
+	}
+	return &filer_pb.Entry{
+		Name:       name,
+		Attributes: &filer_pb.FuseAttributes{FileSize: total}, // Md5 nil → ETagChunks path
+		Chunks:     chunks,
+	}
+}
+
 // --- tests ---

 // TestVerifySyncMissingFile confirms that a file present in A but absent in B
@@ -280,6 +311,271 @@ func TestVerifySyncETagMismatch(t *testing.T) {
 	}
 }

+// TestVerifySyncChunkReorder confirms that two entries holding the SAME
+// chunks (same offsets, same per-chunk MD5s) in a DIFFERENT slice order are
+// classified as CHUNK_REORDER (content-equal), not ETAG_MISMATCH, and are
+// therefore not counted as errors. This is the S3-multipart vs filer.backup
+// reordering false positive.
+func TestVerifySyncChunkReorder(t *testing.T) {
+	c0 := verifyChunk(0, 100, "chunk-0")
+	c1 := verifyChunk(100, 100, "chunk-1")
+	c2 := verifyChunk(200, 100, "chunk-2")
+
+	// A stores [c0,c1,c2]; B stores a permutation [c2,c0,c1] of the same chunks.
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{c0, c1, c2})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{c2, c0, c1})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.chunkReorder.Load(); got != 1 {
+		t.Errorf("chunkReorder = %d, want 1", got)
+	}
+	if got := result.etagMismatch.Load(); got != 0 {
+		t.Errorf("etagMismatch = %d, want 0 (reordered chunks are content-equal)", got)
+	}
+	if got := result.sizeMismatch.Load(); got != 0 {
+		t.Errorf("sizeMismatch = %d, want 0", got)
+	}
+}
+
+// TestVerifySyncGenuineChunkDivergence confirms that when one chunk's content
+// actually differs (same offsets and count, different per-chunk MD5), the file
+// stays classified as ETAG_MISMATCH and is NOT downgraded to CHUNK_REORDER.
+func TestVerifySyncGenuineChunkDivergence(t *testing.T) {
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{
+				verifyChunk(0, 100, "chunk-0"),
+				verifyChunk(100, 100, "chunk-1"),
+				verifyChunk(200, 100, "chunk-2"),
+			})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{
+				verifyChunk(0, 100, "chunk-0"),
+				verifyChunk(100, 100, "chunk-1"),
+				verifyChunk(200, 100, "chunk-2-DIFFERENT"), // real content divergence
+			})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.etagMismatch.Load(); got != 1 {
+		t.Errorf("etagMismatch = %d, want 1 (genuine chunk content divergence)", got)
+	}
+	if got := result.chunkReorder.Load(); got != 0 {
+		t.Errorf("chunkReorder = %d, want 0 (content actually differs)", got)
+	}
+}
+
+// TestVerifySyncChunkCountDiffStaysEtagMismatch confirms that a differing chunk
+// count with an equal file size (e.g. differently split content) is NOT treated
+// as a reordering — it stays ETAG_MISMATCH.
+func TestVerifySyncChunkCountDiffStaysEtagMismatch(t *testing.T) {
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{
+				verifyChunk(0, 100, "a"),
+				verifyChunk(100, 100, "b"),
+				verifyChunk(200, 100, "c"),
+			})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{
+				verifyChunk(0, 150, "x"),
+				verifyChunk(150, 150, "y"),
+			})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.sizeMismatch.Load(); got != 0 {
+		t.Errorf("sizeMismatch = %d, want 0 (both total 300 bytes)", got)
+	}
+	if got := result.etagMismatch.Load(); got != 1 {
+		t.Errorf("etagMismatch = %d, want 1", got)
+	}
+	if got := result.chunkReorder.Load(); got != 0 {
+		t.Errorf("chunkReorder = %d, want 0", got)
+	}
+}
+
+// TestVerifySyncDuplicateOffsetStaysEtagMismatch confirms that overlapping
+// chunks at the same offset — where the visible bytes are resolved by chunk
+// timestamp, not by the raw chunk list — are NOT fast-pathed to CHUNK_REORDER.
+// Both sides hold the same two ETags at offset 0 in a different order, so the
+// file ETags differ; the visible content is ambiguous from the list alone, so
+// this must stay ETAG_MISMATCH.
+func TestVerifySyncDuplicateOffsetStaysEtagMismatch(t *testing.T) {
+	c1 := verifyChunk(0, 100, "v1")
+	c2 := verifyChunk(0, 100, "v2") // same offset → overlap
+
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{c1, c2})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{c2, c1})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.etagMismatch.Load(); got != 1 {
+		t.Errorf("etagMismatch = %d, want 1 (overlapping offsets are not a safe reorder)", got)
+	}
+	if got := result.chunkReorder.Load(); got != 0 {
+		t.Errorf("chunkReorder = %d, want 0", got)
+	}
+}
+
+// TestVerifySyncManifestChunkStaysEtagMismatch confirms that entries containing
+// a manifest chunk are not fast-pathed to CHUNK_REORDER: manifest chunks
+// represent compacted, possibly overlapping history that this check does not
+// resolve.
+func TestVerifySyncManifestChunkStaysEtagMismatch(t *testing.T) {
+	manifest := func(offset int64, size uint64, seed string) *filer_pb.FileChunk {
+		c := verifyChunk(offset, size, seed)
+		c.IsChunkManifest = true
+		return c
+	}
+	a0, a1 := manifest(0, 100, "m0"), verifyChunk(100, 100, "m1")
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{a0, a1})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{a1, a0})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.etagMismatch.Load(); got != 1 {
+		t.Errorf("etagMismatch = %d, want 1 (manifest chunks are not fast-pathed)", got)
+	}
+	if got := result.chunkReorder.Load(); got != 0 {
+		t.Errorf("chunkReorder = %d, want 0", got)
+	}
+}
+
+// TestVerifySyncEmptyChunkETagStaysEtagMismatch confirms that a chunk with an
+// empty per-chunk ETag is never fast-pathed to CHUNK_REORDER: an empty ETag is
+// not a content fingerprint, so (offset, size, ETag) equality cannot prove the
+// bytes match. The two sides reorder their non-empty chunks so the file ETags
+// differ and the reorder check is reached, but the empty-ETag chunk in the
+// middle must keep it ETAG_MISMATCH rather than assert a false content-equality.
+func TestVerifySyncEmptyChunkETagStaysEtagMismatch(t *testing.T) {
+	c0 := verifyChunk(0, 100, "e0")
+	empty := &filer_pb.FileChunk{Offset: 100, Size: 100} // no ETag → not a fingerprint
+	c2 := verifyChunk(200, 100, "e2")
+
+	clientA := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{c0, empty, c2})},
+		},
+	}
+	clientB := &verifyTestFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/root": {verifyChunkedEntry("data.bin", []*filer_pb.FileChunk{c2, c0, empty})},
+		},
+	}
+
+	result := &VerifyResult{}
+	sem := make(chan struct{}, verifySyncConcurrency)
+	if err := compareDirectory(context.Background(), clientA, clientB,
+		"/root", "/root", false, time.Time{}, sem, result); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got := result.etagMismatch.Load(); got != 1 {
+		t.Errorf("etagMismatch = %d, want 1 (empty per-chunk ETag is not a content fingerprint)", got)
+	}
+	if got := result.chunkReorder.Load(); got != 0 {
+		t.Errorf("chunkReorder = %d, want 0", got)
+	}
+}
+
+// captureStdout runs fn with os.Stdout redirected to a pipe and returns what it
+// wrote. Tests that assert on emitted diff records use it.
+func captureStdout(t *testing.T, fn func()) string {
+	t.Helper()
+	orig := os.Stdout
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("os.Pipe: %v", err)
+	}
+	os.Stdout = w
+	fn()
+	w.Close()
+	os.Stdout = orig
+	data, err := io.ReadAll(r)
+	if err != nil {
+		t.Fatalf("read captured stdout: %v", err)
+	}
+	return string(data)
+}
+
+// TestVerifySyncChunkReorderJSONEmittedByDefault confirms that in JSON output
+// mode a CHUNK_REORDER record is emitted at default verbosity: the -v=1 gate
+// suppresses only the human text report, so the NDJSON per-record stream stays
+// consistent with the summary count a machine consumer reads.
+func TestVerifySyncChunkReorderJSONEmittedByDefault(t *testing.T) {
+	c0 := verifyChunk(0, 100, "chunk-0")
+	c1 := verifyChunk(100, 100, "chunk-1")
+	entryA := verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{c0, c1})
+	entryB := verifyChunkedEntry("model.bin", []*filer_pb.FileChunk{c1, c0})
+
+	result := &VerifyResult{jsonOutput: true}
+	out := captureStdout(t, func() {
+		reportDiff(diffChunkReorder, "/root", entryA, entryB, result)
+	})
+
+	if got := result.chunkReorder.Load(); got != 1 {
+		t.Fatalf("chunkReorder = %d, want 1", got)
+	}
+	if !strings.Contains(out, `"type":"CHUNK_REORDER"`) {
+		t.Errorf("JSON output missing CHUNK_REORDER record at default verbosity; got %q", out)
+	}
+}
+
 // TestVerifySyncCutoffTime verifies that entries newer than cutoffTime are
 // skipped in both the A-only (MISSING) and B-only (ONLY_IN_B) branches.
 func TestVerifySyncCutoffTime(t *testing.T) {