Merge pull request #5048 from chu11/issue5029_job_list_stats_consistency

job-list: make job stats consistent to job results

Author: mergify[bot]

doc/man1/flux-jobs.rst

@@ -104,6 +104,9 @@ OPTIONS
    will display a summary of statistics along with the top 25
    running jobs, updated every 2 seconds.
 
+   Note that all job failures, including canceled and timeout jobs,
+   are collectively counted as "failed" in ``--stats``.
+
 **--stats-only**
    Output a summary of job statistics and exit.  By default shows
    global statistics.  If ``--queue`` is specified, shows statistics
@@ -116,6 +119,9 @@ OPTIONS
    All options other than ``--queue`` are ignored when
    ``--stats-only`` is used.
 
+   Note that all job failures, including canceled and timeout jobs,
+   are collectively counted as "failed" in ``--stats-only``.
+
 **-R, --recursive**
    List jobs recursively. Each child job which is also an instance of
    Flux is prefixed by its jobid "path" followed by the list of jobs,
@@ -156,7 +162,7 @@ states also exist: "pending", an alias for DEPEND,SCHED; "running", an
 alias for RUN,CLEANUP; "active", an alias for "pending,running".
 
 After a job has finished and is in the INACTIVE state, it can be
-marked with one of three possible results: COMPLETED, FAILED,
+marked with one of the possible results: COMPLETED, FAILED,
 CANCELED, TIMEOUT. Under the *result_abbrev* field name, these are
 abbreviated as CD, F, CA, and TO respectively.
 

src/bindings/python/flux/job/stats.py

@@ -78,6 +78,12 @@ def _update_cb(self, rpc):
         self.running = self.run + self.cleanup
         self.active = self.total - self.inactive
 
+        # This class reports the total number of unsuccessful jobs in
+        # the 'failed' attribute, not just the count of jobs that ran
+        # to completion with nonzero exit code
+        self.failed += self.timeout
+        self.failed += self.canceled
+
         if self.callback:
             self.callback(self, **self.cb_kwargs)
 

src/cmd/top/summary_pane.c

@@ -152,7 +152,11 @@ static void draw_stats (struct summary_pane *sum)
     }
 
     if (sum->show_details) {
-        int failed = sum->stats.failed;
+        /* flux-top reports the total number of unsuccessful jobs in
+         * the 'failed' display, not just the count of jobs that ran
+         * to completion with nonzero exit code
+         */
+        int failed = sum->stats.failed + sum->stats.timeout + sum->stats.canceled;
         int complete = sum->stats.successful;
 
         if (complete)

src/modules/job-list/stats.c

@@ -112,13 +112,16 @@ static void stats_add (struct job_stats *stats,
 
     if (state == FLUX_JOB_STATE_INACTIVE) {
         if (!job->success) {
-            stats->failed++;
             if (job->exception_occurred) {
                 if (streq (job->exception_type, "cancel"))
                     stats->canceled++;
                 else if (streq (job->exception_type, "timeout"))
                     stats->timeout++;
+                else
+                    stats->failed++;
             }
+            else
+                stats->failed++;
         }
         else
             stats->successful++;
@@ -162,13 +165,16 @@ static void stats_purge (struct job_stats *stats, struct job *job)
     stats->state_count[state_index (job->state)]--;
 
     if (!job->success) {
-        stats->failed--;
         if (job->exception_occurred) {
             if (streq (job->exception_type, "cancel"))
                 stats->canceled--;
             else if (streq (job->exception_type, "timeout"))
                 stats->timeout--;
+            else
+                stats->failed--;
         }
+        else
+            stats->failed--;
     }
     else
         stats->successful--;

t/t2260-job-list.t

@@ -352,7 +352,7 @@ test_expect_success 'job stats lists jobs in correct state (mix)' '
 	flux job stats | jq -e ".job_states.inactive == $(job_list_state_count inactive)" &&
 	flux job stats | jq -e ".job_states.total == $(job_list_state_count all)" &&
 	flux job stats | jq -e ".successful == $(job_list_state_count completed)" &&
-	flux job stats | jq -e ".failed == $(job_list_state_count failed canceled timeout)" &&
+	flux job stats | jq -e ".failed == $(job_list_state_count failed)" &&
 	flux job stats | jq -e ".canceled == $(job_list_state_count canceled)" &&
 	flux job stats | jq -e ".timeout == $(job_list_state_count timeout)" &&
 	flux job stats | jq -e ".inactive_purged == 0" &&
@@ -395,8 +395,8 @@ test_expect_success 'job-list: list successfully reconstructed' '
 	test_cmp before_reload.out after_reload.out
 '
 
-# the failed and canceled checks may look confusing.  We canceled all active jobs
-# right above here, so all those active jobs became failed / canceled as a result
+# the canceled checks may look confusing.  We canceled all active jobs
+# right above here, so all those active jobs became canceled as a result
 test_expect_success 'job stats lists jobs in correct state (all inactive)' '
 	flux job stats | jq -e ".job_states.depend == 0" &&
 	flux job stats | jq -e ".job_states.priority == 0" &&
@@ -406,7 +406,7 @@ test_expect_success 'job stats lists jobs in correct state (all inactive)' '
 	flux job stats | jq -e ".job_states.inactive == $(job_list_state_count all)" &&
 	flux job stats | jq -e ".job_states.total == $(job_list_state_count all)" &&
 	flux job stats | jq -e ".successful == $(job_list_state_count completed)" &&
-	flux job stats | jq -e ".failed == $(job_list_state_count active failed canceled timeout)" &&
+	flux job stats | jq -e ".failed == $(job_list_state_count failed)" &&
 	flux job stats | jq -e ".canceled == $(job_list_state_count active canceled)" &&
 	flux job stats | jq -e ".timeout == $(job_list_state_count timeout)" &&
 	flux job stats | jq -e ".inactive_purged == 0" &&