dailz
0739966e55
Per design §3.2 line 248-272, segment directory fsync is a hard requirement for durable-ready state, not best-effort. rename is atomic in memory but not guaranteed to survive power loss without a directory fsync. The previous code silently swallowed both os.Open(dir) and dirFD.Sync() errors, leaving WAL writer to confirm batches as durable when their segment might not exist after a crash. Failure propagation: - Initial segment creation: NewSegmentWriter fails -> NewSegmentManager fails -> DB.Open fails (user sees error, no data promise violated). - Rotation during AppendBatch: NewSegmentWriter fails -> AppendBatch fails -> WalWriter.stopWithError(ErrCommitUnknown) -> write-stopped (per design line 272). Changes: - wal/segment_writer.go: extract dirFsync helper (Open -> f.Stat -> IsDir -> f.Sync, avoiding TOCTOU window), replace silent swallow with fatal error; on failure clean up resources (fd.Close + os.Remove) and surface cleanup errors via errors.Join so nothing is silently lost. - wal/dir_fsync_test.go (new): unit test the helper with valid dir, non-existent dir (fails at os.Open), and not-a-dir (fails at IsDir). - wal/segment_writer_test.go: add TestNewSegmentWriterDirFsyncFailure (injects failure via package-level dirFsyncFn override; documents the not-parallel-safe constraint), TestNewSegmentWriterNormalPathStillWorks (regression), and TestNewSegmentWriterRetryAfterDirFsyncFailure (verifies cleanup is effective for retry). - wal/segment_manager_test.go: add TestSegmentManagerRotateFailsOnDirFsyncFailure (fills segment until rotation triggers, injects failure, verifies propagation through AppendBatch path) and TestNewSegmentManagerFailsOnDirFsyncFailure (covers the DB.Open failure path). dirFsyncFn injection note: tests that override this package-level var must not use t.Parallel(). All existing wal tests run serially within the package; this is the lightest mechanism that doesn't require interface indirection in production code. Verified: each new test fails on pre-fix code (silent swallow returned nil error) and passes after the fix. Full suite green including go test -race ./... . Audit context: docs/audit-3.2.md C6 (Oracle-verified bg_ef425776).
43 lines
1.2 KiB
Go
43 lines
1.2 KiB
Go
package wal
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
)
|
|
|
|
// dirFsyncFn is the package-level indirection over dirFsync so tests can
|
|
// inject failures without interface plumbing in production code.
|
|
//
|
|
// NOT PARALLEL-SAFE: tests that override this must not use t.Parallel().
|
|
// All existing wal tests run serially within the package.
|
|
var dirFsyncFn = dirFsync
|
|
|
|
// dirFsync opens the directory and fsyncs it. Required for durable-ready
|
|
// state per design §3.2 line 258. This is a hard requirement, not
|
|
// best-effort: rename is atomic in memory but not guaranteed to survive
|
|
// power loss without a directory fsync.
|
|
//
|
|
// Order: os.Open → f.Stat → IsDir → f.Sync. The open-then-stat sequence
|
|
// avoids the TOCTOU window between a separate os.Stat and os.Open, and
|
|
// ensures IsDir is checked against the actually-opened file.
|
|
func dirFsync(dir string) error {
|
|
f, err := os.Open(dir)
|
|
if err != nil {
|
|
return fmt.Errorf("open dir %q: %w", dir, err)
|
|
}
|
|
defer f.Close()
|
|
|
|
info, err := f.Stat()
|
|
if err != nil {
|
|
return fmt.Errorf("stat dir %q: %w", dir, err)
|
|
}
|
|
if !info.IsDir() {
|
|
return fmt.Errorf("path %q is not a directory", dir)
|
|
}
|
|
|
|
if err := f.Sync(); err != nil {
|
|
return fmt.Errorf("fsync dir %q: %w", dir, err)
|
|
}
|
|
return nil
|
|
}
|