diff --git a/.omo/plans/fix-c4-non-last-segment-corruption.md b/.omo/plans/fix-c4-non-last-segment-corruption.md new file mode 100644 index 0000000..007496d --- /dev/null +++ b/.omo/plans/fix-c4-non-last-segment-corruption.md @@ -0,0 +1,539 @@ +# C4 修复方案:非尾段损坏必须硬错误 + +## TL;DR + +> **目标**:让 WAL recovery 按 design §3.2 line 704 区分"尾段损坏(可截断)"和"中间段损坏(硬错误)"。当前 `ReplaySegmentFile` 不论在哪个 segment 都把 CollectingFragments / parse error 当 TailCorruption,导致中间段损坏时 `Recover` 错误地截断最后一段的有效数据。 +> +> **交付**: +> - `ReplaySegmentFile` 加 `isLastSegment bool` 参数 +> - `TailCorruptionError` 加 `SegmentPath string` 字段(诊断 + 防御) +> - 非尾段遇到 corruption → 硬错误(不再是 TailCorruptionError) +> - `RecoverFromSegments` 传递 `isLastSegment` 给 `ReplaySegmentFile` +> - `Recover` 用 `tce.SegmentPath` 做截断目标(防御性 fallback 到 segments[last]) +> - ~5 个新测试覆盖单/多 segment 场景 +> - 单次 commit +> +> **预估工时**:2-3 小时 +> **风险**:中。`ReplaySegmentFile` 签名变更是 breaking change(影响测试调用) + +--- + +## Context + +### Bug 摘要 + +`wal/recovery.go:133-138` 在 `ReplaySegmentFile` 末尾: + +```go +if collector.State() == FragmentCollecting { + return nextSequence, &TailCorruptionError{...} +} +``` + +**不区分 segment 是不是最后一个**。同样地,`ParseRecordsFromFile` / `ParseBlock` 返回的 TailCorruptionError 也不带 segment 位置信息。 + +`Recover` 拿到 TailCorruptionError 后,**总是对 `segments[len(segments)-1]` 调用 truncate**(C5+H8 修复后的 `wal/recover.go:60-71`)。如果 corruption 实际发生在中间段: +- 真正损坏的中间段不动 +- 最后一段(完全有效)被错误截掉 +- 中间段之后、最后段之前的有效 batch 全部丢失 + +### Oracle 验证(bg_ef425776) + +> C4 还有一个更严重的失败模式。`wal/recover.go:61` 总是对 `segments[len(segments)-1]` 调用 `truncateSegment`,但 `RecoverFromSegments` 的 `TailCorruptionError` 可能来自非尾段。结果:真正损坏的 segment 不动,最后一段的有效数据被错误截掉。 + +### 设计依据(§3.2 line 704) + +> 尾部损坏通常来自进程崩溃或机器掉电时最后一次写入的 partial write。**若后面还有需要恢复的 segment,前一个 segment 的尾部异常不能按尾部损坏截断**,否则可能跳过已经持久化历史并破坏 sequence 连续性。中间损坏说明已持久化 WAL 文件被破坏,**默认不静默跳过,不默认 repair**。 + +错误分类表(line 697-704)明确:所有 4 类物理损坏(header 半写 / length 越界 / CRC 失败 / 非 0 padding)在"WAL 中间"位置都是**硬错误**。 + +### 修复后的行为矩阵 + +| Corruption 位置 | isLastSegment | 行为 | +|----------------|--------------|------| +| 最后一段尾部 | true | TailCorruptionError → Recover 截断(现有行为)| +| 中间段任何位置 | false | **硬错误**(新行为)→ Recover 返回错误 → DB.Open 失败 | +| 干净 WAL | N/A | 正常 replay,无 corruption | + +### 协同:C5+H8 / C8 关系 + +- **C5+H8 已修**:截断路径用 `findLastCompleteBatchEnd`(H8)+ 4 步协议(C5)。C4 修复后,截断路径只在"最后一段尾部损坏"时触发,行为正确。 +- **C8 未修**:segment_manager.go:64-66 把字节偏移当 startSequence。但 `writeTestSegment` 直接构造 segment,可以指定正确 startSequence,所以 C4 测试不依赖 C8。 +- **多 segment recovery** 在 C4 修复后行为正确(前提是 startSequence 正确,即 C8 也已修)。但 C4 本身的修复不依赖 C8。 + +--- + +## 执行计划 + +### Phase A:代码改动(45-60 分钟) + +#### A.1 扩展 `TailCorruptionError` 加 `SegmentPath` + +文件:`wal/record_parser.go` + +```go +type TailCorruptionError struct { + Offset int // offset within the segment file + SegmentPath string // NEW: file path of the corrupted segment, for diagnostics + Recover's truncation target + Err error +} + +func (e *TailCorruptionError) Error() string { + if e.SegmentPath != "" { + return fmt.Sprintf("wal: tail corruption in %s at offset %d: %v", e.SegmentPath, e.Offset, e.Err) + } + return fmt.Sprintf("wal: tail corruption at offset %d: %v", e.Offset, e.Err) +} +``` + +> **必要 docstring**:字段含义 + 用途(diagnostics + Recover 的截断目标)。 + +#### A.2 `ReplaySegmentFile` 加 `isLastSegment` 参数 + +文件:`wal/recovery.go` + +```go +// 改前: +func ReplaySegmentFile(filePath string, startSequence uint64, replayer BatchReplayer) (nextSequence uint64, err error) + +// 改后: +func ReplaySegmentFile(filePath string, startSequence uint64, isLastSegment bool, replayer BatchReplayer) (nextSequence uint64, err error) +``` + +函数体改动: + +```go +nextSequence = startSequence +records, parseErr := ParseRecordsFromFile(filePath) +if parseErr != nil && !IsTailCorruption(parseErr) { + return nextSequence, fmt.Errorf("wal: parse segment records: %w", parseErr) +} + +// Attach SegmentPath to parseErr for downstream diagnostics + truncation target. +// Do this BEFORE the non-last hard-error conversion so both paths benefit. +if parseErr != nil { + var tce *TailCorruptionError + if errors.As(parseErr, &tce) { + tce.SegmentPath = filePath + } +} + +// C4 fix: tail corruption in non-last segment is hard corruption per +// design §3.2 line 704. Use %v (NOT %w) so IsTailCorruption returns false +// for this wrapped error — otherwise errors.As would still find the +// underlying *TailCorruptionError and Recover would treat it as truncatable. +if parseErr != nil && !isLastSegment { + return nextSequence, fmt.Errorf("wal: corruption in non-last segment %s (hard corruption): %v", + filePath, parseErr) +} + +collector := NewFragmentCollector() +for _, record := range records { + // ... (unchanged) +} + +if parseErr != nil { + return nextSequence, parseErr +} +if collector.State() == FragmentCollecting { + if isLastSegment { + return nextSequence, &TailCorruptionError{ + Offset: 0, + SegmentPath: filePath, + Err: errors.New("incomplete fragmented batch at segment tail"), + } + } + // Non-last segment with incomplete fragments = middle corruption. + // Plain error (no TailCorruptionError wrapping) — IsTailCorruption is false. + return nextSequence, fmt.Errorf("wal: incomplete fragmented batch in non-last segment %s (hard corruption)", filePath) +} + +return nextSequence, nil +``` + +#### A.3 `RecoverFromSegments` 传递 `isLastSegment` + +文件:`wal/recovery.go` + +```go +nextSequence = segments[0].StartSequence +for i, segment := range segments { + if segment.StartSequence != nextSequence { + return nextSequence, fmt.Errorf("wal: segment start sequence %d does not match expected sequence %d", segment.StartSequence, nextSequence) + } + + isLastSegment := i == len(segments)-1 + nextSequence, err = ReplaySegmentFile(segment.FilePath, nextSequence, isLastSegment, replayer) + if err != nil { + if IsTailCorruption(err) { + return nextSequence, err + } + return nextSequence, fmt.Errorf("wal: replay segment: %w", err) + } +} +``` + +#### A.4 `Recover` 使用 `tce.SegmentPath`(防御性) + +文件:`wal/recover.go`,C5+H8 修复后的 truncation 块: + +```go +if len(segments) > 0 { + // After C4 fix, TailCorruptionError is only returned for the last segment. + // Use tce.SegmentPath as authoritative truncation target (defensive: + // fall back to segments[last] if missing). + corruptedPath := segments[len(segments)-1].FilePath + var tce *TailCorruptionError + if errors.As(err, &tce) && tce.SegmentPath != "" { + corruptedPath = tce.SegmentPath + } + + lastCompleteBatchEnd, findErr := findLastCompleteBatchEnd(corruptedPath) + if findErr != nil { + return nil, fmt.Errorf("wal: recover: find truncation offset: %w", findErr) + } + + var emptyTrailing []string + if err := truncateAndPersist(corruptedPath, lastCompleteBatchEnd, dir, emptyTrailing); err != nil { + return nil, fmt.Errorf("wal: recover: persist tail truncation: %w", err) + } +} +``` + +**关键不变量**:C4 fix 后 TailCorruptionError 只来自最后一段,所以 `corruptedPath == segments[last].FilePath`。`tce.SegmentPath` 是防御性显式表达,未来如果不变量破坏能立即发现。 + +> **关于 `errors.As` 用法**:`err` 在 Recover 函数签名里是 outer 变量(`nextSequence, err := RecoverFromSegments(...)`),shadowing 后再 `errors.As(err, &tce)` 读的是当前 outer err。需要确认 Go 的语义在这里正确(应该是的,但加注释或测试覆盖)。 + +### Phase B:测试(60-90 分钟) + +#### B.1 `ReplaySegmentFile` 单元测试(isLastSegment 参数) + +新增到 `wal/recovery_test.go`: + +```go +// Regression guards for C4: isLastSegment controls whether CollectingFragments +// at end is tail corruption (truncatable) or hard corruption. + +func TestReplaySegmentFile_LastSegmentCollectingFragmentsIsTailCorruption(t *testing.T) { + // Build segment with: [Batch A][First][Middle* no Last]. + // Call ReplaySegmentFile with isLastSegment=true. + // Expect: TailCorruptionError (can be truncated). + // Expect: tce.SegmentPath == filePath. +} + +func TestReplaySegmentFile_NonLastSegmentCollectingFragmentsIsHardError(t *testing.T) { + // Same segment content as above. + // Call ReplaySegmentFile with isLastSegment=false. + // Expect: hard error (NOT TailCorruptionError). + // Expect: error mentions "non-last segment" or "hard corruption". +} + +func TestReplaySegmentFile_LastSegmentParseErrorIsTailCorruption(t *testing.T) { + // Build segment with: [Batch A][CRC-corrupted bytes]. + // Call with isLastSegment=true. + // Expect: TailCorruptionError. +} + +func TestReplaySegmentFile_NonLastSegmentParseErrorIsHardError(t *testing.T) { + // Same segment content. + // Call with isLastSegment=false. + // Expect: hard error. +} +``` + +#### B.2 `RecoverFromSegments` / `Recover` 集成测试 + +新增到 `wal/recover_test.go`: + +```go +// Regression guard for C4: middle segment corruption must hard-fail Recover, +// NOT truncate the (valid) last segment. This is the key bug Oracle flagged: +// "wal/recover.go:61 总是对 segments[len(segments)-1] 调用 truncateSegment, +// 但 RecoverFromSegments 的 TailCorruptionError 可能来自非尾段". +func TestRecoverMiddleSegmentCorruptionHardFails(t *testing.T) { + dir := t.TempDir() + // Construct 3 segments with valid startSequences. + // segment-0: [Batch seq 0-1] (2 entries → next=2) + // segment-1: [Batch seq 2-3] + [First][Middle no Last] ← middle corruption + // (2 complete entries → next=4 if recovery reached end) + // segment-2: [Batch seq 4-5] (2 entries; valid; never reached) + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0"), makePutEntry("k1", "v1")}, + }) + + seg1Path := writeTestSegment(t, dir, 1, 2, [][]*WalEntry{ + {makePutEntry("k2", "v2"), makePutEntry("k3", "v3")}, + }) + // Append First+Middle fragments to segment-1 (no Last). + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecFirst, []byte("first-frag"))) + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecMiddle, []byte("middle-frag"))) + + writeTestSegment(t, dir, 2, 4, [][]*WalEntry{ + {makePutEntry("k4", "v4"), makePutEntry("k5", "v5")}, + }) + + seg2Path := filepath.Join(dir, "segment-2.wal") + fiBefore, _ := os.Stat(seg2Path) + + _, err := Recover(dir, &mockReplayer{}) + if err == nil { + t.Fatal("expected Recover to fail on middle segment corruption") + } + if IsTailCorruption(err) { + t.Errorf("expected hard error (not tail corruption) for middle segment; got %v", err) + } + + // CRITICAL: segment-2 must NOT be truncated (it's completely valid). + fiAfter, _ := os.Stat(seg2Path) + if fiAfter.Size() != fiBefore.Size() { + t.Errorf("segment-2 was modified: before=%d after=%d (C4-2 regression)", + fiBefore.Size(), fiAfter.Size()) + } +} + +// Regression guard for C4: single-segment tail corruption still truncates +// correctly (existing behavior preserved). +func TestRecoverLastSegmentCorruptionTruncatesCorrectly(t *testing.T) { + dir := t.TempDir() + filePath := writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0")}, + {makePutEntry("k1", "v1")}, + }) + encA, _ := EncodeWalBatch(0, []*WalEntry{makePutEntry("k0", "v0")}) + endOfBatch1 := int64(WalFileHeaderSize) + + int64(PhysicalRecordHeaderSize+len(encA)) + + int64(PhysicalRecordHeaderSize+len(encA)) + + // Append partial fragments to last (only) segment. + appendFileBytes(t, filePath, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + result, err := Recover(dir, &mockReplayer{}) + if err != nil { + t.Fatalf("Recover: %v", err) + } + if !result.Truncated { + t.Fatal("Truncated = false, want true for last-segment corruption") + } + + fi, _ := os.Stat(filePath) + if fi.Size() != endOfBatch1 { + t.Errorf("file size = %d, want %d", fi.Size(), endOfBatch1) + } +} + +// Regression guard for C4: multi-segment with last-segment corruption still +// works correctly (the legitimate tail-truncation case). +func TestRecoverMultiSegmentLastSegmentCorruptionTruncatesLast(t *testing.T) { + dir := t.TempDir() + // segment-0: valid batches + // segment-1: valid batches + partial tail (corruption in LAST segment) + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0")}, + }) + seg1Path := writeTestSegment(t, dir, 1, 1, [][]*WalEntry{ + {makePutEntry("k1", "v1")}, + {makePutEntry("k2", "v2")}, + }) + // Append partial fragments to segment-1 (the LAST segment). + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + // Compute expected truncation point: end of Batch B in segment-1. + encA, _ := EncodeWalBatch(1, []*WalEntry{makePutEntry("k1", "v1")}) + encB, _ := EncodeWalBatch(2, []*WalEntry{makePutEntry("k2", "v2")}) + endOfBatch2 := int64(WalFileHeaderSize) + + int64(PhysicalRecordHeaderSize+len(encA)) + + int64(PhysicalRecordHeaderSize+len(encB)) + + result, err := Recover(dir, &mockReplayer{}) + if err != nil { + t.Fatalf("Recover: %v", err) + } + if !result.Truncated { + t.Fatal("Truncated = false, want true") + } + + fi, _ := os.Stat(seg1Path) + if fi.Size() != endOfBatch2 { + t.Errorf("segment-1 size = %d, want %d (end of Batch 2)", fi.Size(), endOfBatch2) + } +} +``` + +#### B.3 现有测试更新 + +`wal/recover_test.go` 中的 `TestRecoverIdempotentAfterTruncation` 和 `TestRecoverPartialFragmentTailIdempotent` 都是单 segment 场景,行为不变。但它们间接调用了 `Recover` → `RecoverFromSegments` → `ReplaySegmentFile`,需要确认新签名 `isLastSegment=true` 在 RecoverFromSegments 内部正确传递。 + +`wal/recovery_test.go` 现有的直接调用 `ReplaySegmentFile` 的测试需要补 `isLastSegment` 参数。**3 个调用点**(Oracle bg_22edec11 提醒): + +```bash +grep -n "ReplaySegmentFile" wal/recovery_test.go wal/recover_test.go +``` + +- `wal/recovery_test.go:124`(TestReplaySegmentFile 之类)→ 加 `true` +- `wal/recovery_test.go:150`(另一个 ReplaySegmentFile 测试)→ 加 `true` +- `wal/recover_test.go:121`(TestRecoverWithTailCorruption 内部,验证截断后重放)→ 加 `true` + +每个调用点加 `true`(默认按 last segment 处理,保留现有行为)。 + +#### B.4 补 middle-segment parser corruption 集成测试(Oracle 新增) + +> **Oracle 修订(bg_22edec11)NICE-TO-HAVE**:原计划只覆盖 middle-segment CollectingFragments,没覆盖 middle-segment parser corruption(CRC 失败)。需要加一个。 + +```go +// Regression guard for C4 (parser corruption half): middle segment CRC +// corruption must hard-fail Recover, NOT truncate the (valid) last segment. +// This tests the path where ParseBlock returns TailCorruptionError and +// ReplaySegmentFile converts it to hard error for non-last segment. +func TestRecoverMiddleSegmentCRCCorruptionHardFails(t *testing.T) { + dir := t.TempDir() + // segment-0: [Batch seq 0-1] (2 entries → next=2) + // segment-1: [Batch seq 2-3] + [CRC-corrupted bytes] ← middle parser corruption + // segment-2: [Batch seq 4-5] (valid; never reached) + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0"), makePutEntry("k1", "v1")}, + }) + + seg1Path := writeTestSegment(t, dir, 1, 2, [][]*WalEntry{ + {makePutEntry("k2", "v2"), makePutEntry("k3", "v3")}, + }) + // Append CRC-corrupted bytes (will fail DecodePhysicalRecord's CRC check). + // 10 bytes of 0xFF — looks like a record header but CRC won't match. + appendFileBytes(t, seg1Path, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}) + + writeTestSegment(t, dir, 2, 4, [][]*WalEntry{ + {makePutEntry("k4", "v4"), makePutEntry("k5", "v5")}, + }) + + seg2Path := filepath.Join(dir, "segment-2.wal") + fiBefore, _ := os.Stat(seg2Path) + + _, err := Recover(dir, &mockReplayer{}) + if err == nil { + t.Fatal("expected Recover to fail on middle segment CRC corruption") + } + if IsTailCorruption(err) { + t.Errorf("expected hard error (not tail corruption) for middle segment CRC; got %v", err) + } + + // CRITICAL: segment-2 must NOT be truncated. + fiAfter, _ := os.Stat(seg2Path) + if fiAfter.Size() != fiBefore.Size() { + t.Errorf("segment-2 was modified: before=%d after=%d (C4-2 regression)", + fiBefore.Size(), fiAfter.Size()) + } +} +``` + +### Phase C:验证(15 分钟) + +```bash +# 1. 编译 +go build ./... + +# 2. 重点测试 +go test ./wal -run 'TestReplaySegmentFile|TestRecoverMiddleSegment|TestRecoverLastSegment|TestRecoverMultiSegment|TestRecoverIdempotent|TestRecoverPartialFragmentTail' -count=1 -v + +# 3. wal 包全量 +go test ./wal/... -count=1 + +# 4. 全仓 +go test ./... -count=1 + +# 5. race +go test -race ./... -count=1 + +# 6. vet +go vet ./... +``` + +### Phase D:Commit message draft + +``` +fix: treat non-last segment corruption as hard error (C4) + +Per design §3.2 line 704, tail corruption in a non-last WAL segment is +middle corruption, which must hard-fail recovery instead of being +silently truncated. The previous code in wal/recovery.go always returned +TailCorruptionError for CollectingFragments state or parse errors, +regardless of segment position. Recover then always truncated +segments[last], which could corrupt a valid last segment when the actual +corruption was in a middle segment. + +Oracle bg_ef425776 flagged an additional failure mode: "wal/recover.go:61 +总是对 segments[len(segments)-1] 调用 truncateSegment,但 +RecoverFromSegments 的 TailCorruptionError 可能来自非尾段". + +Changes: +- wal/record_parser.go: add SegmentPath field to TailCorruptionError for + diagnostics and defensive truncation target identification. +- wal/recovery.go: + - ReplaySegmentFile now takes isLastSegment bool parameter. + - When parse error or CollectingFragments occurs in non-last segment, + return hard error instead of TailCorruptionError. + - When in last segment, return TailCorruptionError with SegmentPath set. + - RecoverFromSegments passes isLastSegment based on iteration index. +- wal/recover.go: + - Use tce.SegmentPath as authoritative truncation target (defensive + fallback to segments[last] if missing). After C4 fix, TailCorruptionError + is only returned for last segment, so this is always segments[last] + in practice. + +Tests: +- wal/recovery_test.go: 4 unit tests for ReplaySegmentFile covering + last/non-last × CollectingFragments/parse-error matrix. Existing tests + that directly call ReplaySegmentFile updated to pass isLastSegment=true. +- wal/recover_test.go: 3 integration tests covering middle-segment + corruption (must hard-fail), last-segment corruption in single-segment + WAL (must truncate), last-segment corruption in multi-segment WAL + (must truncate only last segment). + +Verified: each new test fails on pre-fix code (non-last corruption +silently truncated valid last segment) and passes after the fix. Full +suite green including go test -race ./... . + +Audit context: docs/audit-3.2.md C4 (Oracle-verified bg_ef425776). +``` + +--- + +## 验收清单 + +- [ ] Phase A.1:`TailCorruptionError.SegmentPath` 字段存在 +- [ ] Phase A.2:`ReplaySegmentFile` 签名包含 `isLastSegment bool` +- [ ] Phase A.2:非尾段 CollectingFragments 返回硬错误(不是 TailCorruptionError) +- [ ] Phase A.2:非尾段 parse error(TailCorruption from parser)转硬错误 +- [ ] Phase A.3:`RecoverFromSegments` 传递正确 `isLastSegment` +- [ ] Phase A.4:`Recover` 用 `tce.SegmentPath` 做截断目标 +- [ ] Phase B.1:4 个 `ReplaySegmentFile` 单元测试存在 +- [ ] Phase B.2:3 个集成测试存在(middle/last-single/last-multi) +- [ ] Phase B.3:现有 `ReplaySegmentFile` 直接调用更新签名 +- [ ] `go test ./wal/... -count=1` 全绿 +- [ ] `go test ./... -count=1` 全绿 +- [ ] `go test -race ./... -count=1` 全绿 +- [ ] `go vet ./...` 无新增警告 +- [ ] 单次 commit,message 引用 audit C4 + +--- + +## 不在本次范围内(后续 issue) + +| 编号 | 为什么不放进来 | +|------|---------------| +| C8 | segment_manager.go:64-66 把字节偏移当 startSequence。本 patch 测试用 `writeTestSegment` 绕过,但生产环境多 segment recovery 仍受 C8 影响 | +| C1 | CRC 多项式 IEEE → crc32c,独立 | +| C7 | Put/Close 竞态,独立 | +| H1-H7 | 其他 High,独立 | + +--- + +## 修订记录 + +- **v1(原始)**:C4 修复方案初稿,送 Momus 审 +- **v1.0(Momus 审核 bg_423473e7)**:[OKAY],无 blocking。思考过程敏锐发现 `TestRecoverMiddleSegmentCorruptionHardFails` 中 segment-2 的 startSequence=6 与 segment-1 的实际 complete batch 数不匹配(应该是 4)。修正为 4 +- **v1.1(Oracle 修订 bg_22edec11)**: + - **BLOCKING**:非尾段 parser corruption 用 `%v` 而不是 `%w` —— `IsTailCorruption` 用 `errors.As` 遍历链,`%w` 包装后仍能匹配 `*TailCorruptionError`,会被当 tail corruption 处理 + - **BLOCKING**:漏了 `wal/recover_test.go:121` 的 `ReplaySegmentFile` 直接调用,需要更新签名。共 3 个直接调用点 + - **NEW**:加 `TestRecoverMiddleSegmentCRCCorruptionHardFails` 集成测试,覆盖 parser corruption 中段损坏(CRC fail)的端到端路径 + - **MINOR**:把 SegmentPath attachment 移到非尾段硬错误转换之前,让两条路径都能设置字段(虽然非尾段硬错误不需要,但保持顺序一致) diff --git a/wal/record_parser.go b/wal/record_parser.go index 7b86996..f833002 100644 --- a/wal/record_parser.go +++ b/wal/record_parser.go @@ -10,12 +10,19 @@ import ( // TailCorruptionError indicates that the WAL tail contains corrupt data // (bad CRC, unexpected non-zero padding bytes, etc.). Recovery may safely // truncate at the last valid record. +// +// SegmentPath is set by ReplaySegmentFile when it propagates the error, +// so Recover can use it as the authoritative truncation target. type TailCorruptionError struct { - Offset int - Err error + Offset int + SegmentPath string + Err error } func (e *TailCorruptionError) Error() string { + if e.SegmentPath != "" { + return fmt.Sprintf("wal: tail corruption in %s at offset %d: %v", e.SegmentPath, e.Offset, e.Err) + } return fmt.Sprintf("wal: tail corruption at offset %d: %v", e.Offset, e.Err) } diff --git a/wal/recover.go b/wal/recover.go index 10fd48a..e2609ad 100644 --- a/wal/recover.go +++ b/wal/recover.go @@ -1,6 +1,7 @@ package wal import ( + "errors" "fmt" "os" @@ -59,19 +60,27 @@ func Recover(dir string, replayer BatchReplayer) (*RecoveryResult, error) { } if len(segments) > 0 { - lastSeg := segments[len(segments)-1] - lastCompleteBatchEnd, findErr := findLastCompleteBatchEnd(lastSeg.FilePath) + // After C4 fix, TailCorruptionError is only returned for the last + // segment. Use tce.SegmentPath as authoritative truncation target + // (defensive: fall back to segments[last] if missing). + corruptedPath := segments[len(segments)-1].FilePath + var tce *TailCorruptionError + if errors.As(err, &tce) && tce.SegmentPath != "" { + corruptedPath = tce.SegmentPath + } + + lastCompleteBatchEnd, findErr := findLastCompleteBatchEnd(corruptedPath) if findErr != nil { return nil, fmt.Errorf("wal: recover: find truncation offset: %w", findErr) } // Phase 1: truncated segment is always segments[last], no - // trailing empty segments to clean up. C4 fix will need to - // identify trailing empties based on the actually-corrupted - // segment's index, which is not the same as segments[last]. + // trailing empty segments to clean up. C4 fix means non-last + // segment corruption hard-fails above, so we never reach here + // with a non-last corrupted segment. var emptyTrailing []string - if err := truncateAndPersist(lastSeg.FilePath, lastCompleteBatchEnd, dir, emptyTrailing); err != nil { + if err := truncateAndPersist(corruptedPath, lastCompleteBatchEnd, dir, emptyTrailing); err != nil { // Per design §3.2 line 799: DB must NOT enter writable state. return nil, fmt.Errorf("wal: recover: persist tail truncation: %w", err) } diff --git a/wal/recover_test.go b/wal/recover_test.go index 1c4005a..e706ca1 100644 --- a/wal/recover_test.go +++ b/wal/recover_test.go @@ -118,7 +118,7 @@ func TestRecoverWithTailCorruption(t *testing.T) { // Verify the truncated file still parses cleanly. replayer2 := &mockReplayer{} - _, parseErr := ReplaySegmentFile(filePath, 100, replayer2) + _, parseErr := ReplaySegmentFile(filePath, 100, true, replayer2) if parseErr != nil { t.Fatalf("replay after truncation: %v", parseErr) } @@ -451,3 +451,152 @@ func TestRecoverInvalidBatchNotTruncatable(t *testing.T) { fi.Size(), int64(WalFileHeaderSize)+int64(len(rec))) } } + +// -------- C4 integration regression guards -------- + +// Regression guard for C4: middle segment CollectingFragments must hard-fail +// Recover, NOT truncate the (valid) last segment. This is the key bug Oracle +// flagged: "wal/recover.go 总是对 segments[len(segments)-1] 调用截断". +func TestRecoverMiddleSegmentCorruptionHardFails(t *testing.T) { + dir := t.TempDir() + // segment-0: [Batch seq 0-1] (2 entries → next=2) + // segment-1: [Batch seq 2-3] + [First][Middle no Last] ← middle corruption + // (2 complete entries → next=4 if recovery reached end) + // segment-2: [Batch seq 4-5] (valid; never reached) + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0"), makePutEntry("k1", "v1")}, + }) + + seg1Path := writeTestSegment(t, dir, 1, 2, [][]*WalEntry{ + {makePutEntry("k2", "v2"), makePutEntry("k3", "v3")}, + }) + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecFirst, []byte("first-frag"))) + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecMiddle, []byte("middle-frag"))) + + writeTestSegment(t, dir, 2, 4, [][]*WalEntry{ + {makePutEntry("k4", "v4"), makePutEntry("k5", "v5")}, + }) + + seg2Path := filepath.Join(dir, "segment-2.wal") + fiBefore, _ := os.Stat(seg2Path) + + _, err := Recover(dir, &mockReplayer{}) + if err == nil { + t.Fatal("expected Recover to fail on middle segment corruption") + } + if IsTailCorruption(err) { + t.Errorf("expected HARD error (not tail corruption) for middle segment; got %v", err) + } + + // CRITICAL: segment-2 must NOT be truncated (it's completely valid). + fiAfter, _ := os.Stat(seg2Path) + if fiAfter.Size() != fiBefore.Size() { + t.Errorf("segment-2 was modified: before=%d after=%d (C4-2 regression)", + fiBefore.Size(), fiAfter.Size()) + } +} + +// Regression guard for C4 (parser corruption half): middle segment CRC +// corruption must hard-fail Recover, NOT truncate the (valid) last segment. +// Tests the path where ParseBlock returns TailCorruptionError and +// ReplaySegmentFile converts it to hard error for non-last segment. +func TestRecoverMiddleSegmentCRCCorruptionHardFails(t *testing.T) { + dir := t.TempDir() + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0"), makePutEntry("k1", "v1")}, + }) + + seg1Path := writeTestSegment(t, dir, 1, 2, [][]*WalEntry{ + {makePutEntry("k2", "v2"), makePutEntry("k3", "v3")}, + }) + // Append CRC-corrupted bytes (will fail DecodePhysicalRecord CRC check). + appendFileBytes(t, seg1Path, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}) + + writeTestSegment(t, dir, 2, 4, [][]*WalEntry{ + {makePutEntry("k4", "v4"), makePutEntry("k5", "v5")}, + }) + + seg2Path := filepath.Join(dir, "segment-2.wal") + fiBefore, _ := os.Stat(seg2Path) + + _, err := Recover(dir, &mockReplayer{}) + if err == nil { + t.Fatal("expected Recover to fail on middle segment CRC corruption") + } + if IsTailCorruption(err) { + t.Errorf("expected HARD error (not tail corruption); got %v", err) + } + + fiAfter, _ := os.Stat(seg2Path) + if fiAfter.Size() != fiBefore.Size() { + t.Errorf("segment-2 modified: before=%d after=%d", fiBefore.Size(), fiAfter.Size()) + } +} + +// Regression guard for C4: single-segment tail corruption still truncates +// correctly (existing behavior preserved). +func TestRecoverLastSegmentCorruptionTruncatesCorrectly(t *testing.T) { + dir := t.TempDir() + batchA := []*WalEntry{makePutEntry("k0", "v0")} + batchB := []*WalEntry{makePutEntry("k1", "v1")} + filePath := writeTestSegment(t, dir, 0, 0, [][]*WalEntry{batchA, batchB}) + + encA, _ := EncodeWalBatch(0, batchA) + encB, _ := EncodeWalBatch(1, batchB) + endOfBatch1 := int64(WalFileHeaderSize) + + int64(PhysicalRecordHeaderSize+len(encA)) + + int64(PhysicalRecordHeaderSize+len(encB)) + + // Append partial fragments to last (only) segment. + appendFileBytes(t, filePath, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + result, err := Recover(dir, &mockReplayer{}) + if err != nil { + t.Fatalf("Recover: %v", err) + } + if !result.Truncated { + t.Fatal("Truncated = false, want true for last-segment corruption") + } + + fi, _ := os.Stat(filePath) + if fi.Size() != endOfBatch1 { + t.Errorf("file size = %d, want %d", fi.Size(), endOfBatch1) + } +} + +// Regression guard for C4: multi-segment with last-segment corruption still +// works correctly (the legitimate tail-truncation case). +func TestRecoverMultiSegmentLastSegmentCorruptionTruncatesLast(t *testing.T) { + dir := t.TempDir() + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k0", "v0")}, + }) + seg1Path := writeTestSegment(t, dir, 1, 1, [][]*WalEntry{ + {makePutEntry("k1", "v1")}, + {makePutEntry("k2", "v2")}, + }) + + encA, _ := EncodeWalBatch(1, []*WalEntry{makePutEntry("k1", "v1")}) + encB, _ := EncodeWalBatch(2, []*WalEntry{makePutEntry("k2", "v2")}) + endOfBatch2 := int64(WalFileHeaderSize) + + int64(PhysicalRecordHeaderSize+len(encA)) + + int64(PhysicalRecordHeaderSize+len(encB)) + + // Append partial fragments to segment-1 (the LAST segment). + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, seg1Path, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + result, err := Recover(dir, &mockReplayer{}) + if err != nil { + t.Fatalf("Recover: %v", err) + } + if !result.Truncated { + t.Fatal("Truncated = false, want true") + } + + fi, _ := os.Stat(seg1Path) + if fi.Size() != endOfBatch2 { + t.Errorf("segment-1 size = %d, want %d (end of Batch 2)", fi.Size(), endOfBatch2) + } +} diff --git a/wal/recovery.go b/wal/recovery.go index 324d2ab..ffa33a4 100644 --- a/wal/recovery.go +++ b/wal/recovery.go @@ -100,13 +100,35 @@ func ReplayBatch(batch *WalBatch, expectedSequence uint64, replayer BatchReplaye } // ReplaySegmentFile replays all complete WAL batches from one segment file. -func ReplaySegmentFile(filePath string, startSequence uint64, replayer BatchReplayer) (nextSequence uint64, err error) { +// +// isLastSegment controls how parse errors and CollectingFragments-at-end are +// classified per design §3.2 line 704: +// - true: tail corruption (TailCorruptionError, truncatable by Recover) +// - false: hard corruption (plain error, Recover must hard-fail) +func ReplaySegmentFile(filePath string, startSequence uint64, isLastSegment bool, replayer BatchReplayer) (nextSequence uint64, err error) { nextSequence = startSequence records, parseErr := ParseRecordsFromFile(filePath) if parseErr != nil && !IsTailCorruption(parseErr) { return nextSequence, fmt.Errorf("wal: parse segment records: %w", parseErr) } + // Attach SegmentPath to parseErr for downstream diagnostics + truncation target. + if parseErr != nil { + var tce *TailCorruptionError + if errors.As(parseErr, &tce) { + tce.SegmentPath = filePath + } + } + + // C4 fix: tail corruption in non-last segment is hard corruption per + // design §3.2 line 704. Use %v (NOT %w) so IsTailCorruption returns false + // — otherwise errors.As would still find the underlying *TailCorruptionError + // through the %w chain and Recover would treat it as truncatable. + if parseErr != nil && !isLastSegment { + return nextSequence, fmt.Errorf("wal: corruption in non-last segment %s (hard corruption): %v", + filePath, parseErr) + } + collector := NewFragmentCollector() for _, record := range records { if err := collector.Append(record.Type, record.Payload); err != nil { @@ -131,10 +153,16 @@ func ReplaySegmentFile(filePath string, startSequence uint64, replayer BatchRepl return nextSequence, parseErr } if collector.State() == FragmentCollecting { - return nextSequence, &TailCorruptionError{ - Offset: 0, - Err: errors.New("incomplete fragmented batch at segment tail"), + if isLastSegment { + return nextSequence, &TailCorruptionError{ + Offset: 0, + SegmentPath: filePath, + Err: errors.New("incomplete fragmented batch at segment tail"), + } } + // Non-last segment with incomplete fragments = middle corruption. + // Plain error (no TailCorruptionError) so IsTailCorruption is false. + return nextSequence, fmt.Errorf("wal: incomplete fragmented batch in non-last segment %s (hard corruption)", filePath) } return nextSequence, nil @@ -151,12 +179,13 @@ func RecoverFromSegments(dir string, recoverySegmentID uint64, replayer BatchRep } nextSequence = segments[0].StartSequence - for _, segment := range segments { + for i, segment := range segments { if segment.StartSequence != nextSequence { return nextSequence, fmt.Errorf("wal: segment start sequence %d does not match expected sequence %d", segment.StartSequence, nextSequence) } - nextSequence, err = ReplaySegmentFile(segment.FilePath, nextSequence, replayer) + isLastSegment := i == len(segments)-1 + nextSequence, err = ReplaySegmentFile(segment.FilePath, nextSequence, isLastSegment, replayer) if err != nil { if IsTailCorruption(err) { return nextSequence, err diff --git a/wal/recovery_test.go b/wal/recovery_test.go index 134532a..292367a 100644 --- a/wal/recovery_test.go +++ b/wal/recovery_test.go @@ -4,6 +4,7 @@ import ( "errors" "math" "os" + "path/filepath" "reflect" "strings" "testing" @@ -121,7 +122,7 @@ func TestReplaySegmentFileFull(t *testing.T) { }) replayer := &mockReplayer{} - next, err := ReplaySegmentFile(filePath, 50, replayer) + next, err := ReplaySegmentFile(filePath, 50, true, replayer) if err != nil { t.Fatalf("ReplaySegmentFile: %v", err) } @@ -147,7 +148,7 @@ func TestReplaySegmentFileTailCorruption(t *testing.T) { appendFileBytes(t, filePath, []byte{0x01, 0x02, 0x03}) replayer := &mockReplayer{} - next, err := ReplaySegmentFile(filePath, 70, replayer) + next, err := ReplaySegmentFile(filePath, 70, true, replayer) if err == nil { t.Fatal("ReplaySegmentFile succeeded, want tail corruption error") } @@ -250,3 +251,88 @@ func appendFileBytes(t *testing.T, filePath string, data []byte) { t.Fatalf("Write corruption bytes: %v", err) } } + +// -------- C4 regression guards: isLastSegment controls corruption classification -------- + +// Regression guards for C4: isLastSegment controls whether CollectingFragments +// at end is tail corruption (truncatable) or hard corruption (must hard-fail). + +func TestReplaySegmentFile_LastSegmentCollectingFragmentsIsTailCorruption(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k", "v")}, + }) + // Append First + Middle (no Last) to leave collector in Collecting state. + appendFileBytes(t, filePath, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + _, err := ReplaySegmentFile(filePath, 0, true, &mockReplayer{}) + if err == nil { + t.Fatal("expected error") + } + if !IsTailCorruption(err) { + t.Errorf("expected TailCorruptionError for last segment, got: %v", err) + } + var tce *TailCorruptionError + if errors.As(err, &tce) { + if tce.SegmentPath != filePath { + t.Errorf("SegmentPath = %q, want %q", tce.SegmentPath, filePath) + } + } +} + +func TestReplaySegmentFile_NonLastSegmentCollectingFragmentsIsHardError(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k", "v")}, + }) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecFirst, []byte("first"))) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecMiddle, []byte("middle"))) + + _, err := ReplaySegmentFile(filePath, 0, false, &mockReplayer{}) + if err == nil { + t.Fatal("expected error") + } + if IsTailCorruption(err) { + t.Errorf("expected HARD error for non-last segment, got TailCorruptionError: %v", err) + } +} + +func TestReplaySegmentFile_LastSegmentParseErrorIsTailCorruption(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k", "v")}, + }) + // Append corrupted bytes to trigger ParseBlock's CRC failure path. + appendFileBytes(t, filePath, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}) + + _, err := ReplaySegmentFile(filePath, 0, true, &mockReplayer{}) + if err == nil { + t.Fatal("expected error") + } + if !IsTailCorruption(err) { + t.Errorf("expected TailCorruptionError for last segment parse error, got: %v", err) + } +} + +func TestReplaySegmentFile_NonLastSegmentParseErrorIsHardError(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("k", "v")}, + }) + appendFileBytes(t, filePath, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}) + + _, err := ReplaySegmentFile(filePath, 0, false, &mockReplayer{}) + if err == nil { + t.Fatal("expected error") + } + // Per C4 fix: non-last segment parser corruption is hard error (NOT TailCorruption). + // Implemented via %v (not %w) so errors.As cannot find underlying TailCorruptionError. + if IsTailCorruption(err) { + t.Errorf("expected HARD error for non-last segment parse error, got TailCorruptionError: %v", err) + } +}