diff --git a/.omo/plans/fix-c5-h8-truncation-persist.md b/.omo/plans/fix-c5-h8-truncation-persist.md new file mode 100644 index 0000000..b5564dc --- /dev/null +++ b/.omo/plans/fix-c5-h8-truncation-persist.md @@ -0,0 +1,592 @@ +# 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 diff --git a/wal/recover.go b/wal/recover.go index 907b452..10fd48a 100644 --- a/wal/recover.go +++ b/wal/recover.go @@ -13,12 +13,15 @@ type RecoveryResult struct { NextSegmentID uint64 ReplayedEntries int Truncated bool - TruncateError error // non-nil if tail corruption was found + // TruncateError is informational only: non-nil means "tail corruption + // was found and repair was attempted". It does NOT report persistence + // failures — those cause Recover to return an error instead. + TruncateError error } // Recover performs a full WAL recovery: reads the recovery checkpoint from -// MANIFEST (or CURRENT), scans segments, replays entries, and handles tail -// truncation. On success the MANIFEST is updated with the new recovery state. +// MANIFEST, scans segments, replays entries, and persists tail truncation +// per design §3.2 line 787-800. func Recover(dir string, replayer BatchReplayer) (*RecoveryResult, error) { if replayer == nil { return nil, fmt.Errorf("wal: recover: replayer is nil") @@ -37,15 +40,14 @@ func Recover(dir string, replayer BatchReplayer) (*RecoveryResult, error) { return nil, fmt.Errorf("wal: recover: %w", err) } - // Step 3: Tail corruption — truncate the last segment and accept - // partial data loss for Phase 1. + // Step 3: Tail corruption — truncate the last segment per design + // §3.2 line 787-800. result := &RecoveryResult{ NextSequence: nextSequence, Truncated: true, - TruncateError: err, + TruncateError: err, // informational: tail corruption was detected } - // Determine nextSegmentID from scanned segments. segments, scanErr := ScanSegments(dir, recoverySegmentID) if scanErr != nil { return nil, fmt.Errorf("wal: recover: scan after tail corruption: %w", scanErr) @@ -56,27 +58,31 @@ func Recover(dir string, replayer BatchReplayer) (*RecoveryResult, error) { result.NextSegmentID = recoverySegmentID } - // Truncate the last segment file to remove corrupted tail. if len(segments) > 0 { lastSeg := segments[len(segments)-1] - validOffset, truncErr := findValidOffset(lastSeg.FilePath) - if truncErr != nil { - // Best-effort: record the truncation error but don't fail recovery. - 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) + 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, which is not the same as segments[last]. + 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) } } - // Count replayed entries by re-scanning the replayer state. - // For Phase 1 we accept that ReplayedEntries may be approximate; - // the replayer interface doesn't expose a count. - result.ReplayedEntries = 0 // caller can inspect replayer directly + result.ReplayedEntries = 0 // Phase 1: replayer interface doesn't expose count // Per design §3.2 line 280, recovery repair must NOT update MANIFEST. - // The truncated WAL state is persisted via ftruncate (see C5 for the - // remaining fsync gaps). MANIFEST can only advance via checkpoint - // (MemTable flush) in future phases. + // The truncated WAL state is persisted via ftruncate + fsync segment + // + fsync dir (see truncateAndPersist). MANIFEST can only advance via + // checkpoint (MemTable flush) in future phases. return result, nil } @@ -114,23 +120,76 @@ func resolveRecoverySegmentID(dir string) (uint64, error) { return mf.RecoverySegmentID, nil } -// truncateSegment truncates the file at filePath to validOffset bytes, -// removing any corrupted data after that point. -func truncateSegment(filePath string, validOffset int64) error { - if validOffset < 0 { - return fmt.Errorf("wal: truncate: invalid offset %d", validOffset) +// segmentFsyncFn is the package-level indirection for fsyncing a truncated +// segment file. Tests that override this must not use t.Parallel(). +// Same pattern as dirFsyncFn (see wal/dir_fsync.go). +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) } - return os.Truncate(filePath, validOffset) + defer f.Close() + if err := f.Sync(); err != nil { + return fmt.Errorf("fsync: %w", err) + } + return nil } -// findValidOffset parses a segment file and returns the byte offset of the -// last valid record boundary. The offset includes the file header size. -func findValidOffset(filePath string) (int64, error) { - // Re-parse the file to find where valid records end. - // We need to track the byte offset as we parse. +// 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. +func truncateAndPersist( + filePath string, + lastCompleteBatchEnd int64, + dir string, + emptyTrailingSegments []string, +) error { + // Step 1: ftruncate + if lastCompleteBatchEnd < 0 { + return fmt.Errorf("wal: invalid truncate offset %d", lastCompleteBatchEnd) + } + 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 +} + +// 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. A previous +// version (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 CONTINUES to the next block on padding in a full block, and +// only RETURNS 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 for offset scan: %w", err) + return 0, fmt.Errorf("open %s: %w", filePath, err) } defer f.Close() @@ -138,58 +197,56 @@ func findValidOffset(filePath string) (int64, error) { return 0, fmt.Errorf("seek past header: %w", err) } - validOffset := int64(WalFileHeaderSize) + collector := NewFragmentCollector() + lastCompleteEnd := int64(WalFileHeaderSize) + blockStartOffset := int64(WalFileHeaderSize) buf := make([]byte, WalBlockSize) for { n, readErr := f.Read(buf) - if readErr != nil { - break - } - if n == 0 { - break - } + if n > 0 { + blockData := buf[:n] + isFullBlock := n == WalBlockSize && readErr == nil + pos := 0 + for pos < len(blockData) { + remaining := len(blockData) - pos - blockData := buf[:n] - blockStartOffset := validOffset - pos := 0 - - for pos < len(blockData) { - remaining := len(blockData) - pos - - if remaining < PhysicalRecordHeaderSize { - // Check if remaining bytes are zero-padding. - if isAllZeros(blockData[pos:]) { - // Valid padding — update offset to end of last valid record. - validOffset = blockStartOffset + int64(pos) + if remaining < PhysicalRecordHeaderSize { + if isFullBlock { + break // padding in full block, continue to next block + } + return lastCompleteEnd, nil // tail padding in short block } - // Either way, we're done with this block. - break - } - if isAllZeros(blockData[pos : pos+PhysicalRecordHeaderSize]) { - if isAllZeros(blockData[pos:]) { - validOffset = blockStartOffset + int64(pos) + if isAllZeros(blockData[pos : pos+PhysicalRecordHeaderSize]) { + if isFullBlock { + break // zero-led padding in full block, continue + } + return lastCompleteEnd, nil // tail padding } - break - } - _, consumed, err := DecodePhysicalRecord(blockData[pos:]) - if err != nil { - // Corruption starts here — offset is up to last valid record. - validOffset = blockStartOffset + int64(pos) - return validOffset, nil - } + 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 + } - // Valid record found. - validOffset = blockStartOffset + int64(pos+consumed) - pos += consumed + pos += consumed + recordEndAbsolute := blockStartOffset + int64(pos) + + if collector.IsComplete() { + lastCompleteEnd = recordEndAbsolute + collector.Reset() + } + } + blockStartOffset += int64(n) } - - if n < WalBlockSize { + if readErr != nil || n < WalBlockSize { break } } - return validOffset, nil + return lastCompleteEnd, nil } diff --git a/wal/recover_offset_test.go b/wal/recover_offset_test.go new file mode 100644 index 0000000..e577874 --- /dev/null +++ b/wal/recover_offset_test.go @@ -0,0 +1,363 @@ +package wal + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeRawSegmentHeader writes a 32-byte WAL header to filePath. Used by +// findLastCompleteBatchEnd tests to construct minimal segment files. +func writeRawSegmentHeader(t *testing.T, filePath string) { + t.Helper() + hdr := &WalFileHeader{ + BlockSize: 32 * 1024, + SegmentID: 0, + StartSequence: 0, + } + encoded := EncodeWalHeader(hdr) + if err := os.WriteFile(filePath, encoded[:], 0o644); err != nil { + t.Fatalf("WriteFile header: %v", err) + } +} + +// appendRawBytes appends arbitrary bytes to filePath. +func appendRawBytes(t *testing.T, filePath string, data []byte) { + t.Helper() + f, err := os.OpenFile(filePath, os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + t.Fatalf("OpenFile append: %v", err) + } + defer f.Close() + if _, err := f.Write(data); err != nil { + t.Fatalf("Write: %v", err) + } +} + +// appendFullRecord encodes and appends a complete WAL batch as a Full record. +func appendFullRecord(t *testing.T, filePath string, batch []byte) { + t.Helper() + rec := EncodePhysicalRecord(RecFull, batch) + appendRawBytes(t, filePath, rec) +} + +// appendFragment encodes and appends a single fragment record. +func appendFragment(t *testing.T, filePath string, recType uint8, payload []byte) { + t.Helper() + rec := EncodePhysicalRecord(recType, payload) + appendRawBytes(t, filePath, rec) +} + +func TestFindLastCompleteBatchEnd_CleanSegment(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + batchA := []byte("batch-A-content") + batchB := []byte("batch-B-content") + appendFullRecord(t, filePath, batchA) + endOfA, _ := fileSize(filePath) + appendFullRecord(t, filePath, batchB) + endOfB, _ := fileSize(filePath) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfB { + t.Errorf("got %d, want %d (end of Batch B)", got, endOfB) + } + if got == endOfA { + t.Errorf("got end of Batch A, should be end of Batch B") + } +} + +// Regression guard for H8: partial fragment tail must return end of last +// COMPLETE batch, not end of last physical record. +func TestFindLastCompleteBatchEnd_PartialTailFragment(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + batchA := []byte("batch-A-content") + appendFullRecord(t, filePath, batchA) + endOfA, _ := fileSize(filePath) + + // Append First + Middle fragments (no Last) — H8 case. + appendFragment(t, filePath, RecFirst, []byte("first-fragment-data")) + appendFragment(t, filePath, RecMiddle, []byte("middle-fragment-data")) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfA { + t.Errorf("got %d, want %d (end of Batch A, NOT end of Middle fragment)", got, endOfA) + } +} + +func TestFindLastCompleteBatchEnd_NoBatches(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != int64(WalFileHeaderSize) { + t.Errorf("got %d, want %d (WalFileHeaderSize)", got, WalFileHeaderSize) + } +} + +func TestFindLastCompleteBatchEnd_PhysicalCorruption(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + batchA := []byte("batch-A-content") + appendFullRecord(t, filePath, batchA) + endOfA, _ := fileSize(filePath) + + // Append corrupted bytes (will fail CRC). + appendRawBytes(t, filePath, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfA { + t.Errorf("got %d, want %d (end of Batch A, before corruption)", got, endOfA) + } +} + +func TestFindLastCompleteBatchEnd_PartialBatchOnly(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + // Only First + Middle fragments, no complete batch ever. + appendFragment(t, filePath, RecFirst, []byte("first-data")) + appendFragment(t, filePath, RecMiddle, []byte("middle-data")) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != int64(WalFileHeaderSize) { + t.Errorf("got %d, want %d (no complete batch, stay at header)", got, WalFileHeaderSize) + } +} + +// Oracle BLOCKING test: must continue past padding in a full block to read +// the next block. Original code returned on first padding, truncating all +// later batches. +func TestFindLastCompleteBatchEnd_BlockBoundaryPadding(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + // Batch A: large enough to nearly fill block 1. + // Block 1 layout: [32-byte header][Batch A (Full record)] [padding to end] + // We need: 32 + len(Full record of A) + padding == 32 + WalBlockSize + // Full record = 7 (header) + len(payload). So: + // len(A) such that 7 + len(A) leaves < 7 bytes before block end + // Then writer pads to end of block and Batch B goes in block 2. + + // Available in block 1 after file header = WalBlockSize - 32 = 32736 + // We want Batch A record size to be 32736 - 6 = 32730 (leaving 6 bytes, < 7, padding) + // So payload size = 32730 - 7 = 32723 + // Batch A = batch header (18) + entry bytes. Entry: Put with key + value. + // Simplest: use raw bytes (we're testing physical layout, not batch validity). + bigPayload := make([]byte, 32723) + for i := range bigPayload { + bigPayload[i] = byte('A') + } + recA := EncodePhysicalRecord(RecFull, bigPayload) + appendRawBytes(t, filePath, recA) + + endOfBlock1 := int64(WalFileHeaderSize + WalBlockSize) // 32 + 32768 + currentSize, _ := fileSize(filePath) + // Pad to end of block 1 with zeros. + padLen := int(endOfBlock1 - currentSize) + if padLen > 0 { + appendRawBytes(t, filePath, make([]byte, padLen)) + } + + // Batch B in block 2 (normal small batch after padding boundary). + batchB := []byte("batch-B") + recB := EncodePhysicalRecord(RecFull, batchB) + appendRawBytes(t, filePath, recB) + endOfB, _ := fileSize(filePath) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfB { + t.Errorf("got %d, want %d (end of Batch B in block 2, after padding)", got, endOfB) + } + // Specifically: must NOT be at end of Batch A (would be ~32755, before padding). + if got < endOfBlock1 { + t.Errorf("got %d < %d (returned at end of Batch A, missed block 2)", got, endOfBlock1) + } +} + +func TestFindLastCompleteBatchEnd_NonZeroTailPadding(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + batchA := []byte("batch-A") + appendFullRecord(t, filePath, batchA) + endOfA, _ := fileSize(filePath) + + // Append 3 non-zero bytes (< PhysicalRecordHeaderSize=7). This is + // technically invalid padding (per design: padding must be zeros), but + // findLastCompleteBatchEnd should still return end of last complete + // batch — this is "tail corruption" classification territory. + appendRawBytes(t, filePath, []byte{0xFF, 0xFF, 0xFF}) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfA { + t.Errorf("got %d, want %d (end of Batch A)", got, endOfA) + } +} + +func TestFindLastCompleteBatchEnd_ZeroTailPadding(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + batchA := []byte("batch-A") + appendFullRecord(t, filePath, batchA) + endOfA, _ := fileSize(filePath) + + // Append 3 zero bytes (valid tail padding in a short final block). + appendRawBytes(t, filePath, []byte{0, 0, 0}) + + got, err := findLastCompleteBatchEnd(filePath) + if err != nil { + t.Fatalf("findLastCompleteBatchEnd: %v", err) + } + if got != endOfA { + t.Errorf("got %d, want %d (end of Batch A)", got, endOfA) + } +} + +func fileSize(filePath string) (int64, error) { + fi, err := os.Stat(filePath) + if err != nil { + return 0, err + } + return fi.Size(), nil +} + +// -------- truncateAndPersist tests -------- + +func TestTruncateAndPersist_Success(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + appendRawBytes(t, filePath, []byte("batch-A")) + appendRawBytes(t, filePath, []byte("extra-bytes-to-be-truncated")) + + endOfA, _ := fileSize(filePath) + truncateOffset := int64(WalFileHeaderSize) + 7 // just past header, before "batch-A" + + if err := truncateAndPersist(filePath, truncateOffset, dir, nil); err != nil { + t.Fatalf("truncateAndPersist: %v", err) + } + + gotSize, _ := fileSize(filePath) + if gotSize != truncateOffset { + t.Errorf("file size = %d, want %d (truncated)", gotSize, truncateOffset) + } + _ = endOfA // not used; verify only that size matches truncate point +} + +func TestTruncateAndPersist_FtruncateFailure(t *testing.T) { + dir := t.TempDir() + nonExistent := filepath.Join(dir, "no-such-file.wal") + + err := truncateAndPersist(nonExistent, 100, dir, nil) + if err == nil { + t.Fatal("expected error on non-existent file") + } + if !strings.Contains(err.Error(), "ftruncate") { + t.Errorf("error should mention 'ftruncate', got: %v", err) + } +} + +func TestTruncateAndPersist_DirFsyncFailure(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + appendRawBytes(t, filePath, []byte("extra")) + + orig := dirFsyncFn + dirFsyncFn = func(string) error { return errors.New("simulated dir fsync failure") } + t.Cleanup(func() { dirFsyncFn = orig }) + + err := truncateAndPersist(filePath, int64(WalFileHeaderSize), dir, nil) + if err == nil { + t.Fatal("expected error on dir fsync failure") + } + if !strings.Contains(err.Error(), "fsync WAL dir") { + t.Errorf("error should mention 'fsync WAL dir', got: %v", err) + } +} + +func TestTruncateAndPersist_SegmentFsyncFailure(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + appendRawBytes(t, filePath, []byte("extra")) + + orig := segmentFsyncFn + segmentFsyncFn = func(string) error { return errors.New("simulated segment fsync failure") } + t.Cleanup(func() { segmentFsyncFn = orig }) + + err := truncateAndPersist(filePath, int64(WalFileHeaderSize), dir, nil) + if err == nil { + t.Fatal("expected error on segment fsync failure") + } + if !strings.Contains(err.Error(), "fsync truncated segment") { + t.Errorf("error should mention 'fsync truncated segment', got: %v", err) + } +} + +// Oracle nice-to-have: verify retry after dir-fsync failure. The first +// call fails after ftruncate succeeded; the second call (with fsync +// restored) must succeed and the file must end up correctly truncated. +func TestTruncateAndPersist_RetryAfterDirFsyncFailure(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + appendRawBytes(t, filePath, []byte("extra-bytes")) + + truncateOffset := int64(WalFileHeaderSize) + + // First call: inject dir fsync failure. + orig := dirFsyncFn + dirFsyncFn = func(string) error { return errors.New("simulated") } + if err := truncateAndPersist(filePath, truncateOffset, dir, nil); err == nil { + t.Fatal("first call should fail") + } + dirFsyncFn = orig + + // Second call: must succeed (ftruncate is idempotent). + if err := truncateAndPersist(filePath, truncateOffset, dir, nil); err != nil { + t.Fatalf("retry truncateAndPersist: %v", err) + } + + gotSize, _ := fileSize(filePath) + if gotSize != truncateOffset { + t.Errorf("after retry, file size = %d, want %d", gotSize, truncateOffset) + } +} diff --git a/wal/recover_test.go b/wal/recover_test.go index de026e6..1c4005a 100644 --- a/wal/recover_test.go +++ b/wal/recover_test.go @@ -2,9 +2,11 @@ package wal import ( "bytes" + "errors" "os" "path/filepath" "reflect" + "strings" "testing" "github.com/dailz/go-kv/manifest" @@ -333,3 +335,119 @@ func fileExists(t *testing.T, path string) bool { t.Fatalf("stat %s: %v", path, err) return false } + +// Regression guard for C5+H8: end-to-end recovery with partial fragment +// tail must persist truncation at the last COMPLETE batch boundary +// (H8), and the truncation must be persisted with all 4 steps (C5). +// After repair, second recovery must not see corruption. +func TestRecoverPartialFragmentTailIdempotent(t *testing.T) { + dir := t.TempDir() + batchA := []*WalEntry{makePutEntry("key-A", "val-A")} + batchB := []*WalEntry{makePutEntry("key-B", "val-B")} + + filePath := writeTestSegment(t, dir, 0, 0, [][]*WalEntry{batchA, batchB}) + + // Compute exact byte offset where Batch B's Full record ends. + // Layout: [header][Batch A Full record][Batch B Full record][padding to 32KB] + encA, _ := EncodeWalBatch(0, batchA) + encB, _ := EncodeWalBatch(1, batchB) + endOfBatchB := int64(WalFileHeaderSize) + + int64(PhysicalRecordHeaderSize+len(encA)) + + int64(PhysicalRecordHeaderSize+len(encB)) + + fiBefore, _ := os.Stat(filePath) + + // Append First + Middle* (no Last) to simulate partial fragment tail. + appendFileBytes(t, filePath, EncodePhysicalRecord(RecFirst, []byte("first-fragment-payload"))) + appendFileBytes(t, filePath, EncodePhysicalRecord(RecMiddle, []byte("middle-fragment-payload"))) + + replayer1 := &mockReplayer{} + result1, err := Recover(dir, replayer1) + if err != nil { + t.Fatalf("1st Recover: %v", err) + } + if !result1.Truncated { + t.Fatal("1st Recover: Truncated = false, want true") + } + + fiAfter, _ := os.Stat(filePath) + if fiAfter.Size() != endOfBatchB { + t.Errorf("file size after truncation = %d, want %d (end of Batch B, H8)", + fiAfter.Size(), endOfBatchB) + } + if fiAfter.Size() >= fiBefore.Size() { + t.Errorf("file should shrink after truncation: before=%d after=%d", + fiBefore.Size(), fiAfter.Size()) + } + + replayer2 := &mockReplayer{} + result2, err := Recover(dir, replayer2) + if err != nil { + t.Fatalf("2nd Recover: %v", err) + } + if result2.Truncated { + t.Error("2nd Recover: Truncated = true, want false (truncation should be persisted)") + } + if result1.NextSequence != result2.NextSequence { + t.Errorf("NextSequence differs: %d vs %d", result1.NextSequence, result2.NextSequence) + } +} + +// Regression guard for C5: any truncation persist step failure must +// fail Recover, causing DB.Open to fail. No swallowing allowed. +func TestRecoverTruncationFailureFailsRecovery(t *testing.T) { + dir := t.TempDir() + filePath := writeTestSegment(t, dir, 0, 0, [][]*WalEntry{ + {makePutEntry("key-A", "val-A")}, + }) + // Append corruption to trigger tail corruption path. + appendFileBytes(t, filePath, []byte{0xDE, 0xAD, 0xBE, 0xEF}) + + // Inject dir fsync failure (Step 4 of truncateAndPersist). + orig := dirFsyncFn + dirFsyncFn = func(string) error { return errors.New("simulated dir fsync failure") } + 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) + } +} + +// Regression guard for design line 778-781: CRC-valid but batch-content- +// invalid must hard-fail through DecodeWalBatch, NOT enter truncation path. +func TestRecoverInvalidBatchNotTruncatable(t *testing.T) { + dir := t.TempDir() + // Build segment with: physical records CRC-valid, but assembled batch + // has invalid header (entryCount=0). + filePath := filepath.Join(dir, "segment-0.wal") + writeRawSegmentHeader(t, filePath) + + // Construct an "invalid batch": WalBatchHeaderSize=18 bytes, with + // entryCount=0 (invalid per ReplayBatch check at recovery.go). + invalidBatch := make([]byte, WalBatchHeaderSize) + // flags(2) + baseSequence(8) + entryCount(4)=0 + entriesSize(4)=0 + // All zeros, except entryCount=0 is invalid by itself. + // Encode as Full physical record (CRC-valid). + rec := EncodePhysicalRecord(RecFull, invalidBatch) + appendFileBytes(t, filePath, rec) + + _, err := Recover(dir, &mockReplayer{}) + if err == nil { + t.Fatal("expected Recover to fail on invalid batch content") + } + // Should NOT mention truncation — must be a different error path. + if strings.Contains(err.Error(), "truncat") { + t.Errorf("error should not be about truncation; got: %v", err) + } + + // File must NOT have been truncated (size unchanged). + fi, _ := os.Stat(filePath) + if fi.Size() != int64(WalFileHeaderSize)+int64(len(rec)) { + t.Errorf("file was truncated; size = %d, want %d", + fi.Size(), int64(WalFileHeaderSize)+int64(len(rec))) + } +}