Atomic writes for persisting service/check state #2480
Conversation
| if err := ioutil.WriteFile(file, buf, 0600); err != nil { | ||
| return fmt.Errorf("failed writing file %q: %s", file, err) | ||
|
|
||
| // Create temp file in same dir, to make more likely atomic |
There was a problem hiding this comment.
Let's add a comment here about why we aren't calling writeFileAtomic().
| if err != nil { | ||
| return err | ||
| } | ||
| if _, err := fh.Write(contents); err != nil { |
There was a problem hiding this comment.
Super late on this minor comment but better late than never: Shouldn't we defer the fh.Close() right after we open successfully? We otherwise have an exit path where it remains open. We can still close before we rename too.
There was a problem hiding this comment.
Oh that's a good point. We should probably even do a best-effort delete on the temp file if something goes wrong (do a deferred fn that we disarm at the end). We originally used a file name that was 1:1 with the service name but changed that to fix a race condition.
|
just as an fyi. possibly related to this change, recently I just got the name seems extra long. |
Modified #2240 to call file.Sync() when persisting services/checks. We don't want to do it for check state though because that happens a lot more frequently, so instead just handle the case where we load a malformed file.
Fixes #1221