Merge pull request #5150 from garlick/issue#5147

job-manager: make some replay errors non-fatal

Author: chu11

src/modules/job-list/job_state.c

@@ -673,7 +673,7 @@ static int depthfirst_map_one (struct job_state_ctx *jsctx,
     flux_future_t *f3 = NULL;
     const char *eventlog, *jobspec, *R;
     char path[64];
-    int rc = -1;
+    int rc = 0; // non-fatal error
 
     if (strlen (key) <= dirskip) {
         errno = EINVAL;
@@ -714,7 +714,7 @@ static int depthfirst_map_one (struct job_state_ctx *jsctx,
     if (job->states_mask & FLUX_JOB_STATE_RUN) {
         if (flux_job_kvs_key (path, sizeof (path), id, "R") < 0) {
             errno = EINVAL;
-            return -1;
+            goto done;
         }
         if (!(f3 = flux_kvs_lookup (jsctx->h, NULL, 0, path)))
             goto done;
@@ -736,7 +736,7 @@ static int depthfirst_map_one (struct job_state_ctx *jsctx,
 
     rc = 1;
 done:
-    if (rc < 0)
+    if (rc == 0)
         job_destroy (job);
     flux_future_destroy (f1);
     flux_future_destroy (f2);

src/modules/job-manager/restart.c

@@ -46,7 +46,10 @@ int restart_count_char (const char *s, char c)
     return count;
 }
 
-static struct job *lookup_job (flux_t *h, flux_jobid_t id, flux_error_t *error)
+static struct job *lookup_job (flux_t *h,
+                               flux_jobid_t id,
+                               flux_error_t *error,
+                               bool *fatal)
 {
     flux_future_t *f1 = NULL;
     flux_future_t *f2 = NULL;
@@ -63,24 +66,33 @@ static struct job *lookup_job (flux_t *h, flux_jobid_t id, flux_error_t *error)
                    "cannot send lookup requests for job %ju: %s",
                    (uintmax_t)id,
                    strerror (errno));
+        *fatal = true;
         goto done;
     }
     if (flux_kvs_lookup_get (f1, &eventlog) < 0) {
         errprintf (error, "lookup %s: %s", k1, strerror (errno));
+        *fatal = false;
         goto done;
     }
     if (flux_kvs_lookup_get (f2, &jobspec) < 0) {
         errprintf (error, "lookup %s: %s", k2, strerror (errno));
+        *fatal = false;
         goto done;
     }
-    if (!(job = job_create_from_eventlog (id, eventlog, jobspec, &e)))
+    if (!(job = job_create_from_eventlog (id, eventlog, jobspec, &e))) {
         errprintf (error, "replay %s: %s", k1, e.text);
+        *fatal = true;
+    }
 done:
     flux_future_destroy (f1);
     flux_future_destroy (f2);
     return job;
 }
 
+/* Create a 'struct job' from the KVS, using synchronous KVS RPCs.
+ * Return 1 on success, 0 on non-fatal error, or -1 on a fatal error,
+ * where a fatal error will prevent flux from starting.
+ */
 static int depthfirst_map_one (flux_t *h,
                                const char *key,
                                int dirskip,
@@ -101,8 +113,22 @@ static int depthfirst_map_one (flux_t *h,
         errprintf (error, "could not decode %s to job ID", key + dirskip + 1);
         return -1;
     }
-    if (!(job = lookup_job (h, id, error)))
-        return -1;
+
+    flux_error_t lookup_error;
+    bool fatal = false;
+    if (!(job = lookup_job (h, id, &lookup_error, &fatal))) {
+        if (fatal) {
+            errprintf (error, "%s", lookup_error.text);
+            return -1;
+        }
+        flux_log (h,
+                  LOG_ERR,
+                  "job %ju not replayed: %s",
+                  (uintmax_t)id,
+                  lookup_error.text);
+        return 0;
+    }
+
     if (cb (job, arg, error) < 0)
         goto done;
     rc = 1;

…er/dumps/invalid/dump-noeventlog.tar.bz2 …nager/dumps/warn/dump-noeventlog.tar.bz2t/job-manager/dumps/invalid/dump-noeventlog.tar.bz2 renamed to t/job-manager/dumps/warn/dump-noeventlog.tar.bz2 t/job-manager/dumps/invalid/dump-noeventlog.tar.bz2 renamed to t/job-manager/dumps/warn/dump-noeventlog.tar.bz2

@@ -19,6 +19,16 @@ restart_flux() {
 		flux module stats job-manager
 }
 
+# Returns 0 if dump file is replayed successfully AND one or more
+# "not replayed" warnings were logged
+restart_with_job_warning() {
+	local out=$(basename $1).dmesg
+	flux start -o,-Scontent.restore=$1 /bin/true 2>$out
+	result=$?
+	cat $out
+	test $result -eq 0 && grep -q "not replayed:" $out
+}
+
 test_expect_success 'verify that job manager can restart with current dump' '
 	restart_flux dump.tar >stats.out
 '
@@ -193,6 +203,11 @@ for dump in ${DUMPS}/valid/*.tar.bz2; do
     test_expect_success 'successfully started from '$testname "restart_flux $dump"
 done
 
+for dump in ${DUMPS}/warn/*.tar.bz2; do
+    testname=$(basename $dump)
+    test_expect_success 'successfully started with warning from '$testname "restart_with_job_warning $dump"
+done
+
 for dump in ${DUMPS}/invalid/*.tar.bz2; do
     testname=$(basename $dump)
     test_expect_success 'failed on '$testname "test_must_fail restart_flux $dump"