Implement WriteAheadLog #640
Merged
Conversation
This file contains 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
|
@masih @Stebalien I would appriciate a light review of the WAL itself, integration is incoming after I fix some issues in it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
=======================================
Coverage ? 79.25%
=======================================
Files ? 54
Lines ? 4878
Branches ? 0
=======================================
Hits ? 3866
Misses ? 635
Partials ? 377
|
masih
reviewed
Sep 12, 2024
Stebalien
reviewed
Sep 13, 2024
Stebalien
reviewed
Sep 13, 2024
Stebalien
reviewed
Sep 13, 2024
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Co-authored-by: Masih H. Derkani <m@derkani.org> Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
masih
approved these changes
Sep 18, 2024
internal/writeaheadlog/wal.go
Outdated
|
|
||
| err := wal.hydrate() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reading the WAL: %v", err) |
There was a problem hiding this comment.
Might as well wrap error?
Suggested change
| return nil, fmt.Errorf("reading the WAL: %v", err) | |
| return nil, fmt.Errorf("reading the WAL: %w", err) |
internal/writeaheadlog/wal.go
Outdated
| } | ||
|
|
||
| type logStat struct { | ||
| logname string |
There was a problem hiding this comment.
Either name or logName?
Suggested change
| logname string | |
| logName string |
internal/writeaheadlog/wal.go
Outdated
| return nil | ||
| } | ||
|
|
||
| const walExtension = ".wal.cbor" |
There was a problem hiding this comment.
Subjective: Would be easier to read if all constants are grouped at the top.
internal/writeaheadlog/wal.go
Outdated
| return nil | ||
| } | ||
|
|
||
| const roatateAt = 1 << 20 // 1MiB |
There was a problem hiding this comment.
Suggested change
| const roatateAt = 1 << 20 // 1MiB | |
| const rotateAt = 1 << 20 // 1MiB |
internal/writeaheadlog/wal.go
Outdated
| } | ||
|
|
||
| // Purge removes files containing entries only older than keepEpoch | ||
| // It is not guarnateed to remove all entries older than keepEpoch as it will keep the current |
There was a problem hiding this comment.
Suggested change
| // It is not guarnateed to remove all entries older than keepEpoch as it will keep the current | |
| // It is not guaranteed to remove all entries older than keepEpoch as it will keep the current |
internal/writeaheadlog/wal.go
Outdated
|
|
||
| switch { | ||
| case err == nil: | ||
| // the EOF case is expected, this is slightly unexpected by still handle it |
There was a problem hiding this comment.
Suggested change
| // the EOF case is expected, this is slightly unexpected by still handle it | |
| // the EOF case is expected, this is slightly unexpected but still handle it |
internal/writeaheadlog/wal.go
Outdated
| func (wal *WriteAheadLog[T, PT]) readLogFile(logname string, keepVaues bool) (logStat, []T, error) { | ||
| var res logStat | ||
| if len(logname) == 0 { | ||
| return res, nil, fmt.Errorf("empty logname") |
There was a problem hiding this comment.
Suggested change
| return res, nil, fmt.Errorf("empty logname") | |
| return res, nil, errors.New("empty logname") |
| @@ -0,0 +1,177 @@ | |||
| package writeaheadlog | |||
There was a problem hiding this comment.
Would be sweet to have a concurrency stress test.
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
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.
Pushing the first commit with complete implementation of WAL for peliminary review.
Integration in progress.