proposal: os: add a package to atomically create or replace a file

[stapelberg]

I propose adding a package to atomically create or replace a file to the golang.org/x/ namespace.

Moving the required logic into a Go package slightly reduces code length and duplication across programs, but more importantly documents the intricacies of this pattern in a central place.

An implementation of such a package can be found at https://go-review.googlesource.com/c/exp/+/72379

[rasky]

There are a few implementations in the wild, I think they should be analyzed and mentioned in the proposal.

[ianlancetaylor]

Yes, this seems like something that should start as an external package. https://golang.org/doc/faq#x_in_std

[ianlancetaylor]

But perhaps we could add something to golang.org/x/tools or golang.org/x/sync.

[stapelberg]

I analyzed a number of implementations I could find using GitHub’s search feature:

os.Rename + chmod:

• (now deleted) https://github.com/getflagman/flago/blob/c037674b89a9a1b55566564f91a2f709c8faf768/accounts/keystore/key.go#L189
• https://github.com/kooksee/kchain/blob/f8f42a48e057bac7ac7b2be60376289e7605b055/src/github.com/tendermint/tmlibs/common/os.go#L146
• https://github.com/facebookgo/atomicfile/blob/master/atomicfile.go
  • vendored: https://github.com/textileio/textile-go/blob/129812cb7a211cf8d50ce3c996feba5d88d8cc92/vendor/gx/ipfs/QmdYwCmx8pZRkzdcd8MhmLJqYVoVTC1aGsy5Q4reMGLNLg/atomicfile/atomicfile.go#L14 (formerly listed as a separate implementation)

os.Rename + chmod + chown:

• https://github.com/albertito/chasquid/blob/f3b01cb493ce79e04b2920759de44d6337c0cc23/internal/safeio/safeio.go#L24
• https://github.com/kubernetes-incubator/bootkube/blob/2de93149749b07baa314cba9e3f5d8282d811725/pkg/checkpoint/manifest.go#L84

These implementations unconditionally sync before renaming:

• https://github.com/influxdata/influxdb/blob/7c3ae91d1e1937347b372356e5b970aa6b5514a2/tsdb/shard.go#L1562
• https://github.com/flatcar-linux/docker/blob/867a5999cefccefb3e0fb79ffa22f01dd7c1a5e1/pkg/ioutils/fswriters.go#L13
  • Note that this syncs before Chmod, so the Chmod is not necessarily ordered correctly when the file system journal is replayed after a crash.

This implementation syncs the containing directory to persist the rename:

• https://github.com/monitoring-monkey/thanos/blob/45ef0734bbb730ccc589ab160fe18dcc8b3ab0fb/pkg/block/block.go#L86

These implementations have Windows-specific code:

• ReplaceFile: https://github.com/politician/go-atomic-rename/blob/d089cd53013aceaca483cc35e55f04823cc29ffa/rename_windows.go#L15
• MoveFileExW: https://github.com/ecqsou53/kitcoin/blob/f96f46e08cb459d18861cd91e16fdaa8b8390beb/internal/legacy/rename/rename_windows.go#L38

Action items:

• Verify whether Chown needs a sync call, too (like Chmod).
• Find out which of the two windows syscalls is preferable for replacing os.Rename on Windows.

@rasky Does that satisfy your concern? Did you have any particular implementations in mind that you wanted to include in the analysis?

[rasky]

@stapelberg thanks that's a good start. What I miss from the above comment is why those libraries are not "good enough". You seem to know the problem space well, but are not explaining it. When you say for instance "os.Rename + chmod + chown". I'm not sure if that's correct, or a bug, or just a different way of doing it.

I suggest a structure that you can use to edit your proposal above, to provide more information. To clarify, I'm not able to approve your proposal, I'm just trying to help you providing enough information for a maintainer to make a call on this. This is just a suggestion.

• Problem statement
  Atomically create or update a file is important for [...]. (I think everybody knows that, so just a short concise paragraph).
  A basic implementation is often buggy because [common bugs or side cases not handled] [complex issues about filesystems working differently from system to system]
  Thus, I propose to add a standard implementation somewhere in "x/".

• Existing options
  There are N main libraries with at least X stars on GitHub. They have the following problems:

  • A does this but has this bug
  • B does that but has this other bug
  • ecc.

• Proposal
  I propose to write a library that does this and that, and fixes all the above problems. A prototype is here (link).

