From b019755f268ad5b77de333f1afc20d849a795612 Mon Sep 17 00:00:00 2001
From: Albert Chu <chu11@llnl.gov>
Date: Tue, 27 Aug 2024 11:57:32 -0700
Subject: [PATCH] kvs: call content.flush before checkpoint

Problem: When the KVS module is unloaded, a checkpoint of the root
reference is attempted.  However, a content.flush is not done
beforehand.  This could result in an invalid checkpoint reference
as data is not guaranteed to be flushed to the backing store.

Solution: Call content.flush before checkpointing.

Fixes #6237
---
 src/modules/content/cache.c    |  2 +-
 src/modules/kvs/kvs.c          | 21 +++++++++++++++++----
 t/t0028-content-backing-none.t |  3 ++-
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/modules/content/cache.c b/src/modules/content/cache.c
index 6d4da698e9ce..68116bdfc886 100644
--- a/src/modules/content/cache.c
+++ b/src/modules/content/cache.c
@@ -34,7 +34,7 @@
  * The callback is synchronized with the instance heartbeat, with a
  * sync period upper bound set to 'sync_max' seconds.
  */
-static double sync_max = 10.;
+static double sync_max = 100.;
 
 static const char *default_hash = "sha1";
 
diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c
index 6b3b8a2179c9..06652e000ef7 100644
--- a/src/modules/kvs/kvs.c
+++ b/src/modules/kvs/kvs.c
@@ -2914,15 +2914,28 @@ static int checkpoint_get (flux_t *h, char *buf, size_t len, int *seq)
  */
 static int checkpoint_put (flux_t *h, const char *rootref, int rootseq)
 {
-    flux_future_t *f = NULL;
+    flux_future_t *f1 = NULL;
+    flux_future_t *f2 = NULL;
     int rv = -1;
 
-    if (!(f = kvs_checkpoint_commit (h, NULL, rootref, rootseq, 0, 0))
-        || flux_rpc_get (f, NULL) < 0)
+    /* first must ensure all content is flushed */
+    if (!(f1 = flux_rpc (h, "content.flush", NULL, 0, 0))
+        || flux_rpc_get (f1, NULL) < 0) {
+        /* fallthrough to kvs_checkpoint_commit(), ENOSYS may be due
+         * to no backing store, but checkpoint can still be done to
+         * content cache.
+         */
+        if (errno != ENOSYS)
+            goto error;
+    }
+
+    if (!(f2 = kvs_checkpoint_commit (h, NULL, rootref, rootseq, 0, 0))
+        || flux_rpc_get (f2, NULL) < 0)
         goto error;
     rv = 0;
 error:
-    flux_future_destroy (f);
+    flux_future_destroy (f1);
+    flux_future_destroy (f2);
     return rv;
 }
 
diff --git a/t/t0028-content-backing-none.t b/t/t0028-content-backing-none.t
index abc72df1b079..9bf6723f0847 100755
--- a/t/t0028-content-backing-none.t
+++ b/t/t0028-content-backing-none.t
@@ -54,7 +54,8 @@ test_expect_success 'load kvs and create some kvs data' '
 '
 
 test_expect_success 'reload kvs' '
-        flux module reload kvs &&
+        flux module reload kvs
+        flux kvs get a &&
         test $(flux kvs get a) = "1" &&
         test $(flux kvs get b) = "foo"
 '