Refactor rebuf API, add comprehensive tests, and improve reliability#22
Merged
Refactor rebuf API, add comprehensive tests, and improve reliability#22
Conversation
Phase 1 - Correctness & Reliability: - Fix Write() concurrency: mutex now protects the entire write path - Graceful goroutine shutdown via context cancellation and done channel - Fix Init() error handling: use MkdirAll with error propagation - Add CRC32 checksums per entry for data integrity verification - Fix segment ID recovery bug (off-by-one on reopen) Phase 2 - Testing: - Add 36 unit tests for rebuf package (Write, WriteBatch, Replay, ReplayFrom, Purge, PurgeThrough, Close, concurrency, recovery, etc.) - Add 22 unit tests for utils package (all functions fully covered) - Add race-detector-clean concurrent write tests - Add benchmarks (Write, WriteParallel, Replay, WriteBatch) - Coverage: 80.8% rebuf, 90.8% utils Phase 3 - API Improvements: - Rename Init() to New() with context.Context support - Functional options pattern (WithMaxLogSize, WithMaxSegments, etc.) - Configurable sync strategies (SyncEveryWrite, SyncPeriodic, SyncManual) - Expose read-only state: SegmentCount(), CurrentLogSize(), Dir() - Add Sync() method for manual sync control Phase 4 - Features: - Selective replay via ReplayFrom(segmentId, callback) - Purge() to delete all segments and reset - PurgeThrough(segmentId) for partial cleanup - WriteBatch() for amortized multi-entry writes Phase 5 - Documentation & Project Health: - Godoc comments on all exported types and functions - Updated README with API overview and on-disk format docs - Added CONTRIBUTING.md - CI: added -race flag and go vet step - Fix utils bugs: IsDirectoryEmpty now reads all entries, GetNumSegments/GetOldestSegmentFile use filename-based ID parsing https://claude.ai/code/session_01VSZuYevFutSrFay5XmL1qh
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR significantly improves rebuf's API design, test coverage, and reliability. The main changes include:
API Redesign: Replaced the
Init(RebufOptions)constructor with a modernNew(ctx, dir, opts...)pattern using functional options, following Go conventions.Comprehensive Test Suite: Added 1000+ lines of tests covering initialization, writes, batches, replay, segment rotation, concurrency, sync strategies, and edge cases.
Data Integrity: Implemented CRC32 checksums for every log entry to detect corruption during replay.
Concurrency Fixes: Protected the entire write path with mutex locks (previously only the final sync was protected), fixing race conditions in concurrent scenarios.
Graceful Shutdown: Added context-based cancellation for the background sync goroutine to prevent leaks.
Key Changes
rebuf/rebuf.go:
RebufOptionsstruct with functionalOptionpattern (WithMaxLogSize,WithMaxSegments, etc.)Init()toNew()with signatureNew(ctx context.Context, logDir string, opts ...Option)SyncStrategyenum (SyncEveryWrite, SyncPeriodic, SyncManual) for flexible durability control[8-byte size][4-byte crc32][data]Write()andWriteBatch()methods now protected by mutexSync()method for manual sync strategycontext.Contextand done channelSegmentCount(),CurrentLogSize(),Dir()ReplayFrom(segmentId)for selective replay starting from a specific segmentrebuf/rebuf_test.go (new file):
utils/utils.go:
ParseSegmentId()to extract numeric ID from filenamesGetSortedSegmentFiles()to return segments sorted by ID with .tmp appendedGetLatestSegmentId()to use new helpersIsDirectoryEmpty()logicutils/utils_test.go:
IsDirectoryEmpty,ParseSegmentId,GetSortedSegmentFiles,GetLatestSegmentId,GetNumSegments,GetOldestSegmentFile,FileSizeDocumentation:
CI/CD:
-raceflag to test runs in GitHub Actions workflowgo vetstepNotable Implementation Details
[8-byte big-endian size][4-byte big-endian CRC32][data]for integrity checkinghttps://claude.ai/code/session_01VSZuYevFutSrFay5XmL1qh