# C5 + H8 修复方案:truncation 持久化 + batch-aware 截断点 ## TL;DR > **目标**:让 WAL 尾部截断真正按设计 §3.2 line 787-800 执行 4 步协议(ftruncate + fsync segment + 删空 segment + fsync dir),并让截断点基于完整 batch 边界而不是物理 record 边界。两条 bug 在同一段代码(truncation 路径),捆绑修最经济。 > > **交付**: > - 新函数 `findLastCompleteBatchEnd` —— batch-aware 的截断点计算(替换 `findValidOffset`) > - 新函数 `truncateAndPersist` —— 4 步串行执行(替换 `truncateSegment`) > - `Recover` 调用新函数,**失败硬错误**(不再吞) > - 复用 C6 的 `dirFsyncFn` 做 step 4 > - 新增 ~6 个测试覆盖各场景 > - 单次 commit > > **预估工时**:3.5-5 小时(Oracle 修订后加了 block-boundary、padding、retry、invalid-batch 等测试) > **风险**:中。改动局限在 `wal/recover.go` 一个文件 + 一处 record_parser 钩子,但 truncation 涉及多步 fsync,要小心顺序 --- ## Context ### Bug 摘要 #### C5(truncation 4 步缺失) `wal/recover.go:62-68`: ```go validOffset, truncErr := findValidOffset(lastSeg.FilePath) if truncErr != nil { result.TruncateError = fmt.Errorf("%w (find valid offset: %v)", err, truncErr) // ← 吞 } else if truncErr := truncateSegment(lastSeg.FilePath, validOffset); truncErr != nil { result.TruncateError = fmt.Errorf("%w (truncate: %v)", err, truncErr) // ← 吞 } ``` `truncateSegment` 只调 `os.Truncate`,**缺**: - Step 2: fsync 被截断的 segment - Step 3: 删除空的后续 segment - Step 4: fsync WAL directory 错误被吞到 `result.TruncateError`,recovery 仍返回成功。DB 进入可写状态。 #### H8(截断点不跟踪 batch 边界) `findValidOffset` 只用 `DecodePhysicalRecord` 校验物理记录,**不跟踪 fragment 状态机**。尾部 `First + Middle*` 没 `Last` 时返回错误截断点(残留半截 fragment),导致下次启动再次报 tail corruption,**反复 repair**。 ### 设计依据 `docs/design.md` §3.2 line 786-800: > 截断目标始终是**最后一个完整 WAL Batch 的结束位置** `lastCompleteBatchEnd` ``` 1. ftruncate 当前 active segment 到 lastCompleteBatchEnd 2. fsync 被截断的 segment 3. 删除 startSequence == expectedSequence 且不含任何 complete batch 的后续空 segment 4. fsync WAL directory ``` > 若 ftruncate、segment fsync、空 segment 删除或 WAL directory fsync **任一步失败,recovery 必须报错,DB 不得进入可写状态** ### 协同:复用 C6 的 `dirFsyncFn` C6 已经抽出 `dirFsyncFn`(package-level 变量支持测试注入)。本次 fix 的 Step 4 直接复用,无需再抽一个。 ### 错误传播 `Recover` 失败 → `DB.Open` 失败 → 用户看到 error,DB 没进可写状态 ✓ --- ## 执行计划 ### Phase A:代码改动(90-120 分钟) #### A.1 新增 `findLastCompleteBatchEnd`(替换 `findValidOffset`) 文件:`wal/recover.go` > **Oracle 修订(bg_f1e4db4f)BLOCKING**:原版看到 zero padding 立即 return,但 WAL 格式允许 full block 末尾 padding + 下一个 block 继续写 record。必须区分"full block 内 padding(continue 下一块)"和"short block 尾部 padding(return)"。 ```go // findLastCompleteBatchEnd walks the segment file, runs physical records // through the FragmentCollector state machine, and returns the byte offset // of the END of the last complete WAL Batch. // // This is the correct truncation target per design §3.2 line 786. The // previous findValidOffset only checked physical record CRCs, missing // the case where a First + Middle* fragment chain has no Last (H8 bug): // physical CRCs pass but no complete batch exists at that offset. // // Block-boundary handling: WAL format allows a full block to end with // zero padding when the next record doesn't fit (see BlockWriter.paddingNeeded). // This function must CONTINUE to the next block on padding in a full block, // and only RETURN on padding in a short (final) block or actual corruption. func findLastCompleteBatchEnd(filePath string) (int64, error) { f, err := os.Open(filePath) if err != nil { return 0, fmt.Errorf("open %s: %w", filePath, err) } defer f.Close() if _, err := f.Seek(WalFileHeaderSize, 0); err != nil { return 0, fmt.Errorf("seek past header: %w", err) } collector := NewFragmentCollector() lastCompleteEnd := int64(WalFileHeaderSize) blockStartOffset := int64(WalFileHeaderSize) buf := make([]byte, WalBlockSize) for { n, readErr := f.Read(buf) if n > 0 { blockData := buf[:n] isFullBlock := n == WalBlockSize && readErr == nil pos := 0 for pos < len(blockData) { remaining := len(blockData) - pos if remaining < PhysicalRecordHeaderSize { if isFullBlock { break // padding in full block, continue to next block } return lastCompleteEnd, nil // tail padding in short block } if isAllZeros(blockData[pos : pos+PhysicalRecordHeaderSize]) { if isFullBlock { break // zero-led padding in full block, continue } return lastCompleteEnd, nil // tail padding } rec, consumed, err := DecodePhysicalRecord(blockData[pos:]) if err != nil { return lastCompleteEnd, nil // physical corruption } if err := collector.Append(rec.Type, rec.Payload); err != nil { return lastCompleteEnd, nil // fragment state machine rejected } pos += consumed recordEndAbsolute := blockStartOffset + int64(pos) if collector.IsComplete() { lastCompleteEnd = recordEndAbsolute collector.Reset() } } blockStartOffset += int64(n) } if readErr != nil || n < WalBlockSize { break } } return lastCompleteEnd, nil } ``` **关键不变量**: - `lastCompleteEnd` 只在 collector 回到 Idle(完整 batch 形成)时推进 - 跨 block padding:`isFullBlock` 判断决定 continue vs return - 任何中断(short block padding / 物理损坏 / fragment 顺序错)→ 返回当前 `lastCompleteEnd` - EOF 时如果 collector 在 Collecting 状态 → 不推进(半截 batch 不算) #### A.2 新增 `truncateAndPersist`(替换 `truncateSegment`) 文件:`wal/recover.go` ```go // segmentFsyncFn is the package-level indirection for fsyncing a truncated // segment file. Tests that override this must not use t.Parallel(). var segmentFsyncFn = segmentFsync func segmentFsync(filePath string) error { f, err := os.OpenFile(filePath, os.O_WRONLY, 0o644) if err != nil { return fmt.Errorf("open for fsync: %w", err) } defer f.Close() if err := f.Sync(); err != nil { return fmt.Errorf("fsync: %w", err) } return nil } // truncateAndPersist executes the 4-step tail-truncation protocol per // design §3.2 line 787-794. Any step failure is fatal: per line 799, // DB must NOT enter writable state if truncation cannot be persisted. // // Steps: // 1. ftruncate segment to lastCompleteBatchEnd // 2. fsync the truncated segment // 3. delete trailing empty segments (startSequence == expectedSequence // and no complete batch) // 4. fsync WAL directory (reuses dirFsyncFn from C6) func truncateAndPersist( filePath string, lastCompleteBatchEnd int64, dir string, emptyTrailingSegments []string, ) error { // Step 1: ftruncate if err := os.Truncate(filePath, lastCompleteBatchEnd); err != nil { return fmt.Errorf("ftruncate %s to %d: %w", filePath, lastCompleteBatchEnd, err) } // Step 2: fsync the truncated segment if err := segmentFsyncFn(filePath); err != nil { return fmt.Errorf("fsync truncated segment: %w", err) } // Step 3: delete empty trailing segments for _, segPath := range emptyTrailingSegments { if err := os.Remove(segPath); err != nil { return fmt.Errorf("remove empty segment %s: %w", segPath, err) } } // Step 4: fsync WAL directory (reuses C6's dirFsyncFn) if err := dirFsyncFn(dir); err != nil { return fmt.Errorf("fsync WAL dir after truncation: %w", err) } return nil } ``` > **关于 hook 触发的注释**:docstring 引用设计行号 + 列出 4 步,是必要的回归防护。`segmentFsyncFn` 的 not-parallel-safe 注释和 C6 的 `dirFsyncFn` 一致。 #### A.3 重构 `Recover` 失败路径 文件:`wal/recover.go`,替换 line 59-69 的 truncation 块: ```go // 改前(C5 bug:吞错误): if len(segments) > 0 { lastSeg := segments[len(segments)-1] validOffset, truncErr := findValidOffset(lastSeg.FilePath) if truncErr != nil { result.TruncateError = fmt.Errorf("%w (find valid offset: %v)", err, truncErr) } else if truncErr := truncateSegment(lastSeg.FilePath, validOffset); truncErr != nil { result.TruncateError = fmt.Errorf("%w (truncate: %v)", err, truncErr) } } // 改后(按设计 line 787-800 执行 4 步,失败硬错误): if len(segments) > 0 { lastSeg := segments[len(segments)-1] lastCompleteBatchEnd, findErr := findLastCompleteBatchEnd(lastSeg.FilePath) 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. var emptyTrailing []string if err := truncateAndPersist(lastSeg.FilePath, 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) } } ``` > **TruncateError 字段语义澄清(Oracle 修订)**:`result.TruncateError` 在 `Recover` line 42-46 创建 result 时被设置为 `err`(the original tail corruption error)。这个字段保留为 **informational**:"tail corruption was found and repaired"。现有测试 `TestRecoverWithTailCorruption` 和 `TestRecoverIdempotentAfterTruncation` 依赖这个非 nil 检查。 > > **不要再**用 `TruncateError` 报告 truncation 持久化失败 —— 那种失败现在硬错误返回。两种语义不混。 #### A.4 `emptyTrailingSegments` Phase 1 简化(Oracle 修订) > **Oracle 修订(bg_f1e4db4f)**:原计划的 `identifyEmptyTrailingSegments` 函数在 Phase 1 永远返回 nil,且其签名不足以支持 C4 修复(C4 需要 corrupted segment 的 index/path,不只是 `segments` 和 `expectedSequence`)。**删除函数**,改用 inline 注释 + `nil`。C4 修复时再补正确的实现。 `Recover` 内部直接: ```go // 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. var emptyTrailing []string // always nil in Phase 1 if err := truncateAndPersist(lastSeg.FilePath, lastCompleteBatchEnd, dir, emptyTrailing); err != nil { return nil, fmt.Errorf("wal: recover: persist tail truncation: %w", err) } ``` `truncateAndPersist` 的签名保留 `emptyTrailingSegments []string` 参数(forward compat),Phase 1 永远传 nil。`hasCompleteBatch` helper 也不需要了。 #### A.5 删除老函数 `wal/recover.go` 中删除: - `truncateSegment`(被 `truncateAndPersist` 替换) - `findValidOffset`(被 `findLastCompleteBatchEnd` 替换) ### Phase B:测试(60-90 分钟) #### B.1 `findLastCompleteBatchEnd` 单元测试 新增 `wal/recover_offset_test.go`: ```go // Regression guards for H8: findLastCompleteBatchEnd must return the // offset of the last COMPLETE batch, not the last physical record. func TestFindLastCompleteBatchEnd_CleanSegment(t *testing.T) { // 2 complete batches, no partial tail. // Expect: offset = end of batch 2. } func TestFindLastCompleteBatchEnd_PartialTailFragment(t *testing.T) { // 1 complete batch + First + Middle* (no Last). // Expect: offset = end of batch 1 (NOT after the Middle fragment). // This is the H8 regression case. } func TestFindLastCompleteBatchEnd_NoBatches(t *testing.T) { // Empty segment (only header). // Expect: offset = WalFileHeaderSize. } func TestFindLastCompleteBatchEnd_PhysicalCorruption(t *testing.T) { // 1 complete batch + corrupted bytes (bad CRC). // Expect: offset = end of batch 1. } func TestFindLastCompleteBatchEnd_PartialBatchOnly(t *testing.T) { // Only First + Middle* at the start (no complete batch ever). // Expect: offset = WalFileHeaderSize. } // Oracle BLOCKING test: must distinguish "padding in full block (continue // to next block)" from "padding in short block (return)". func TestFindLastCompleteBatchEnd_BlockBoundaryPadding(t *testing.T) { // Construct: [Batch A in block 1 (filling)] [padding to end of block 1] // [Batch B in block 2] // Expect: offset = end of Batch B in block 2 (NOT end of Batch A). // This catches the bug where the original plan returned on first padding. } func TestFindLastCompleteBatchEnd_NonZeroTailPadding(t *testing.T) { // 1 complete batch + 3 non-zero bytes (< PhysicalRecordHeaderSize). // Expect: offset = end of batch 1. } func TestFindLastCompleteBatchEnd_ZeroTailPadding(t *testing.T) { // 1 complete batch + 3 zero bytes. // Expect: offset = end of batch 1. } ``` #### B.2 `truncateAndPersist` 测试 新增到 `wal/recover_offset_test.go`: ```go // Regression guards for C5: 4-step protocol must execute all steps. func TestTruncateAndPersist_Success(t *testing.T) { // Create segment, write some bytes past lastCompleteBatchEnd. // Call truncateAndPersist. // Verify: // - File size == lastCompleteBatchEnd (Step 1) // - File fsync'd (hard to verify directly; trust the call) // - Empty trailing segments deleted (Step 3) // - Dir fsync'd (Step 4) } func TestTruncateAndPersist_FtruncateFailure(t *testing.T) { // Pass non-existent file path. // Expect: error mentioning "ftruncate". } func TestTruncateAndPersist_DirFsyncFailure(t *testing.T) { // Inject dirFsyncFn failure (reuse C6 injection). // Expect: error mentioning "fsync WAL dir". // Note: file IS truncated (step 1 succeeded), but step 4 failed. // Per design, Recover must return error → DB.Open fails. } func TestTruncateAndPersist_SegmentFsyncFailure(t *testing.T) { // Inject segmentFsyncFn failure. // Expect: error mentioning "fsync truncated segment". } // Oracle nice-to-have: verify retry after dir-fsync failure. func TestTruncateAndPersist_RetryAfterDirFsyncFailure(t *testing.T) { // First call: inject dirFsyncFn failure → error. // Second call: restore dirFsyncFn, call again → success. // Verifies state stays recoverable across failures. } ``` #### B.3 `Recover` 集成测试 修改或新增到 `wal/recover_test.go`: ```go // Regression guard for C5+H8: end-to-end recovery with partial fragment // tail must persist truncation correctly and be idempotent. func TestRecoverPartialFragmentTailIdempotent(t *testing.T) { dir := t.TempDir() // Build segment with: [Batch A] [Batch B] [First + Middle* no Last] // ... replayer1 := &mockReplayer{} result1, err := Recover(dir, replayer1) if err != nil { t.Fatalf("1st Recover: %v", err) } if !result1.Truncated { t.Fatal("Truncated = false, want true") } // Verify file size == end of Batch B (H8 fix) fi, _ := os.Stat(segmentPath) if fi.Size() != expectedBatchBEnd { t.Errorf("file size = %d, want %d (last complete batch end)", fi.Size(), expectedBatchBEnd) } // Second recovery should not see corruption (file is clean now) replayer2 := &mockReplayer{} result2, err := Recover(dir, replayer2) if err != nil { t.Fatalf("2nd Recover: %v", err) } if result2.Truncated { t.Error("2nd Recover should not see corruption") } if result1.NextSequence != result2.NextSequence { t.Errorf("NextSequence differs: %d vs %d", result1.NextSequence, result2.NextSequence) } } // Regression guard for C5: any truncation step failure must fail DB.Open. func TestRecoverTruncationFailureFailsRecovery(t *testing.T) { dir := t.TempDir() // Build segment with tail corruption // ... // Inject dirFsyncFn failure orig := dirFsyncFn dirFsyncFn = func(string) error { return errors.New("simulated") } t.Cleanup(func() { dirFsyncFn = orig }) _, err := Recover(dir, &mockReplayer{}) if err == nil { t.Fatal("expected Recover to fail when truncation persist fails") } if !strings.Contains(err.Error(), "persist tail truncation") { t.Errorf("error should mention 'persist tail truncation', got: %v", err) } } // Oracle nice-to-have: CRC-valid but batch-content-invalid must hard-fail // through DecodeWalBatch, NOT enter truncation path. Per design line 778-781. func TestRecoverInvalidBatchNotTruncatable(t *testing.T) { dir := t.TempDir() // Construct segment with: physical records CRC-valid, but assembled // batch has invalid header (e.g., entryCount=0). // Expect: Recover returns error (NOT tail corruption, NOT success). // Expect: file is NOT truncated. } ``` > **注意**:现有的 `TestRecoverIdempotentAfterTruncation` 测试(C2+C3 加的)已经覆盖了"截断后第二次 Recover 干净"的场景。本次新增的 `TestRecoverPartialFragmentTailIdempotent` 专门覆盖 **H8**(partial fragment tail),是更严格的回归测试。 > > **Oracle 提醒**:检查现有 `TestRecoverIdempotentAfterTruncation` 的 `TruncateError` 期望是否需要更新(原期望非 nil,新行为下仍非 nil 因为 `result.TruncateError` 在 result 创建时就设了 original err,仍保留 informational 语义)。 ### Phase C:验证(15 分钟) ```bash # 1. 编译 go build ./... # 2. 重点测试 go test ./wal -run 'TestFindLastCompleteBatchEnd|TestTruncateAndPersist|TestRecoverPartialFragmentTail|TestRecoverTruncationFailure|TestRecoverIdempotent' -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 ./... # 7. lint(如果装了) if command -v golangci-lint >/dev/null 2>&1; then golangci-lint run ./wal/... fi ``` ### Phase D:Commit message draft ``` fix: persist WAL tail truncation per design protocol (C5+H8) Recovery tail-truncation had two compounding bugs in wal/recover.go: C5: truncateSegment only called os.Truncate. Missing per design §3.2 line 787-794: - Step 2: fsync the truncated segment - Step 3: delete empty trailing segments - Step 4: fsync WAL directory And all errors were swallowed into result.TruncateError with recovery still returning success, violating design line 799: "若 ftruncate、 segment fsync、空 segment 删除或 WAL directory fsync 任一步失败, recovery 必须报错,DB 不得进入可写状态". H8: findValidOffset only checked physical record CRCs, ignoring the FragmentCollector state machine. For a tail of First + Middle* without Last, it returned the offset AFTER the last Middle fragment instead of the last COMPLETE batch end. Result: residual half-batch fragments caused repeated tail-corruption reports on every restart. Changes: - wal/recover.go: - Add findLastCompleteBatchEnd: batch-aware offset finder using FragmentCollector state machine. Returns true last batch boundary. - Add truncateAndPersist: 4-step protocol (ftruncate + fsync segment + delete empty trailing + fsync dir). Any step failure is fatal. - Add segmentFsyncFn (package-level var for test injection, same pattern as C6's dirFsyncFn). - Refactor Recover failure path: use new functions, hard-error on truncation persist failure (was: swallow to TruncateError). - Delete findValidOffset and truncateSegment (replaced). - wal/recover_offset_test.go (new): 5 unit tests for findLastCompleteBatchEnd covering clean/partial-tail/no-batch/ physical-corruption/partial-only cases. 4 unit tests for truncateAndPersist covering success/ftruncate-fail/dir-fsync-fail/ segment-fsync-fail. - wal/recover_test.go: add TestRecoverPartialFragmentTailIdempotent (H8 e2e regression) and TestRecoverTruncationFailureFailsRecovery (C5 e2e regression). Injection note: segmentFsyncFn and dirFsyncFn (from C6) are package-level vars; tests that override either must not use t.Parallel(). Verified: each new test fails on pre-fix code and passes after the fix. Full suite green including go test -race ./... . Phase 1 simplification: emptyTrailingSegments is always nil in Phase 1 (truncated segment is always segments[last]). The parameter is kept in truncateAndPersist's signature for forward compatibility with the C4 fix. Audit context: docs/audit-3.2.md C5 and H8 (H8 Oracle-verified bg_ef425776). ``` --- ## 验收清单 - [ ] Phase A.1:`findLastCompleteBatchEnd` 存在,使用 FragmentCollector - [ ] Phase A.2:`truncateAndPersist` 存在,4 步串行执行 - [ ] Phase A.2:`segmentFsyncFn` package-level 变量存在 - [ ] Phase A.3:`Recover` 失败路径调用新函数,失败硬错误 - [ ] Phase A.4:`identifyEmptyTrailingSegments` + `hasCompleteBatch` helper 存在 - [ ] Phase A.5:`truncateSegment` 和 `findValidOffset` 已删除 - [ ] Phase B.1:5 个 `findLastCompleteBatchEnd` 单元测试存在 - [ ] Phase B.2:4 个 `truncateAndPersist` 单元测试存在 - [ ] Phase B.3:2 个 Recover 集成测试存在 - [ ] `go test ./wal/... -count=1` 全绿 - [ ] `go test ./... -count=1` 全绿 - [ ] `go test -race ./... -count=1` 全绿 - [ ] `go vet ./...` 无新增警告 - [ ] 单次 commit,message 引用 audit C5+H8 --- ## 不在本次范围内(后续 issue) | 编号 | 为什么不放进来 | |------|---------------| | C4 | 同一文件但不耦合。C4 修好后 `identifyEmptyTrailingSegments` 才会返回非 nil,但本 patch 已经写了前向兼容代码 | | C8 | segment_manager.go startSequence bug,独立 | | C1 | CRC 多项式错,独立 | | C7 | Put/Close 竞态,独立 | | H1-H7 | 其他 High,独立 | --- ## 修订记录 - **v1(原始)**:C5+H8 修复方案初稿,送 Momus 审 - **v1.0(Momus 审核 bg_c8823195)**:[OKAY],无 blocking。思考过程提到 batch-content validity 担忧,Oracle Q2 确认非阻塞(upstream ReplaySegmentFile 已经处理) - **v1.1(Oracle 修订 bg_f1e4db4f)**: - **BLOCKING**:`findLastCompleteBatchEnd` 跨 block padding bug — full block 末尾 padding 应 continue 到下一块,不是 return。加 `isFullBlock` 判断 - **BLOCKING**:缺跨 block 测试,加 `TestFindLastCompleteBatchEnd_BlockBoundaryPadding` - **NEW**:加非零 padding / 零 padding 测试(Q1 f/g) - **NEW**:加 `TestTruncateAndPersist_RetryAfterDirFsyncFailure` - **NEW**:加 `TestRecoverInvalidBatchNotTruncatable`(CRC-valid 但 batch 内容无效,按 design line 778-781 应硬错误不截断) - **CHANGE**:删除 `identifyEmptyTrailingSegments` + `hasCompleteBatch`(dead code,签名不足以支持 C4)。改 inline `var emptyTrailing []string` + 注释 - **CLARIFY**:`TruncateError` 字段保留 informational 语义("tail corruption was found and repaired"),不再用于报告 truncation 持久化失败 - **FIX**:commit message 去掉 "C5 Oracle-verified bg_ef425776"(C5 实际未经过 Oracle 修订,只有 C2/C3/C4/H8 有) - **BUMP**:估时 2.5-3.5h → 3.5-5h