payout: flush the ledger to the device before the rename that publishes it

WritePendingPayoutsFile, SavePayingLocked and SavePoolRoundsLocked all wrote a
temp file and renamed it, with no fsync anywhere. That is atomic against the
process dying -- which is what the crash test exercised -- and not against the
machine dying. A power cut or a hard VM reset can land the rename on the
platter ahead of the data it points at, and what comes back is a short or
empty file.

For pending_payouts.json that is not a corrupt ledger, it is an absent one:
LoadPendingPayouts catches the parse error, logs one warning, and the node
starts up owing nobody anything. Total silent loss of the unpaid ledger, from
a clean-looking boot.

WriteFileAtomic now fflush()es and fsync()s the temp file before renaming it,
and fsync()s the containing directory afterwards so the new directory entry is
durable as well. If the data cannot be made durable the rename does not happen
at all: the previous file is still intact and still correct, and the next
write will try again. Ordering the sync before the rename is the point --
publishing a name that points at nothing is the exact failure being prevented.

The directory sync is best effort; a filesystem that will not let the
directory be opened for it is not a reason to refuse to write the ledger.

This is only affordable because the preceding commit moved all three writes
out from under gPayoutLedgerMutex / gPayoutExecMutex / gPoolRoundsMutex. An
fsync under any of those would park the pool event loop on the payout thread's
disk, which is the contention the merge was right to remove.

Applies to upstream too and is being reported there.

No on-disk format change.

1 files changed, 58 insertions, 3 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index d655ad5..45130ff 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -31,6 +31,9 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <unistd.h>
+#include <fcntl.h>
+#else
+#include <io.h>
 #endif
 
 #include <atomic>
@@ -457,9 +460,50 @@ struct VersionedBlob {
     uint64      seq = 0;
 };
 
-// Replace `path` with `data` atomically. Caller must hold the write mutex for
-// `path`: the temp name is derived from the target, so two threads in here at
-// once for one path is precisely the race this exists to prevent.
+// Push a file's contents out of the C library's buffer AND out of the kernel's
+// page cache, so the bytes exist on the device before anything points at them.
+static bool SyncFileHandle(FILE* f)
+{
+    if (fflush(f) != 0)
+        return false;
+#ifdef _WIN32
+    return _commit(_fileno(f)) == 0;
+#else
+    return fsync(fileno(f)) == 0;
+#endif
+}
+
+// Make the directory entry that rename() created durable too.
+//
+// A tmp+rename is only atomic with respect to a crash of the process. Against
+// a power cut or a hard VM reset the rename can reach the platter while the
+// data it points at has not, and the file that comes back is short or empty.
+// For pending_payouts.json that is not a corrupt file, it is a silent one:
+// LoadPendingPayouts catches the parse error, logs a single warning, and the
+// node comes up owing nobody anything.
+//
+// Best effort -- a filesystem that does not allow the directory to be opened
+// for this is not a reason to refuse to write the ledger.
+static void SyncDirectoryOf(const std::string& path)
+{
+#ifndef _WIN32
+    size_t slash = path.find_last_of('/');
+    std::string dir = (slash == std::string::npos) ? std::string(".")
+                                                   : path.substr(0, slash);
+    if (dir.empty())
+        dir = "/";
+    int fd = open(dir.c_str(), O_RDONLY);
+    if (fd < 0)
+        return;
+    fsync(fd);
+    close(fd);
+#endif
+}
+
+// Replace `path` with `data` atomically and durably. Caller must hold the
+// write mutex for `path`: the temp name is derived from the target, so two
+// threads in here at once for one path is precisely the race this exists to
+// prevent.
 static bool WriteFileAtomic(const std::string& path, const std::string& data,
                             const char* what)
 {
@@ -476,6 +520,16 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
         fclose(f);
         return false;
     }
+    // Before the rename, not after: publishing a name that points at nothing
+    // is the failure this ordering exists to prevent. If the data cannot be
+    // made durable, do not rename -- the previous file is still intact and
+    // still correct, and the next write will try again.
+    if (!SyncFileHandle(f)) {
+        LogPrint("payout", "[payout] WARNING: could not flush %s to disk (%s)"
+                 " -- keeping the previous file\n", tmp.c_str(), what);
+        fclose(f);
+        return false;
+    }
     // fclose() is where buffered data actually reaches the OS, so a failure
     // here means the .tmp is short -- renaming it over the real file would
     // destroy the good copy, which is exactly what these files exist to
@@ -494,6 +548,7 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
                  path.c_str(), what);
         return false;
     }
+    SyncDirectoryOf(path);
     return true;
 }