ovsdb: Allow online compacting on Windows.

This patch allows online compacting to be done under Windows.

To achieve the above we need to close all file handles before trying to
rename the file, switch from rename to MoveFileEx (because rename/MoveFile
fails if the destination exists), reopen the right type of log after the
rename.

If we could not reopen the compacted database or the original database
after the close simply abort and rely on the service manager. This
can be changed in the future.

Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Co-authored-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 7f8554ada418c79f84e19c63f3d8ca215fde9a1c..6a406da2a8386f477c05130048a890aed08cda0a 100644 (file)
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -622,6 +622,25 @@ ovsdb_file_commit(struct ovsdb_replica *replica,
     return NULL;
 }
 
+/* Rename 'old' to 'new', replacing 'new' if it exists.  Returns NULL if
+ * successful, otherwise an ovsdb_error that the caller must destroy. */
+static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
+ovsdb_rename(const char *old, const char *new)
+{
+#ifdef _WIN32
+    int error = (MoveFileEx(old, new, MOVEFILE_REPLACE_EXISTING
+                            | MOVEFILE_WRITE_THROUGH | MOVEFILE_COPY_ALLOWED)
+                 ? 0 : EACCES);
+#else
+    int error = rename(old, new) ? errno : 0;
+#endif
+
+    return (error
+            ? ovsdb_io_error(error, "failed to rename \"%s\" to \"%s\"",
+                             old, new)
+            : NULL);
+}
+
 struct ovsdb_error *
 ovsdb_file_compact(struct ovsdb_file *file)
 {
@@ -667,22 +686,68 @@ ovsdb_file_compact(struct ovsdb_file *file)
         goto exit;
     }
 
-    /* Replace original by temporary. */
-    if (rename(tmp_name, file->file_name)) {
-        error = ovsdb_io_error(errno, "failed to rename \"%s\" to \"%s\"",
-                               tmp_name, file->file_name);
+    /* Replace original file by the temporary file.
+     *
+     * We support two strategies:
+     *
+     *     - The preferred strategy is to rename the temporary file over the
+     *       original one in-place, then close the original one.  This works on
+     *       Unix-like systems.  It does not work on Windows, which does not
+     *       allow open files to be renamed.  The approach has the advantage
+     *       that, at any point, we can drop back to something that already
+     *       works.
+     *
+     *     - Alternatively, we can close both files, rename, then open the new
+     *       file (which now has the original name).  This works on all
+     *       systems, but if reopening the file fails then we're stuck and have
+     *       to abort (XXX although it would be better to retry).
+     *
+     * We make the strategy a variable instead of an #ifdef to make it easier
+     * to test both strategies on Unix-like systems, and to make the code
+     * easier to read. */
+#ifdef _WIN32
+    bool rename_open_files = false;
+#else
+    bool rename_open_files = true;
+#endif
+    if (!rename_open_files) {
+        ovsdb_log_close(file->log);
+        ovsdb_log_close(new_log);
+        file->log = NULL;
+        new_log = NULL;
+    }
+    error = ovsdb_rename(tmp_name, file->file_name);
+    if (error) {
         goto exit;
     }
-    fsync_parent_dir(file->file_name);
-
-exit:
-    if (!error) {
+    if (rename_open_files) {
+        fsync_parent_dir(file->file_name);
         ovsdb_log_close(file->log);
         file->log = new_log;
-        file->last_compact = time_msec();
-        file->next_compact = file->last_compact + COMPACT_MIN_MSEC;
-        file->n_transactions = 1;
     } else {
+        /* Re-open the log.  This skips past the schema log record. */
+        error = ovsdb_file_open_log(file->file_name, OVSDB_LOG_READ_WRITE,
+                                    &file->log, NULL);
+        if (error) {
+            ovs_fatal(0, "could not reopen database");
+        }
+
+        /* Skip past the data log reecord. */
+        struct json *json;
+        error = ovsdb_log_read(file->log, &json);
+        if (error) {
+            ovs_fatal(0, "error reading database");
+        }
+        json_destroy(json);
+    }
+
+    /* Success! */
+    file->last_compact = time_msec();
+    file->next_compact = file->last_compact + COMPACT_MIN_MSEC;
+    file->n_transactions = 1;
+
+exit:
+    if (error) {
         ovsdb_log_close(new_log);
         if (tmp_lock) {
             unlink(tmp_name);

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 3fe504e99a9e00982bec85218e1e4c434d2315dd..855c23713750cae62bbb605b2d1f05164d51ea14 100644 (file)
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -739,7 +739,11 @@ AT_CHECK(
 dnl There should be 6 lines in the log now.
 AT_CHECK([test `wc -l < db` -eq 6], [0], [], [],
   [test ! -e pid || kill `cat pid`])
-dnl Then check that the dumped data is correct.
+dnl Then check that the dumped data is correct.  This time first kill
+dnl and restart the database server to ensure that the data is correct on
+dnl disk as well as in memory.
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:socket --log-file="`pwd`"/ovsdb-server.log db], [0], [ignore], [ignore])
 AT_CHECK([ovsdb-client dump unix:socket ordinals], [0], [stdout], [ignore],
   [test ! -e pid || kill `cat pid`])
 AT_CHECK([${PERL} $srcdir/uuidfilt.pl stdout], [0], [dnl