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.

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

kvserver: flush WAL on writing storage checkpoints #89369

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

pav-kv
Copy link
Collaborator

@pav-kv pav-kv commented Oct 5, 2022

If a consistency check fails, it saves checkpoints on all replicas, to help with further investigation. Flushing WAL was disabled, so some checkpoints could be slightly out of date. This commit fixes that.

Fixes #89287

Release justification: important bug fix
Release note (bug fix): flush WAL when writing storage checkpoints on consistency checker failures

@pav-kv pav-kv marked this pull request as ready for review October 5, 2022 09:31
@pav-kv pav-kv requested review from a team as code owners October 5, 2022 09:31
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release note (bug fix): flush WAL on writing storage checkpoints, which makes them correctly represent the state of storage when an inconsistency occurs

This can sound a lot more dramatic than it really is, since users don't know what a storage checkpoint is or when they occur. We should be extra clear here that this is only relevant for consistency checker failures, and does not affect database durability guarantees at all. Add e.g. "flush WAL when writing storage checkpoints on consistency checker failures".

pkg/kv/kvserver/replica_consistency.go Outdated Show resolved Hide resolved
@pav-kv
Copy link
Collaborator Author

pav-kv commented Oct 5, 2022

Addressed comments.

If a consistency check fails, it saves checkpoints on all replicas, to help
with further investigation. Flushing WAL was disabled, so some checkpoints
could be slightly out of date. This commit fixes that.

Release justification: important bug fix
Release note (bug fix): flush WAL when writing storage checkpoints on
consistency checker failures
Copy link
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker and @pavelkalinnikov)

@pav-kv
Copy link
Collaborator Author

pav-kv commented Oct 5, 2022

bors r=erikgrinaker,sumeerbhola

@craig
Copy link
Contributor

craig bot commented Oct 5, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: checkpoint inconsistent with known applied state
4 participants