Adding atomicwrite in stdlib

pitrou · 2021-11-14T16:56:44.835Z

methane:

Without fsync, we may have broken (often, 0byte) file after OS crash, instead of old or new file.

Does this happen with modern (journalled) filesystems?

pitrou · 2021-11-14T16:57:31.512Z

methane:

So I propose to use “staging”. The term “staging” is famous now thanks to git.

It’s not “famous” at all. Even as a git user, I rarely use or encounter the term “staging”. IMHO “atomic” is the right name for this.

cameron · 2021-11-14T21:25:45.000Z

I also prefer the name “atomic”, in that this provides a flow where the
filename is absent, then atomically present with full contents.

I wrote my own version of just this recently, independently, and named
it @atomic_filename which to my mind better indicates the thing that it
atomic - the presence of the filename on completion.

Cheers,
Cameron Simpson cs@cskk.id.au

cameron · 2021-11-14T21:31:12.000Z

I want to use |.| as the default prefix to hide the file on Unix.

We may be able to use hidden file feature on Windows, but I am not sure that we
can make it unhidden and replace atomictly. Do not having file extension may be
enough to prevent end users open staging file with some app accidentally.

Please be careful with hiding such files. In case of process crashes,
this could easily cause lots of those temporary files to pile up in
arbitrary directories, without the user noticing (except perhaps by
checking the free disk space and wondering where all the space went).

IMO, this should be an option, which is off by default.

I am of the opposite view. I prefer the leading-dot-by-default approach.
As a usability thing, it is nice to walking into a dynamic hot folder
(as a typical use situation for this) and do an “ls” and see complete
things.

There’s plenty of precedence for using a leading dot (or otherwise
marking the file as invisible if not on UNIX); for example rsync uses
exactly this approach for its atomic-update effect: scratch file names
with a dot, renamed onto the final name when complete.

I’m using the same in a work project which makes hot folders: we just
tell the consumer that names commencing with a dot should be ignored,
and anything else short be considered ready for use.

A better approach is to keep the file visible and instead add a
special extension to the name of the temporary file, much like
downloaders often do. In this case, “.staging” would probably be
appropriate.

I’m -1 on this. Almost any extension would be likely to collide with
some legitimate use for such an extension.

This could also be made an option, of course.

It certainly sounds like it should be, since we’re always going to
disagree abount this policy setting.

Cheers,
Cameron Simpson cs@cskk.id.au

EpicWink · 2021-11-14T22:00:33.965Z

What’s the case against @pitrou’s suggestion to put this functionality in shutil? That was the first place I thought of, with higher-level file/directory manipulation.

To me, a transaction in the database sense is a sequence of multiple operations which only are committed when all operations succeed. This proposal would only affect one operation (the file write), and not other Python commands which prevent the transaction from completing.

Atomic is when the operation is instant in the eyes of all observers (other threads/processors). This seems to be the intended functionality, regardless of implementation. It seems like the popular implementation is to perform an atomic rename on a fully constructed file (required on all platforms).

Staging seems to reference the popular implementation (and does make sense in that regard, but not because of Git, rather from dev-ops deployments), but not the abstract functionality.

methane · 2021-11-15T00:25:42.648Z

pitrou:

shutil sounds like the best place. os is too low-level. tempfile isn’t adequate (the fact that atomic writing may use a temporary file is an implementation detail).

Hmm…

BTW, why chdir chose contextlib instead of shutil?

eryksun · 2021-11-15T01:33:53.686Z

In Windows, the file can be created in the target directory (with whatever unique name) using CREATE_NEW disposition, GENERIC_WRITE access (or more) and without write/delete access sharing. The single open can be used to write to the file and rename it. To rename, call SetFileInformationByHandle(handle, FileRenameInfo, ...) with ReplaceIfExists enabled. In Windows 10, there’s also FileRenameInfoEx, which supports the flag combination FILE_RENAME_FLAG_REPLACE_IF_EXISTS | FILE_RENAME_FLAG_POSIX_SEMANTICS. This can replace the target file even if it has existing opens, provided they share delete access (still uncommon, unfortunately).

