Fsync before rename after copy in DiskCacheClient (#16832)

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

Co-authored-by: Artem Zinnatullin <[email protected]>

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);
   }
 }