-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
raft: Don't panic on commit index regression #10166
Comments
/cc @jpbetz |
how about adding a corrupted hint in ready or something like that? so the upper layer can do something when raft thinks itself is in a bad state? |
Maybe (that would be an alternative to the panics we have all over the place now), but I think it might be difficult to use correctly in this case. Here we know we're missing information but we can recover if we get the right log entries. "Corruption" in the general case is difficult (if not impossible) to handle without just crashing and giving up. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions. |
Follow up from the community meeting to revive this conversation. @serathius @lavacat Another use case not to panic right away:
To support this, a few more things may need to be changed. etcd db snapshot as backup right now is only for creating a fresh new cluster with that data as disaster recovery mechanism. |
For ref, here is original commit that added |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions. |
@tbg Is this issue still valid? Please feel free to raise an issue in the new raft repo if needed. |
I raised it here again: etcd-io/raft#18 |
We occasionally see issues in production in which writes to our raft log are not fully persisted before MsgAppResps are sent (Sometimes this is deliberate, as when users disable fsync for performance. Sometimes it's a bug in some layer or another). The raft package does not handle this gracefully: if a follower has sent an MsgAppResp for a given index, the leader will send it that index as committed in future MsgApps. If the raft log was not written successfully, this will cause the follower to crash with
panic: tocommit(265625) is out of range [lastIndex(265624)]. Was the raft log corrupted, truncated, or lost?
.The message is correct because the log was truncated, and this behavior could result in a loss of committed writes if it happened on multiple nodes. However, this is a very severe failure mode. The follower panics, and will repeatedly panic until its disk is wiped and it is started from scratch. In most cases a less extreme recovery path is possible. If the leader still has the log entries in question, it can simply re-send them, and if it does not, it can send a snapshot. I'd like to at least have the option to disable this panic and recover a node in-place when it reaches this state.
I haven't thought this all the way through, but I think the solution would involve synthesizing an MsgAppResp with Reject: true and an appropriate IndexHint when the panic would be triggered.
The text was updated successfully, but these errors were encountered: