New WAL implementation with configurable variables #1356
Conversation
|
@arpitbbhayani @JyotinderSingh @gauravsarma1992 @psrvere please review |
|
Adding a comment here for context. We have discussed changes in the community call, please re-request a review once they are done. |
There was a problem hiding this comment.
Thanks for the changes so far @ayushsatyam146. I have left a few more comments.
config/config.go
Outdated
| network.io_buffer_length_max = 51200 | ||
|
|
||
| # WAL Configuration | ||
| LogDir = "tmp/deicdeb-wal-lt" |
There was a problem hiding this comment.
Would suggest having a parent directory for storing logs and persistent data similar to Redis, PostgreSQL etc. which typically define parent directory paths to store logs/data related to app.
Have raised PR for the same here: #1359
| WalMode = "buffered" | ||
| WriteMode = "default" | ||
| BufferSizeMB = 1 | ||
| RotationMode = "segemnt-size" |
config/config.go
Outdated
| WalMode string `config:"mode" default:"buffered" validate:"oneof=buffered unbuffered"` | ||
| WriteMode string `config:"write_mode" default:"default" validate:"oneof=default fsync"` | ||
| BufferSizeMB int `config:"buffer_size" default:"1" validate:"min=1"` | ||
| RotationMode string `config:"rotation_mode" default:"segemnt-size" validate:"oneof=segment-size time"` |
| err := wal.rotateLog() | ||
| wal.lock.Unlock() | ||
| if err != nil { | ||
| log.Printf("Error while performing sync: %v", err) |
There was a problem hiding this comment.
Shall we keep error statements different from L289?
|
@JyotinderSingh @lucifercr07 I have included most of the suggested changes now. I have tried to keep the implementation as close to the design doc as possible. Please let me know if there are other things you would like me to change. Thanks! |
ayushsatyam146
force-pushed
the
wal-implementation
branch
from
feec987 to
42a459c
Compare
ayushsatyam146
force-pushed
the
wal-implementation
branch
from
42a459c to
0d30b68
Compare
|
Hi @JyotinderSingh @arpitbbhayani can you please reveiw the PR, Thanks! |
There was a problem hiding this comment.
Thanks for working on this large contribution and addressing all the reviews @ayushsatyam146! This PR lays a strong foundation for the WAL implementation.
LGTM.
Fixes #1285
It is a new WAL implementation based on the decided configurable values. This doesn't contain support for checkpointing and recovery yet. Appropriate TODO markers are there for checkpointing and recovery.
Previous iteration of basic Benchmark
New interaction of basic benchmark