From 9cb5e0a31665d3b3f25bf58ec2dee696e828d8b9 Mon Sep 17 00:00:00 2001 From: Artem Zinnatullin Date: Tue, 8 Nov 2022 03:22:56 -0800 Subject: [PATCH] Fsync before rename after copy in DiskCacheClient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Debugging `disk_cache` at work, found this TODO in the code, decided to implement. If `fsync` fails we don't rename to final name — which is the correct error handling as it was before if `copy` failed. Closes #16691. PiperOrigin-RevId: 486901488 Change-Id: Ia2aef6bc196e1d0a61f46b7b6c719563938ff48e --- .../devtools/build/lib/remote/disk/DiskCacheClient.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java index 6735a09978ca56..c82f9d387ccabe 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.vfs.Path; import com.google.protobuf.ByteString; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -174,11 +175,13 @@ private void saveFile(String key, InputStream in, boolean actionResult) throws I // Write a temporary file first, and then rename, to avoid data corruption in case of a crash. Path temp = toPathNoSplit(UUID.randomUUID().toString()); - try (OutputStream out = temp.getOutputStream()) { + + try (FileOutputStream out = new FileOutputStream(temp.getPathFile())) { ByteStreams.copy(in, out); + // Fsync temp before we rename it to avoid data loss in the case of machine + // crashes (the OS may reorder the writes and the rename). + out.getFD().sync(); } - // TODO(ulfjack): Fsync temp here before we rename it to avoid data loss in the case of machine - // crashes (the OS may reorder the writes and the rename). temp.renameTo(target); } }