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

etcdserver: call the OnPreCommitUnsafe in unsafeCommit #14730

Merged
merged 1 commit into from
Nov 13, 2022

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Nov 11, 2022

Fix the issue discovered in #14685

unsafeCommit is called by both (*batchTxBuffered) commit and (*backend) defrag. When users perform the defragmentation operation, etcd doesn't update the consistent index, because unsafeCommit doesn't call the OnPreCommitUnsafe. If etcd crashes(e.g. panicking) in the process for whatever reason, when etcd starts again it replays the WAL entries starting from the latest snapshot, accordingly it may re-apply entries which might have already been applied, eventually the revision isn't consistent with other members.

Refer to discussion in pull/14685, especially the comment #14685 (comment).

Signed-off-by: Benjamin Wang wachao@vmware.com

cc @mitake @ptabor @serathius @spzala

`unsafeCommit` is called by both `(*batchTxBuffered) commit` and
`(*backend) defrag`. When users perform the defragmentation
operation, etcd doesn't update the consistent index. If etcd
crashes(e.g. panicking) in the process for whatever reason, then
etcd replays the WAL entries starting from the latest snapshot,
accordingly it may re-apply entries which might have already been
applied, eventually the revision isn't consistent with other members.

Refer to discussion in etcd-io#14685

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Nov 11, 2022

It's really interesting and funny the PR number 14730 is very similar to previous important issue 14370

@ahrtr ahrtr added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Nov 11, 2022
@ahrtr
Copy link
Member Author

ahrtr commented Nov 11, 2022

cc @dims and @liggitt for awareness.

@dims
Copy link
Contributor

dims commented Nov 11, 2022

thanks @ahrtr

@serathius serathius merged commit 0e4bf4a into etcd-io:main Nov 13, 2022
@serathius serathius mentioned this pull request Nov 14, 2022
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Development

Successfully merging this pull request may close these issues.

3 participants