support content.backing-module=none

[garlick]

Problem: the default instance configuration loads content-sqlite but removes the database file when the instance terminates and locates the database file in /tmp, which on many systems is a ram disk. If it's memory either way - are we just expending effort moving it around?

Perhaps we could change rc1 to only load content-sqlite when statedir is defined, and advise workflow/DAT users to use flux start -o,-Sstatedir=/a/persistent/directory

[garlick]

I'm not sure about making this is the default, since there is unexplained job throughput degradation in this mode. However it is not currently possible to even select this mode. Let's allow this issue to be closed once setting content.backing-module=none works as you'd expect.

Extra credit: allow KVS checkpoints to be saved in memory after the KVS module is unloaded so that flux dump --checkpoint works after the KVS module is unloaded, and flux shutdown --dump=FILE works as expected.

[chu11]

Extra credit: allow KVS checkpoints to be saved in memory after the KVS module is unloaded so that flux dump --checkpoint works after the KVS module is unloaded, and flux shutdown --dump=FILE works as expected.

Possible idea, simply allow user to specify special sqlite file of :memory:, and everything might just work?

[garlick]

Well that got me wondering how that would go. Here's a quick patch that would use :memory: if statedir is not defined. IOW, whenever the instance is not restartable anyway. Compared to just making content-cache work standalone, an upside is that larger blobs are compressed. A downside is the unnecessary overhead of moving blobs back and forth between content-cache and content-sqlite. Here's a quick throughput test:

Using :memory: (rank 0 broker reached 1.2g)

ƒ(s=1,d=0,builddir) $ src/test/throughput.py  -x  -n 8192
number of jobs: 8192
submit time:    2.901 s (2824.2 job/s)
script runtime: 368.868s
job runtime:    368.634s
throughput:     22.2 job/s (script:  22.2 job/s)

Compared to master (rss reached 745m)

ƒ(s=1,d=0,builddir) $ src/test/throughput.py  -x  -n 8192
number of jobs: 8192
submit time:    2.947 s (2779.9 job/s)
script runtime: 322.580s
job runtime:    322.333s
throughput:     25.4 job/s (script:  25.4 job/s)

diff --git a/src/modules/content-sqlite/content-sqlite.c b/src/modules/content-sqlite/content-sqlite.c
index 6d9ad61f4..5fb46b734 100644
--- a/src/modules/content-sqlite/content-sqlite.c
+++ b/src/modules/content-sqlite/content-sqlite.c
@@ -539,11 +539,18 @@ static json_t *pack_tstat (tstat_t *ts)
     return o;
 }
 
+static bool isafile (const char *path)
+{
+    if (!strcmp (path, ":memory:"))
+        return false;
+    return true;
+}
+
 static unsigned long long get_file_size (const char *path)
 {
     struct stat sb;
 
-    if (stat (path, &sb) < 0)
+    if (!isafile (path) || stat (path, &sb) < 0)
         return 0;
     return sb.st_size;
 }
@@ -552,7 +559,7 @@ static unsigned long long get_fs_free (const char *path)
 {
     struct statvfs sb;
 
-    if (statvfs (path, &sb) < 0)
+    if (!isafile (path) || statvfs (path, &sb) < 0)
         return 0;
     return sb.f_bsize * sb.f_bavail;
 }
@@ -607,7 +614,7 @@ static int content_sqlite_opendb (struct content_sqlite *ctx, bool truncate)
     char s[128];
     int count;
 
-    if (truncate)
+    if (truncate && isafile (ctx->dbfile))
         (void)unlink (ctx->dbfile);
 
     if (sqlite3_open_v2 (ctx->dbfile, &ctx->db, flags, NULL) != SQLITE_OK) {
@@ -764,29 +771,25 @@ static struct content_sqlite *content_sqlite_create (flux_t *h)
     }
 
     /* Prefer 'statedir' as the location for content.sqlite file, if set.
-     * Otherwise use 'rundir', and enable pragmas that increase performance
-     * but risk database corruption on a crash (since rundir is temporary
-     * and the database is not being preserved after a crash anyway).
+     * Otherwise :memory: will be used.
      */
     if (!(dbdir = flux_attr_get (h, "statedir"))) {
-        dbdir = flux_attr_get (h, "rundir");
         ctx->journal_mode = "OFF";
         ctx->synchronous = "OFF";
+        if (asprintf (&ctx->dbfile, ":memory:") < 0)
+            goto error;
     }
-    if (!dbdir) {
-        flux_log_error (h, "neither statedir nor rundir are set");
-        goto error;
-    }
-    if (asprintf (&ctx->dbfile, "%s/content.sqlite", dbdir) < 0)
-        goto error;
-
-    /* If dbfile exists, we are restarting.
-     * If existing dbfile does not have the right permissions, fail early.
-     */
-    if (access (ctx->dbfile, F_OK) == 0) {
-        if (access (ctx->dbfile, R_OK | W_OK) < 0) {
-            flux_log_error (h, "%s", ctx->dbfile);
+    else {
+        if (asprintf (&ctx->dbfile, "%s/content.sqlite", dbdir) < 0)
             goto error;
+        /* If dbfile exists, we are restarting.
+         * If existing dbfile does not have the right permissions, fail early.
+         */
+        if (access (ctx->dbfile, F_OK) == 0) {
+            if (access (ctx->dbfile, R_OK | W_OK) < 0) {
+                flux_log_error (h, "%s", ctx->dbfile);
+                goto error;
+            }
         }
     }
 

[chu11]

well, I thought I was "pretty close" to finishing this up a few days ago, but kept on hitting hiccups. The latest hiccup appears to be somewhat bad, so I'm thinking I'm going to park this for now and come back to this as the issue no longer on our v0.42.0 milestones.

https://github.com/chu11/flux-core/tree/save_issue4267_content_backing_module_none

1. the testsuite probably needs to be audited a bit more as many simply assume content-sqlite is always loaded. Some tests may just happen to continue to work, but not as intended. Some tests simply need to be updated with text indicating "I'm using content-sqlite here". Some tests use alternate rc1/rc3 to load non-content-sqlite backing modules in some tests, some similar things should be done for the content-sqlite ones. Regression tests that always assumed content-sqlite was loaded need to be adjusted.

2. (this is the bigger issue) I think there is a bug somewhere in the broker's content-cache where the acct_dirty and the flush list can get out of sync. I have debugging that indicates that when a backing module is not loaded, the acct_dirty number can be different than the length of the flush list, which I think should be impossible. I suppose there is just a corner case somewhere in there that we haven't noticed b/c we've always just loaded a backing store all the time. This issue leads to a content flush hang b/c acct_dirty never becomes 0.

For example, one potential path is in cache_store() and cache_store_continuation(). An entry from the flush list is sent to the content backing module for backing, but if an error occurs, cache_store_continuation() will see the error but not put it back on the flush list. So we now have acct_dirty greater than the length of the flush list. This path doesn't appear to have caused the bug I witnessed, but is one possible path.

Edit: Another potential (similar) path may be when we unload the backing-module, so that backing store requests get lost. acct_dirty will stay incremented even though flush entries have been taken off the list.

I believe there is nothing wrong with PR #4463 so I will leave that PR as is (although I may add a cleanup or two I found along the way) for inclusion.