Don't open a raft snapshot file until we have a successful snapshot response.#9894
Conversation
| }, | ||
| } | ||
| defer snapFile.Close() | ||
| defer w.Close() |
There was a problem hiding this comment.
I know this was in the original code, but isn't this a case where we really should be checking the result of the Close call?
There was a problem hiding this comment.
Sure? It makes the code a little uglier but what the heck.
|
|
||
| type lazyOpenWriter struct { | ||
| openFunc func() (io.WriteCloser, error) | ||
| writer io.WriteCloser |
There was a problem hiding this comment.
It makes me a little uncomfortable that this isn't thread/data-race safe, but it looks like it won't break anything in the existing Sys.RaftSnapshot implementation.
There was a problem hiding this comment.
Yeah, my thought was this local, unexportable writer is only used in the one case. If it were reusable I'd stick a mutex in it.
ncabatoff
left a comment
ncabatoff
left a comment
There was a problem hiding this comment.
Approved, but note Brian's new comment in the jira, which may require more work.
|
Looks good! |
…esponse. (#9894) * Don't open the snapshot file until we have a successful response * Check the success of Close if nothing else errors
2 participants
Implement a lazy writer which only opens the file once data starts being
written to it, e.g. the snapshot request has succeeded. This will prevent
opening 0 byte files or clobbering existing files on error.