etcdserver: Use panic instead of fatal on no space left error #10597
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using the embed package to embed etcd, sometimes the storage prefix being used might be full. In this case, this code path triggers, causing an: `etcdserver: create wal error: no space left on device` error, which causes a fatal. A fatal differs from a panic in that it also calls os.Exit(1). In this situation, the calling program that embeds the etcd server will be abruptly killed, which prevents it from cleaning up safely, and giving a proper error message. Depending on what the calling program is, this can cause corruption and data loss. This patch switches the fatal to a panic. Ideally this would be a regular error which would get propagated upwards to the StartEtcd command, but in the meantime at least this can be caught with recover(). This fixes the most common fatal that I've experienced, but there are surely more that need looking into. If possible, the errors should be threaded down into the code path so that embedding etcd can be more robust. Fixes: etcd-io#10588 This is a cherry-picked version of upstream: 368f70a
gyuho
approved these changes
Mar 29, 2019
|
Thank you so much @gyuho! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using the embed package to embed etcd, sometimes the storage prefix
being used might be full. In this case, this code path triggers, causing
an:
etcdserver: create wal error: no space left on deviceerror, whichcauses a fatal. A fatal differs from a panic in that it also calls
os.Exit(1). In this situation, the calling program that embeds the etcd
server will be abruptly killed, which prevents it from cleaning up
safely, and giving a proper error message. Depending on what the calling
program is, this can cause corruption and data loss.
This patch switches the fatal to a panic. Ideally this would be a
regular error which would get propagated upwards to the StartEtcd
command, but in the meantime at least this can be caught with recover().
This fixes the most common fatal that I've experienced, but there are
surely more that need looking into. If possible, the errors should be
threaded down into the code path so that embedding etcd can be more
robust.
Fixes: #10588
This is a cherry-picked version of upstream: 368f70a
Please read https://github.com/etcd-io/etcd/blob/master/CONTRIBUTING.md#contribution-flow.