Skip to content

Commit

Permalink
Fsync before rename after copy in DiskCacheClient
Browse files Browse the repository at this point in the history
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 bazelbuild#16691.

PiperOrigin-RevId: 486901488
Change-Id: Ia2aef6bc196e1d0a61f46b7b6c719563938ff48e
  • Loading branch information
artem-zinnatullin authored and Copybara-Service committed Nov 8, 2022
1 parent 0331b0d commit 9cb5e0a
Showing 1 changed file with 6 additions and 3 deletions.
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

0 comments on commit 9cb5e0a

Please sign in to comment.