Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Poison the database by default if snapshotting fails #160

Merged
merged 2 commits into from
May 30, 2021
Merged

Poison the database by default if snapshotting fails #160

merged 2 commits into from
May 30, 2021

Conversation

ohsayan
Copy link
Member

@ohsayan ohsayan commented May 30, 2021

No description provided.

@ohsayan
Stop accepting writes if snapshotting fails
55c1882 
This is an important consideration: if BGSAVE fails and poisons the
database, snapshotting can and should too. But this is debatable in some
parts. For example, users may configure snapshots to be on a network
file system (symlinked maybe) and this can fail.
Now in some cases, this failure 'may be acceptable'. This commit adds a
way to customize this behavior through the `failsafe` key in the
snapshots section of the cfg file and through the --stop-write-on-fail
option passed to `skyd` on startup. However, BGSAVE remains unchanged:
it will always poison the database if it fails. If the user doesn't want
this, they can simply disable BGSAVE.
@ohsayan
Add changelog
e6c85a3
@ohsayan ohsayan added C-enhancement New feature or request required-improvement This issue or PR addresses an already existing implementation that needs to be revised D-server Related to the server C-storage Relating to storage A-independent Architecture independent issue/PR C-reliability This issue/PR relates to reliability labels May 30, 2021
@ohsayan
Copy link
Member Author

ohsayan commented May 30, 2021

No tests required here. @stellaroid r=me

@ohsayan ohsayan merged commit 952c5ca into next May 30, 2021
@ohsayan ohsayan deleted the poison-on-snap branch May 30, 2021 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glydr glydr glydr approved these changes

Assignees
No one assigned
Labels
A-independent Architecture independent issue/PR C-enhancement New feature or request C-reliability This issue/PR relates to reliability C-storage Relating to storage D-server Related to the server required-improvement This issue or PR addresses an already existing implementation that needs to be revised
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ohsayan @glydr