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
总是对 segments[len(segments)-1] 调用截断,但 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. Uses %v (NOT %w) so IsTailCorruption returns false
    — otherwise errors.As would still find underlying TailCorruptionError
    through the %w chain and Recover would treat it as truncatable.
  - 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 direct
  ReplaySegmentFile calls updated to pass isLastSegment=true.
- wal/recover_test.go: 4 integration tests covering middle-segment
  CollectingFragments corruption (must hard-fail), middle-segment CRC
  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).
This commit is contained in:
dailz
2026-06-16 09:12:24 +08:00
parent 273229ac9b
commit 3d2d0ea025
6 changed files with 836 additions and 17 deletions
@@ -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_22edec11NICE-TO-HAVE**:原计划只覆盖 middle-segment CollectingFragments,没覆盖 middle-segment parser corruptionCRC 失败)。需要加一个。
```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 DCommit 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 errorTailCorruption from parser)转硬错误
- [ ] Phase A.3`RecoverFromSegments` 传递正确 `isLastSegment`
- [ ] Phase A.4`Recover``tce.SegmentPath` 做截断目标
- [ ] Phase B.14 个 `ReplaySegmentFile` 单元测试存在
- [ ] Phase B.23 个集成测试存在(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 ./...` 无新增警告
- [ ] 单次 commitmessage 引用 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.0Momus 审核 bg_423473e7**[OKAY],无 blocking。思考过程敏锐发现 `TestRecoverMiddleSegmentCorruptionHardFails` 中 segment-2 的 startSequence=6 与 segment-1 的实际 complete batch 数不匹配(应该是 4)。修正为 4
- **v1.1Oracle 修订 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 移到非尾段硬错误转换之前,让两条路径都能设置字段(虽然非尾段硬错误不需要,但保持顺序一致)
+7
View File
@@ -10,12 +10,19 @@ import (
// TailCorruptionError indicates that the WAL tail contains corrupt data // TailCorruptionError indicates that the WAL tail contains corrupt data
// (bad CRC, unexpected non-zero padding bytes, etc.). Recovery may safely // (bad CRC, unexpected non-zero padding bytes, etc.). Recovery may safely
// truncate at the last valid record. // 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 { type TailCorruptionError struct {
Offset int Offset int
SegmentPath string
Err error Err error
} }
func (e *TailCorruptionError) Error() string { 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) return fmt.Sprintf("wal: tail corruption at offset %d: %v", e.Offset, e.Err)
} }
+15 -6
View File
@@ -1,6 +1,7 @@
package wal package wal
import ( import (
"errors"
"fmt" "fmt"
"os" "os"
@@ -59,19 +60,27 @@ func Recover(dir string, replayer BatchReplayer) (*RecoveryResult, error) {
} }
if len(segments) > 0 { if len(segments) > 0 {
lastSeg := segments[len(segments)-1] // After C4 fix, TailCorruptionError is only returned for the last
lastCompleteBatchEnd, findErr := findLastCompleteBatchEnd(lastSeg.FilePath) // 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 { if findErr != nil {
return nil, fmt.Errorf("wal: recover: find truncation offset: %w", findErr) return nil, fmt.Errorf("wal: recover: find truncation offset: %w", findErr)
} }
// Phase 1: truncated segment is always segments[last], no // Phase 1: truncated segment is always segments[last], no
// trailing empty segments to clean up. C4 fix will need to // trailing empty segments to clean up. C4 fix means non-last
// identify trailing empties based on the actually-corrupted // segment corruption hard-fails above, so we never reach here
// segment's index, which is not the same as segments[last]. // with a non-last corrupted segment.
var emptyTrailing []string 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. // Per design §3.2 line 799: DB must NOT enter writable state.
return nil, fmt.Errorf("wal: recover: persist tail truncation: %w", err) return nil, fmt.Errorf("wal: recover: persist tail truncation: %w", err)
} }
+150 -1
View File
@@ -118,7 +118,7 @@ func TestRecoverWithTailCorruption(t *testing.T) {
// Verify the truncated file still parses cleanly. // Verify the truncated file still parses cleanly.
replayer2 := &mockReplayer{} replayer2 := &mockReplayer{}
_, parseErr := ReplaySegmentFile(filePath, 100, replayer2) _, parseErr := ReplaySegmentFile(filePath, 100, true, replayer2)
if parseErr != nil { if parseErr != nil {
t.Fatalf("replay after truncation: %v", parseErr) t.Fatalf("replay after truncation: %v", parseErr)
} }
@@ -451,3 +451,152 @@ func TestRecoverInvalidBatchNotTruncatable(t *testing.T) {
fi.Size(), int64(WalFileHeaderSize)+int64(len(rec))) 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)
}
}
+32 -3
View File
@@ -100,13 +100,35 @@ func ReplayBatch(batch *WalBatch, expectedSequence uint64, replayer BatchReplaye
} }
// ReplaySegmentFile replays all complete WAL batches from one segment file. // 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 nextSequence = startSequence
records, parseErr := ParseRecordsFromFile(filePath) records, parseErr := ParseRecordsFromFile(filePath)
if parseErr != nil && !IsTailCorruption(parseErr) { if parseErr != nil && !IsTailCorruption(parseErr) {
return nextSequence, fmt.Errorf("wal: parse segment records: %w", 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() collector := NewFragmentCollector()
for _, record := range records { for _, record := range records {
if err := collector.Append(record.Type, record.Payload); err != nil { if err := collector.Append(record.Type, record.Payload); err != nil {
@@ -131,11 +153,17 @@ func ReplaySegmentFile(filePath string, startSequence uint64, replayer BatchRepl
return nextSequence, parseErr return nextSequence, parseErr
} }
if collector.State() == FragmentCollecting { if collector.State() == FragmentCollecting {
if isLastSegment {
return nextSequence, &TailCorruptionError{ return nextSequence, &TailCorruptionError{
Offset: 0, Offset: 0,
SegmentPath: filePath,
Err: errors.New("incomplete fragmented batch at segment tail"), 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 return nextSequence, nil
} }
@@ -151,12 +179,13 @@ func RecoverFromSegments(dir string, recoverySegmentID uint64, replayer BatchRep
} }
nextSequence = segments[0].StartSequence nextSequence = segments[0].StartSequence
for _, segment := range segments { for i, segment := range segments {
if segment.StartSequence != nextSequence { if segment.StartSequence != nextSequence {
return nextSequence, fmt.Errorf("wal: segment start sequence %d does not match expected sequence %d", 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 err != nil {
if IsTailCorruption(err) { if IsTailCorruption(err) {
return nextSequence, err return nextSequence, err
+88 -2
View File
@@ -4,6 +4,7 @@ import (
"errors" "errors"
"math" "math"
"os" "os"
"path/filepath"
"reflect" "reflect"
"strings" "strings"
"testing" "testing"
@@ -121,7 +122,7 @@ func TestReplaySegmentFileFull(t *testing.T) {
}) })
replayer := &mockReplayer{} replayer := &mockReplayer{}
next, err := ReplaySegmentFile(filePath, 50, replayer) next, err := ReplaySegmentFile(filePath, 50, true, replayer)
if err != nil { if err != nil {
t.Fatalf("ReplaySegmentFile: %v", err) t.Fatalf("ReplaySegmentFile: %v", err)
} }
@@ -147,7 +148,7 @@ func TestReplaySegmentFileTailCorruption(t *testing.T) {
appendFileBytes(t, filePath, []byte{0x01, 0x02, 0x03}) appendFileBytes(t, filePath, []byte{0x01, 0x02, 0x03})
replayer := &mockReplayer{} replayer := &mockReplayer{}
next, err := ReplaySegmentFile(filePath, 70, replayer) next, err := ReplaySegmentFile(filePath, 70, true, replayer)
if err == nil { if err == nil {
t.Fatal("ReplaySegmentFile succeeded, want tail corruption error") 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) 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)
}
}