The created file will have a different create time and file attributes, but that’s not a big deal. What bothers me more is that the created file will have a new security descriptor, so the owner and permissions may change. At a minimum that needs to be documented. We could do more. The original file’s security can be queried via GetNamedSecurityInfoW() and set on the new file via SetSecurityInfo(). For this, the open would have to also include WRITE_OWNER and WRITE_DAC access. Setting the file security might fail. For example, the owner might not be one we’re allowed to set. Also, if the target file has a high-integrity label, our process would need at least that integrity level. If we have administrator access, the file integrity level shouldn’t be a problem, and an administrator can enable SeRestorePrivilege and open with FILE_FLAG_BACKUP_SEMANTICS to set an arbitrary owner.

pitrou · 2021-11-15T09:15:00.623Z

methane:

BTW, why chdir chose contextlib instead of shutil?

I don’t know, but that sounds like a mistake to me. I would never go and look in contextlib for OS-related utilities.

malemburg · 2021-11-15T09:27:01.000Z

AFAIK, the SC chose contextlib.

cameron · 2021-11-15T23:25:22.000Z

That’s not apparent. I think the submitted PR put it in contextlib and
the SC didn’t object.

PR: bpo-25625: add contextlib.chdir by FFY00 · Pull Request #28271 · python/cpython · GitHub

Personally I think both the chdir() context manager and this atomic
suggestion would both better belong in shutil.

Cheers,
Cameron Simpson cs@cskk.id.au

ericvsmith · 2021-11-16T00:29:47.828Z

cameron:

Personally I think both the chdir() context manager and this atomic
suggestion would both better belong in shutil.

I completely agree.

malemburg · 2021-11-16T09:09:16.000Z

There were many suggestions in the os.workdir() thread I had started on
python-ideas and people could not agree (e.g. os, os.path, pathlib,
shutils, contextlib). My initial idea was to have it in the os module,
next to os.chdir() and with a different name.

FWIW, I think contextlib is as good as any module and it also
underlines that the symbol refers to a context manager rather than
a standard function - something which is not always apparent from
looking just at the function names or the documentation (e.g.
the built-in open() doesn’t even mention that the returned file
object can be used as a context manager).

ncoghlan · 2021-11-17T14:22:23.767Z

For an earlier bikeshed on this topic: Issue 8604: Adding an atomic FS write API - Python tracker

The main reason I still believe this is a feature worthy of the standard library is that without it, even something as simple as “process A periodically writes a file, process B periodically reads it” is unreliable, since process B may try to access the file when process A is halfway through writing it.

By only offering in-place file modification by default, we’re setting new users up to run into a well known problem where the patterns for solving it are far from obvious (e.g. why it matters that the temp file be created in the same directory as the target file)

At my current workplace, we ended up with a two layer API:

An “atomic_write” context manager
A “set_file_contents” helper function

The first one is the non-obvious piece that I think is worthy of the stdlib, while the latter probably fails the “not every 3 line helper function needs to be in the stdlib” test.

(FWIW, I think “shutil.staged_write” would be fine as a name, emphasising the body of the context manager over the end of it. We’d still want to use the phrase “atomic write” somewhere in the documentation, though)

ncoghlan · 2021-11-17T14:28:42.690Z

Regarding the boundaries between contextlib and shutil: they’re definitely blurry, but a rough rule of thumb is that contextlib definitely shouldn’t affect anything other than the running process.

So redirecting standard streams and changing the current working directory is reasonable (albeit still debatable), but creating files is a definite “No, put it in shutil instead”.

steven.daprano · 2021-11-18T04:44:11.000Z

Antoine Pitrou:

“I would never go and look in contextlib for OS-related utilities.”

But would you look in contextlib for a context manager?

pitrou · 2021-11-18T08:29:38.961Z

steven.daprano:

But would you look in contextlib for a context manager?

Would you look in functools for a random function?
I don’t, and I don’t look into contextlib for random context managers (and there isn’t a classlib module where I can look for random classes, either).

chrish · 2021-11-19T04:52:24.874Z

Yes. I was coming here to say this. The individual writes are not necessarily atomic, but the whole file is. From the point of view of other processes, it goes from not existing / being there with some old content directly to being there with new content, with nothing in between those two states. So, something like atomicfile (maybe with underscore in the middle?) would feel right to me.

ncoghlan · 2021-11-22T23:01:08.433Z

Despite using “atomic write” as the name for my own staged write implementations, this thread made me realise that there is a major problem with the name: the “atomic” property doesn’t hold in the presence of multiple writers. Truly atomic operations (think atomic increments and decrements in a CPU) include a mechanism to block other writers to ensure two different writers won’t use the same starting state for their modifications.

While one writer, multiple readers is a common underlying assumption in “atomic” write implementations, the key advantage I see to the suggested “staged_write” name is that the name is correct independent of the number of writers (and helps convey why having multiple writers would be a bad idea).

The feature could also theoretically be used as the basis for a future true “atomic_write” implementation in the multiprocessing module, although I personally doubt we’d ever add that (once you need multiple writers, it’s usually worth reaching for sqlite instead of a flat file, and file locking has enough nasty failure modes that it’s hard to create abstract solutions that don’t end up looking like some form of database).

pitrou · 2021-11-23T09:21:21.788Z

ncoghlan:

Despite using “atomic write” as the name for my own staged write implementations, this thread made me realise that there is a major problem with the name: the “atomic” property doesn’t hold in the presence of multiple writers.

I’m not sure what you mean by that, because multiple writers actually should be fine with such an API. This is why it works for e.g. the JIT caching example I gave: you can have multiple independent processes updating a given cache entry at the same time, and the end result will be correct.

It is also how bytecode caching is implemented, btw, and you can be sure that multiple writers being a problem would have resulted in hundreds of bug reports over the years:

github.com

python/cpython/blob/d82f2caf942fa8b94e797a2f116ee54ec303c2df/Lib/importlib/_bootstrap_external.py#L185-L204


      
          def _write_atomic(path, data, mode=0o666):
              """Best-effort function to write data to a path atomically.
              Be prepared to handle a FileExistsError if concurrent writing of the
              temporary file is attempted."""
              # id() is used to generate a pseudo-random filename.
              path_tmp = '{}.{}'.format(path, id(path))
              fd = _os.open(path_tmp,
                            _os.O_EXCL | _os.O_CREAT | _os.O_WRONLY, mode & 0o666)
              try:
                  # We first write data to a temporary file, and then use os.replace() to
                  # perform an atomic rename.
                  with _io.FileIO(fd, 'wb') as file:
                      file.write(data)
                  _os.replace(path_tmp, path)
              except OSError:
                  try:
                      _os.unlink(path_tmp)
                  except OSError:
                      pass
                  raise

malemburg · 2021-11-23T09:52:31.000Z

The rename is the only atomic part in the whole implementation,
which is why I believe that using the term transaction is more
accurate.

In database transactions, the .commit() or .rollback()
makes the changes appear for others (modulo transaction isolation
settings and some other details, but let’s leave those aside).

Multiple writers will each get a different temporary file name
and write to this name. And all will eventually rename this file
to the final name.

Now, if all writers write the same content (as in the bytecode
example Antoine gave), there’s no problem. The inode of the file
will change, but not its contents.

If, however, they write different content, the final state of
the file will be undefined, since it is not clear which of the
writers will run the os.rename() last.

And the writers won’t notice on Unix, since os.rename() replaces
the file, even if it exists.

On Windows, the situation is different: os.rename() won’t
overwrite existing files.

There are situations where one or the other is better. On Unix,
you can use hardlinks to get similar behavior to Windows,
if you want an error raised for existing files.