[bradfitz]

Sounds like we're still waiting on @stapelberg here.

[stapelberg]

Yes. I’ll follow up with the requested write-up as soon as I find a moment of time for it.

[stapelberg]

Problem statement

Atomically creating or replacing files is a technique used to prevent intermediate (regular operation) or corrupted (when crashing) files from appearing in e.g. a webserver directory.

Side note: while related, the concept of durability is not covered by this proposal at all. In other words, if you would like to ensure your data made it to your disk(s), you’ll need to call Sync() yourself. The proposal only concerns itself with ensuring that what has been written was written atomically (i.e. either the old data is present, or the new data is present, but never intermediate/corrupt data).

A naive approach to the problem is to create a temporary file followed by a call to os.Rename. However, there are a number of subtleties which make the correct sequence of operations hard to identify:

• The temporary file should be removed when an error occurs, but a remove must not be attempted if the rename succeeded, as a new file might have been created with the same name. This renders a throwaway defer os.Remove(t.Name()) insufficient; state must be kept.

• The temporary file must be created on the same file system (same mount point) for the rename to work, but the TMPDIR environment variable should still be respected, e.g. to direct temporary files into a separate directory outside of the webserver’s document root but on the same file system.

• On POSIX operating systems, the fsync system call must be used to ensure that the os.Rename() call will not result in a 0-length file.

Existing options

There are 4 notable libraries on GitHub:

• https://github.com/facebookgo/atomicfile (51 stars) has a decent design, but:
  • Does not default to Sync() after Chmod(), i.e. requires an ordered file system without explicitly stating it.
  • Forcibly does a Chmod() even when not required.
  • Does not respect the TMPDIR environment variable.
  • Does not work on Windows.
• https://github.com/natefinch/atomic (72 stars)
  • Uses the signature WriteFile(string, io.Reader), which makes some use-cases cumbersome or impossible to implement, e.g. streaming data into different parts of the temporary file, or calling Sync() on the file for durability.
  • Forcibly does a chmod, copying the permissions from the file which is being replaced, if any.
  • Does not respect the TMPDIR environment variable.
• https://github.com/dchest/safefile (76 stars)
  • Looks very decent. Only found this in early 2019, well after I published renameio.
  • Does not respect the TMPDIR environment variable.
• https://github.com/google/renameio (273 stars)
  • Disclaimer: I authored this.
  • Seems to get everything right so far.

There are a number of other ad-hoc implementations in applications (as opposed to libraries): #22397 (comment)

Some of these conflate durability with atomicity, get the order of operations wrong, or exhibit issues similar to the ones listed in the 2 libraries above.

Proposal

I propose to add a package to the x/ namespace which offers atomic file creation/replacement, using explicit names in its method signatures (accompanied by detailed doc comments), addressing all the shortcomings listed above.

Find the prototype at https://go-review.googlesource.com/c/exp/+/72379

[rsc]

/cc @aclements @bradfitz @bcmills

[rsc]

See also #17869.

[bcmills]

The temporary file must be created on the same file system (same mount point) for the rename to work, but the TMPDIR environment variable should still be respected

What should happen if TMPDIR is not on the destination file system?

[stapelberg]

What should happen if TMPDIR is not on the destination file system?

An error will occur, and the user must set TMPDIR to point to the destination file system.

I’m not sure whether erroring at rename time (more efficient when things go right) or adding explicit checking (more user-friendly when things go wrong) is better. In case the writes are small, there isn’t much difference, so I’m thinking about cases where writes take a long time.

Maybe we could use sync.Once under the assumption that TMPDIR doesn’t change at runtime to remove performance considerations from an explicit check.

[pam4]

I think that TMPDIR is more of a user/admin preference, and it is not uncommon to have it point to its own filesystem. Maybe we have different use cases in mind, but requiring the program user to change TMPDIR seems odd to me.

As the caller I can think of two cases:

• I want it to just work (use same directory as the destination or TMPDIR with automatic fallback to same directory)
• I know better (pass explicit directory)

In the second case I would prefer to be able to pass it, rather then change TMPDIR from inside the Go program, so that I don't have to worry about the implication of changing the environment (concurrent access, child processes).

A minor note about your prototype: in the commented out example you use defer and log.Fatal, but deferred functions will not run on log.Fatal, you probably intended return err.