Files.touch causes unnecessary disk contention

[li-a]

When calling Files.touch(foo) on a file that already exists, Guava will call java.io.File#createNewFile() and assume the file already exists if the creation operation fails. But because Java guarantees the existence check and creation are atomic, the existence check lives inside a call to java.io.FileSystem#createFileExclusively, introducing concurrency penalties that might not be needed.

Instead, Guava should skip file.createNewFile() when file.exists() returns true.

[cpovirk]

As best I can tell, createFileExclusively calls open(..., O_RDWR | O_CREAT | O_EXCL, 0666). That's similar to what touch(1) does. I don't know all the reasons that go into that, but we will follow touch(1) unless we have a pressing need not to. (One guess: I suspect that any touch implementation has to take "exclusive" actions because it has to update the file's timestamp.)

[li-a]

Thank you for looking. The report doesn't claim all exclusive actions can be removed (although actually...). Rather, in some cases they are superfluous and can be bypassed by a cheaper check. Looking at UnixFileSystem, File#exists() amounts to a stat64(2) call, and the current call to File#setLastModified(...) involves calling utimes(2). These don't seem to interact with other threads like the "exclusive open" can.

I believe the reason for the JDK's O_CREAT | O_EXCL is so that File#createNewFile() can return true if and only if that thread created the file. The "exclusivity" does not extend beyond the open call, so it certainly would not affect whether other parts of the touch operation are exclusive.

Looking at GNU's touch(1) implementation, I don't see where Guava is following it or where that command is similar to createFileExclusively. It uses O_WRONLY | O_CREAT | O_NONBLOCK | O_NOCTTY flags -- there's no O_EXCL because it is not concerned with whether it created the file (another difference from Guava, which wants to skip File#setLastModified).

Side effects: The false negative race condition (a file is created after exists() returns false) will be handled by createNewFile(). The false positive race condition (file did exist but got deleted before setLastModified) was already present.

[cpovirk]

Sorry, I thought I had seen O_EXCL on the touch strace output, but you're right. Thanks for all the info. I'm still not sure if we'll end up doing something about this, but now I understand better.

[cpovirk]

(Should we just be doing what we do in MoreFiles.touch (taking into account that the File API returns false instead of throwing)?)

[cpovirk]

(Another possibility is new FileOutputStream(file, /*append=*/ true).close(), but I'm trying to not think about this, even though I'm suddenly interested -- and wondering how important O_NOCTTY|O_NONBLOCK is....)