-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add option to flush WAL on shutdown #25444
Conversation
Add `--storage-wal-flush-on-shutdown` to flush WAL on database shutdown. On successful shutdown, all WAL data will be committed to TSM files and the WAL directories will not contain any .wal files. Closes: #25422
Remove FlushWAL from tsdb.Engine interface make it a private method on tsm1.Engine. Add comment explaining why the directory layout for Store objects in tests was changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM: this is really nice code.
t.Errorf("unexpected tsm-madv-willneed:\n\nexp=%v\n\ngot=%v\n\n", exp, got) | ||
} | ||
`, &c) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice simplification
} | ||
return err | ||
return errors.Join(flushErr, setCloseErr, storeCloseErr, walCloseErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
Add
--storage-wal-flush-on-shutdown
to flush WAL on database shutdown. On successful shutdown, all WAL data will be committed to TSM files and the WAL directories will not contain any .wal files.Closes: #25422