From 3b4a681e53895ee3754334e8bd5a4aca1888ff8c Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 11 Sep 2026 23:08:39 -0700 Subject: [PATCH] test(fuse_failover): dump chunk list and hex on append corruption (#11285) * test(fuse_failover): dump chunk list and hex on append corruption The failover append test (TestAppendWhileVolumeServerRestarts) failed in CI with an 8-byte NUL region at offset 632 that appeared in both the writer mount and the filer own view, but the failure message only showed a quoted-string window around the divergence. That is not enough to tell which chunk covered the zeroed bytes or which volume server held it, so the next recurrence would be just as unattributable. Add a FileChunkList helper that reads the filer resolved chunk list, and on failure dump: - every chunk fid, offset, size, volume id, and current master holders, flagging the chunk that covers the first divergence; - a hex+ASCII dump of the writer mount around the divergence so the exact zero-filled region is visible byte-for-byte. No production code is touched; this only makes the test fail louder. * test(fuse_failover): preserve diagnostic collection errors Address review feedback from CodeRabbit and Greptile on PR #11285: - FileChunkList now returns the wrapped ParseUint error when fid.volume_id is zero and the file_id prefix is invalid, matching FileVolumeIds instead of silently keeping vid=0 (which would query /dir/lookup?volumeId=0 and report the wrong holders). - dumpChunkList captures the VolumeHolders error and renders it as 'lookup failed: ...' so a failed master request is distinguishable from a successful lookup with no holders (both previously showed 'holders=[unknown]'). - runChaosAppend captures the writer-mount read error and includes it in the failure message so an unavailable writer view is not mistaken for corrupted content. --- test/fuse_failover/framework_test.go | 51 +++++++++++++ test/fuse_failover/volume_failover_test.go | 87 +++++++++++++++++++++- 2 files changed, 134 insertions(+), 4 deletions(-) diff --git a/test/fuse_failover/framework_test.go b/test/fuse_failover/framework_test.go index 5ab1338d8..e30c6b3f0 100644 --- a/test/fuse_failover/framework_test.go +++ b/test/fuse_failover/framework_test.go @@ -309,6 +309,57 @@ func (c *failoverCluster) FileVolumeIds(path string) ([]uint32, error) { return vids, nil } +// FileChunkList returns the filer's chunk list for a file: each chunk's file +// id, logical offset, size, and the volume id it lives on. Used to pinpoint +// which chunk covers a corrupted byte range and which volume server holds it. +func (c *failoverCluster) FileChunkList(path string) ([]struct { + FileId string + Offset int64 + Size uint64 + VolumeId uint32 +}, error) { + body, err := c.FilerGet(path + "?metadata=true&resolveManifest=true") + if err != nil { + return nil, err + } + var entry struct { + Chunks []struct { + FileId string `json:"file_id"` + Offset int64 `json:"offset"` + Size uint64 `json:"size"` + Fid struct { + VolumeId uint32 `json:"volume_id"` + } `json:"fid"` + } `json:"chunks"` + } + if err = json.Unmarshal(body, &entry); err != nil { + return nil, fmt.Errorf("decode entry %s: %w", path, err) + } + out := make([]struct { + FileId string + Offset int64 + Size uint64 + VolumeId uint32 + }, len(entry.Chunks)) + for i, ch := range entry.Chunks { + vid := ch.Fid.VolumeId + if vid == 0 && ch.FileId != "" { + parsed, parseErr := strconv.ParseUint(strings.SplitN(ch.FileId, ",", 2)[0], 10, 32) + if parseErr != nil { + return nil, fmt.Errorf("parse file id %s: %w", ch.FileId, parseErr) + } + vid = uint32(parsed) + } + out[i] = struct { + FileId string + Offset int64 + Size uint64 + VolumeId uint32 + }{ch.FileId, ch.Offset, ch.Size, vid} + } + return out, nil +} + // VolumeHolders returns the volume server addresses the master currently lists // for a volume id. func (c *failoverCluster) VolumeHolders(vid uint32) ([]string, error) { diff --git a/test/fuse_failover/volume_failover_test.go b/test/fuse_failover/volume_failover_test.go index 20f64bf87..04aa901bb 100644 --- a/test/fuse_failover/volume_failover_test.go +++ b/test/fuse_failover/volume_failover_test.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "testing" "time" @@ -396,13 +397,91 @@ func runChaosAppend(t *testing.T, c *failoverCluster, name string, chaos func()) // own mount and the filer make of the same file, which says whether the // data was lost on the way in or is only invisible from this side. d := firstDiff(want, got) - fromWriter, _ := os.ReadFile(writePath) + fromWriter, writerReadErr := os.ReadFile(writePath) viaFiler, filerErr := c.FilerGet("/" + name) require.Failf(t, "final content mismatch", - "%s: first difference at offset %d (want %d bytes, got %d)\nwant %q\ngot %q\nmount0 matches=%v filer matches=%v (err %v)\n%s", + "%s: first difference at offset %d (want %d bytes, got %d)\nwant %q\ngot %q\nmount0 matches=%v (writer read err %v) filer matches=%v (err %v)\n%s\n%s\n%s", name, d, len(want), len(got), window(want, d), window(got, d), - string(fromWriter) == want, string(viaFiler) == want, filerErr, - c.tailLog("mount0")) + string(fromWriter) == want, writerReadErr, string(viaFiler) == want, filerErr, + c.tailLog("mount0"), + dumpChunkList(c, "/"+name, d), + dumpHexAround(fromWriter, d, "writer mount"), + ) +} + +// dumpChunkList returns a human-readable summary of the filer's chunk list +// for path, annotated with the volume id and the master's current holders +// for each. When diffAt >= 0, the chunk covering that logical offset is +// flagged so a corruption can be tied to a specific chunk and volume server. +func dumpChunkList(c *failoverCluster, path string, diffAt int) string { + chunks, err := c.FileChunkList(path) + if err != nil { + return fmt.Sprintf("chunk list for %s: %v", path, err) + } + var b strings.Builder + fmt.Fprintf(&b, "chunk list for %s (%d chunks):", path, len(chunks)) + for i, ch := range chunks { + holders, holdersErr := c.VolumeHolders(ch.VolumeId) + holdersStr := "unknown" + if holdersErr != nil { + holdersStr = fmt.Sprintf("lookup failed: %v", holdersErr) + } else if len(holders) > 0 { + holdersStr = strings.Join(holders, ",") + } + marker := "" + if diffAt >= 0 && int64(diffAt) >= ch.Offset && int64(diffAt) < ch.Offset+int64(ch.Size) { + marker = " <-- covers diff" + } + fmt.Fprintf(&b, "\n [%d] fid=%s offset=%d size=%d vid=%d holders=[%s]%s", + i, ch.FileId, ch.Offset, ch.Size, ch.VolumeId, holdersStr, marker) + } + return b.String() +} + +// dumpHexAround returns a hex+ASCII dump of a 32-byte window around off in +// label's copy of the file, so the exact zero-filled region is visible in +// the failure message instead of only a quoted string window. +func dumpHexAround(data []byte, off int, label string) string { + if off < 0 { + return fmt.Sprintf("%s hex dump: no divergence offset", label) + } + start := max(0, off-16) + end := min(len(data), off+16) + if start >= end { + return fmt.Sprintf("%s hex dump: offset %d out of range (len %d)", label, off, len(data)) + } + var b strings.Builder + fmt.Fprintf(&b, "%s hex dump around offset %d:", label, off) + for i := start; i < end; i += 16 { + lineEnd := min(i+16, end) + hexPart := hexDump(data[i:lineEnd]) + asciiPart := asciiDump(data[i:lineEnd]) + fmt.Fprintf(&b, "\n %06x %-48s %s", i, hexPart, asciiPart) + } + return b.String() +} + +func hexDump(b []byte) string { + var sb strings.Builder + for i, x := range b { + if i > 0 { + sb.WriteByte(' ') + } + fmt.Fprintf(&sb, "%02x", x) + } + return sb.String() +} + +func asciiDump(b []byte) string { + var sb strings.Builder + for _, x := range b { + if x >= 0x20 && x < 0x7f { + sb.WriteByte(x) + } else { + sb.WriteByte('.') + } + } + return sb.String() } // firstDiff returns the offset of the first differing byte, or -1 when equal.