go-kv/.omo/plans/fix-c5-h8-truncation-persist.md

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：

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）"。

// 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

// 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 块：

// 改前（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 内部直接：

// 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：

// 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：

// 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：

// 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 分钟）

# 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