fix: address Final Verification Wave findings

- Sync() now flushes BlockWriter before fd.Sync() for durability
- Pre-validate encoding before sequence allocation (design doc compliance)
- Remove dead _ = rec assignment in recover.go
- Remove unused maxPayload field from SegmentWriter
- Handle Put error in MemTable.Publish with panic on invariant violation
This commit is contained in:
dailz
2026-06-12 14:29:53 +08:00
parent 5905dbc06f
commit b833a21848
5 changed files with 100 additions and 10 deletions
+84 -2
View File
@@ -8,7 +8,7 @@
"plan_name": "phase1-wal", "plan_name": "phase1-wal",
"status": "active", "status": "active",
"started_at": "2026-06-12T05:09:32.588Z", "started_at": "2026-06-12T05:09:32.588Z",
"updated_at": "2026-06-12T06:20:06.478Z", "updated_at": "2026-06-12T06:29:05.569Z",
"session_ids": [ "session_ids": [
"opencode:ses_145c3bae9ffeTB2zbsTym0Cev8" "opencode:ses_145c3bae9ffeTB2zbsTym0Cev8"
], ],
@@ -146,6 +146,47 @@
"status": "completed", "status": "completed",
"ended_at": "2026-06-12T06:20:06.478Z", "ended_at": "2026-06-12T06:20:06.478Z",
"elapsed_ms": 85727 "elapsed_ms": 85727
},
"final-wave:f1": {
"task_key": "final-wave:f1",
"task_label": "F1",
"task_title": "**Plan Compliance Audit** — `oracle`",
"session_id": "opencode:ses_1457cb69affe9rYiUhKWDB2kDd",
"agent": "Sisyphus-Junior",
"category": "unspecified-high",
"started_at": "2026-06-12T06:23:49.269Z",
"status": "running",
"updated_at": "2026-06-12T06:29:05.572Z"
},
"final-wave:f3": {
"task_key": "final-wave:f3",
"task_label": "F3",
"task_title": "Real Manual QA — execute all QA scenarios from the plan and verify cross-module integration.",
"session_id": "opencode:ses_1458206c0ffeyx0AQoGlHap1Wb",
"agent": "oracle",
"updated_at": "2026-06-12T06:23:50.560Z",
"started_at": "2026-06-12T06:23:50.560Z",
"status": "running"
},
"final-wave:f2": {
"task_key": "final-wave:f2",
"task_label": "F2",
"task_title": "Code Quality Review — run static analysis and review all .go files for quality issues.",
"session_id": "opencode:ses_145823a08ffeN5NdqAbplhAFfu",
"agent": "oracle",
"updated_at": "2026-06-12T06:23:51.328Z",
"started_at": "2026-06-12T06:23:51.328Z",
"status": "running"
},
"final-wave:f4": {
"task_key": "final-wave:f4",
"task_label": "F4",
"task_title": "Scope Fidelity Check — verify each task's implementation matches its spec with no scope creep.",
"session_id": "opencode:ses_14581c42cffeKPlQGJV5yvUNk5",
"agent": "oracle",
"updated_at": "2026-06-12T06:24:29.653Z",
"started_at": "2026-06-12T06:24:29.653Z",
"status": "running"
} }
} }
} }
@@ -153,7 +194,7 @@
"active_plan": "/home/dailz/workspace/src/go-kv/.omo/plans/phase1-wal.md", "active_plan": "/home/dailz/workspace/src/go-kv/.omo/plans/phase1-wal.md",
"started_at": "2026-06-12T05:09:32.588Z", "started_at": "2026-06-12T05:09:32.588Z",
"status": "active", "status": "active",
"updated_at": "2026-06-12T06:20:06.478Z", "updated_at": "2026-06-12T06:29:05.569Z",
"session_ids": [ "session_ids": [
"opencode:ses_145c3bae9ffeTB2zbsTym0Cev8" "opencode:ses_145c3bae9ffeTB2zbsTym0Cev8"
], ],
@@ -291,6 +332,47 @@
"status": "completed", "status": "completed",
"ended_at": "2026-06-12T06:20:06.478Z", "ended_at": "2026-06-12T06:20:06.478Z",
"elapsed_ms": 85727 "elapsed_ms": 85727
},
"final-wave:f1": {
"task_key": "final-wave:f1",
"task_label": "F1",
"task_title": "**Plan Compliance Audit** — `oracle`",
"session_id": "opencode:ses_1457cb69affe9rYiUhKWDB2kDd",
"agent": "Sisyphus-Junior",
"category": "unspecified-high",
"started_at": "2026-06-12T06:23:49.269Z",
"status": "running",
"updated_at": "2026-06-12T06:29:05.572Z"
},
"final-wave:f3": {
"task_key": "final-wave:f3",
"task_label": "F3",
"task_title": "Real Manual QA — execute all QA scenarios from the plan and verify cross-module integration.",
"session_id": "opencode:ses_1458206c0ffeyx0AQoGlHap1Wb",
"agent": "oracle",
"updated_at": "2026-06-12T06:23:50.560Z",
"started_at": "2026-06-12T06:23:50.560Z",
"status": "running"
},
"final-wave:f2": {
"task_key": "final-wave:f2",
"task_label": "F2",
"task_title": "Code Quality Review — run static analysis and review all .go files for quality issues.",
"session_id": "opencode:ses_145823a08ffeN5NdqAbplhAFfu",
"agent": "oracle",
"updated_at": "2026-06-12T06:23:51.328Z",
"started_at": "2026-06-12T06:23:51.328Z",
"status": "running"
},
"final-wave:f4": {
"task_key": "final-wave:f4",
"task_label": "F4",
"task_title": "Scope Fidelity Check — verify each task's implementation matches its spec with no scope creep.",
"session_id": "opencode:ses_14581c42cffeKPlQGJV5yvUNk5",
"agent": "oracle",
"updated_at": "2026-06-12T06:24:29.653Z",
"started_at": "2026-06-12T06:24:29.653Z",
"status": "running"
} }
}, },
"agent": "atlas" "agent": "atlas"
+6 -1
View File
@@ -2,6 +2,7 @@ package memtable
import ( import (
"errors" "errors"
"fmt"
"sync" "sync"
"sync/atomic" "sync/atomic"
) )
@@ -114,7 +115,11 @@ func (mt *MemTable) Publish(upToSequence uint64) {
// Re-put each entry as published. Each call acquires the skiplist mutex. // Re-put each entry as published. Each call acquires the skiplist mutex.
for _, e := range toPublish { for _, e := range toPublish {
_ = mt.skiplist.Put(e.key, e.value, e.sequence, false) if err := mt.skiplist.Put(e.key, e.value, e.sequence, false); err != nil {
// Reserve guaranteed space, so this should never happen.
// If it does, it's an internal invariant violation.
panic(fmt.Sprintf("memtable: Put failed after Reserve: %v", err))
}
} }
// Update high-water mark after all entries are visible. // Update high-water mark after all entries are visible.
+1 -3
View File
@@ -183,7 +183,7 @@ func findValidOffset(filePath string) (int64, error) {
break break
} }
rec, consumed, err := DecodePhysicalRecord(blockData[pos:]) _, consumed, err := DecodePhysicalRecord(blockData[pos:])
if err != nil { if err != nil {
// Corruption starts here — offset is up to last valid record. // Corruption starts here — offset is up to last valid record.
validOffset = blockStartOffset + int64(pos) validOffset = blockStartOffset + int64(pos)
@@ -192,8 +192,6 @@ func findValidOffset(filePath string) (int64, error) {
// Valid record found. // Valid record found.
validOffset = blockStartOffset + int64(pos+consumed) validOffset = blockStartOffset + int64(pos+consumed)
// Keep the rec reference alive so the compiler doesn't optimize it away.
_ = rec
pos += consumed pos += consumed
} }
+3 -4
View File
@@ -19,7 +19,6 @@ type SegmentWriter struct {
startSequence uint64 startSequence uint64
blockWriter *BlockWriter blockWriter *BlockWriter
currentOffset uint64 // total bytes written (starts at WalFileHeaderSize) currentOffset uint64 // total bytes written (starts at WalFileHeaderSize)
maxPayload uint64 // cfg.MaxSegmentSize - WalFileHeaderSize
} }
// NewSegmentWriter creates a new WAL segment file and writes the file header. // NewSegmentWriter creates a new WAL segment file and writes the file header.
@@ -89,8 +88,6 @@ func NewSegmentWriter(
dirFD.Close() dirFD.Close()
} }
maxPayload := cfg.MaxSegmentSize - WalFileHeaderSize
return &SegmentWriter{ return &SegmentWriter{
fd: fd, fd: fd,
dir: dir, dir: dir,
@@ -99,7 +96,6 @@ func NewSegmentWriter(
startSequence: startSequence, startSequence: startSequence,
blockWriter: NewBlockWriter(), blockWriter: NewBlockWriter(),
currentOffset: WalFileHeaderSize, currentOffset: WalFileHeaderSize,
maxPayload: maxPayload,
}, nil }, nil
} }
@@ -133,6 +129,9 @@ func (sw *SegmentWriter) AppendBatch(encodedBatch []byte) error {
// Sync flushes the segment file to durable storage. // Sync flushes the segment file to durable storage.
func (sw *SegmentWriter) Sync() error { func (sw *SegmentWriter) Sync() error {
if err := sw.blockWriter.Flush(sw.fd); err != nil {
return fmt.Errorf("wal: flushing partial block before sync: %w", err)
}
return sw.fd.Sync() return sw.fd.Sync()
} }
+6
View File
@@ -145,6 +145,12 @@ func (ww *WalWriter) processBatch(requests []*CommitRequest) {
return return
} }
// Pre-validate encoding can succeed before sequence allocation.
if _, err := EncodeWalBatch(0, entries); err != nil {
ww.sendError(requests, err)
return
}
baseSequence, err := ww.seqManager.AllocateBatch(uint32(len(entries))) baseSequence, err := ww.seqManager.AllocateBatch(uint32(len(entries)))
if err != nil { if err != nil {
ww.sendError(requests, err) ww.sendError(requests, err)