fix(scheduler): record each pend reason once per job (bug-1927)

The scheduler appended one NO_RESOURCE/MATCH_FAIL entry per host per
placement plan, so a pending parallel job carried plans x hosts copies
(~93k entries in the 1000-host bounds test). libxce gains
add_job_preason_once() with a per-context hash index keyed on
(host, resource, code) that keeps first-occurrence order; the
scheduler's three builders use it. add_job_preason() is unchanged for
other callers. The parjob equivalence test now expects each short host
once; the bounds test runs in ~2 s instead of ~19 s.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/scheduler/include/sched/types.h

@@ -7,6 +7,7 @@
 #include <stdlib.h>
 
 #include "kvmgr.h"
+#include "preason.h"
 #include "resreq.h"
 #include "typemap.h"
 #include "xce/types.h"
@@ -196,6 +197,11 @@ typedef struct {
     xce_sched_alloc_req_t alloc_req;
     xce_job_alloc_t*      alloc;
     xce_pendreason_t*     reason;
+    /* bug-1927: lookup index for reason's lists, so a host re-evaluated by
+     * every plan of the attempt records each (host, resource, code) once
+     * (add_job_preason_once(), preason.h). Zeroed by sched_ctx_init(),
+     * released by sched_ctx_fini(); it never owns reason itself. */
+    xce_preason_index_t   reason_index;
     /* Req 9.5: defaults to XCE_ALLOC_NORMAL via sched_ctx_init()'s existing
      * memset(ctx, 0, sizeof(*ctx)) -- XCE_ALLOC_NORMAL is the enum's zero
      * value, so no explicit assignment is needed. */

src/scheduler/src/alloc_runner.c

@@ -62,8 +62,9 @@ static int sched_alloc_ensure_reason_(xce_sched_ctx_t* ctx)
  * the same COMPOUND/ALTER alternative (Req 2.3).
  *
  * Unmet: adds one host-less NO_RESOURCE reason per entry of the unmet list
- * the check returns, and returns 0 (Req 2.2). vget() returns 0 on success,
- * so only a negative result skips an entry (bug-1893).
+ * the check returns, unless the attempt already recorded it (bug-1927),
+ * and returns 0 (Req 2.2). vget() returns 0 on success, so only a negative
+ * result skips an entry (bug-1893).
  *
  * Any failure returns its negative error code (Req 2.4). */
 static int sched_alloc_prepare_shared_(xce_sched_core_t* core,
@@ -96,8 +97,10 @@ static int sched_alloc_prepare_shared_(xce_sched_core_t* core,
                 continue;
             }
 
-            add_job_preason(ctx->reason->reasons, PREASON_NO_RESOURCE, NULL,
-                            r->name);
+            /* bug-1927: a COMPOUND's later primary short on the same
+             * pool does not repeat the entry */
+            add_job_preason_once(ctx->reason->reasons, &(ctx->reason_index),
+                                 PREASON_NO_RESOURCE, NULL, r->name);
         }
 
         return 0;

src/scheduler/src/resource.c

@@ -556,8 +556,9 @@ int sched_try_alloc_on_host_(xce_sched_worker_t* w, xce_sched_core_t* core,
             return -errno;
         }
 
-        add_job_preason(ctx->reason->reasons, PREASON_MATCH_FAIL, w->name,
-                        NULL);
+        /* bug-1927: once per (host, code), not once per plan */
+        add_job_preason_once(ctx->reason->reasons, &(ctx->reason_index),
+                             PREASON_MATCH_FAIL, w->name, NULL);
 
         return 0;
     }
@@ -598,8 +599,11 @@ int sched_try_alloc_on_host_(xce_sched_worker_t* w, xce_sched_core_t* core,
                     continue;
                 }
 
-                add_job_preason(ctx->reason->reasons, PREASON_NO_RESOURCE,
-                                w->name, r->name);
+                /* bug-1927: every plan re-checks this host; record each
+                 * (host, resource, code) once */
+                add_job_preason_once(ctx->reason->reasons,
+                                     &(ctx->reason_index),
+                                     PREASON_NO_RESOURCE, w->name, r->name);
             }
         return 0;
     }

src/scheduler/src/schedobj.c

@@ -542,6 +542,7 @@ void sched_ctx_fini(xce_sched_ctx_t* ctx)
     xvec_idval_fini(&(ctx->alloc_req.shared));
     xvec_idval_fini(&(ctx->alloc_req.host));
     xvec_idval_fini(&(ctx->unmeet));
+    erase_preason_index(&(ctx->reason_index));
 }
 
 void sched_ctx_set_primary(xce_sched_ctx_t* ctx, xce_primary_resreq_t* primary)

src/scheduler/testsuite/test_alloc_equivalence_parjob.c

@@ -586,16 +586,19 @@ static void eq_assert_reasons(eq_fixture_t* f, int n, const char** pairs)
     }
 }
 
-/* 1, 3, 4: host reasons (Req 3.6/3.7): exactly one per short resource of
- * every selection-matching unclaimed short host of each bucket search. */
+/* 1, 3, 4: host reasons (Req 3.6/3.7): every bucket search records a
+ * reason for each short resource of every selection-matching unclaimed
+ * short host, and the set holds each (host, resource) once, in first-check
+ * order (bug-1927): a later plan re-checking a host adds nothing new.
+ * 3: plan [1,1] re-finds A and B short on mem; 4: [2,2] and [2,1,1]
+ * re-find B and C short on slots. */
 static void test_parjob_host_reasons_every_short_host(void)
 {
     static const char* two[] = { HOST_A, "slots", HOST_B, "slots" };
-    static const char* hosts[]
-        = { HOST_A, "mem", HOST_A, "slots", HOST_B, "mem",
-            HOST_B, "slots", HOST_A, "mem", HOST_B, "mem" };
-    static const char* back[] = { HOST_A, "slots", HOST_B, "slots", HOST_C,
-                                  "slots", HOST_B, "slots", HOST_C, "slots" };
+    static const char* hosts[] = { HOST_A, "mem", HOST_A, "slots",
+                                   HOST_B, "mem", HOST_B, "slots" };
+    static const char* back[]
+        = { HOST_A, "slots", HOST_B, "slots", HOST_C, "slots" };
     eq_fixture_t        f;
     eq_primary_t        p;
     xce_sched_worker_t* w[3];
@@ -605,11 +608,11 @@ static void test_parjob_host_reasons_every_short_host(void)
     eq_finish(&f, &p);
 
     TEST_ASSERT_EQUAL_INT(0, eq_run(&f, &p, &EQ_SHORT_HOSTS, w));
-    eq_assert_reasons(&f, 6, hosts);
+    eq_assert_reasons(&f, 4, hosts);
     eq_finish(&f, &p);
 
     TEST_ASSERT_EQUAL_INT(1, eq_run(&f, &p, &EQ_ROLLBACK, w));
-    eq_assert_reasons(&f, 5, back);
+    eq_assert_reasons(&f, 3, back);
     eq_finish(&f, &p);
 }
 

src/scheduler/testsuite/test_alloc_runner_search.c

@@ -602,6 +602,39 @@ static void test_every_short_host_gets_reason(void)
     sr_fixture_fini(&f);
 }
 
+/* bug-1927: two plans that each check the same three short hosts record
+ * each (host, slots, NO_RESOURCE) reason once, in first-check order. Every
+ * plan re-evaluates the hosts, which used to append the identical entry
+ * once per plan (6 entries here, plans x hosts in the bounds test). */
+static void test_short_host_reason_recorded_once_across_plans(void)
+{
+    sr_fixture_t        f;
+    xce_sched_worker_t *a, *b, *c;
+
+    sr_fixture_init(&f);
+    a = sr_add_host(&f, "sr-a", 1.0f);
+    b = sr_add_host(&f, "sr-b", 1.0f);
+    c = sr_add_host(&f, "sr-c", 1.0f);
+    sr_ctx_begin(&f, 3);
+    script_plan(1, 3, 0, 0);
+    script_plan(1, 2, 0, 0);
+
+    TEST_ASSERT_EQUAL_INT(0, sr_run(&f));
+    TEST_ASSERT_EQUAL_INT(6, g_nlog); /* both plans checked every host */
+    sr_assert_log(0, a, 3);
+    sr_assert_log(1, b, 3);
+    sr_assert_log(3, a, 2);
+    sr_assert_log(5, c, 2);
+    TEST_ASSERT_NOT_NULL(f.ctx.reason);
+    TEST_ASSERT_EQUAL_INT(0, f.ctx.reason->reasons->main_reason);
+    TEST_ASSERT_EQUAL_INT(0, f.ctx.reason->reasons->nhost_reasons);
+    TEST_ASSERT_EQUAL_INT(3, f.ctx.reason->reasons->nres_reasons);
+    sr_assert_slots_reason(&f, 0, "sr-a");
+    sr_assert_slots_reason(&f, 1, "sr-b");
+    sr_assert_slots_reason(&f, 2, "sr-c");
+    sr_fixture_fini(&f);
+}
+
 /* ---------------------------------------------------------------------- */
 /* Malformed plans and the claims buffer (Req 3.8, 5.7)                    */
 /* ---------------------------------------------------------------------- */
@@ -761,6 +794,7 @@ int main(void)
     RUN_TEST(test_failing_bucket_releases_earlier_claims);
     RUN_TEST(test_rollback_then_later_plan_with_bucket_counts);
     RUN_TEST(test_every_short_host_gets_reason);
+    RUN_TEST(test_short_host_reason_recorded_once_across_plans);
     RUN_TEST(test_malformed_plans_skipped);
     RUN_TEST(test_claims_buffer_grows_to_larger_plan);
     RUN_TEST(test_claims_buffer_growth_failure);

src/scheduler/testsuite/test_trace_bounds.c

@@ -15,6 +15,9 @@
  * count every other line as omitted (stored + omitted == the unbounded
  * record's lines) and keep every decision-wide counter identical to the
  * unbounded run. Its rendering shows the truncation marker and the totals.
+ * The pend reason set holds one (host, slots, NO_RESOURCE) entry per worker
+ * although every plan re-evaluated every host (bug-1927: it held one per
+ * host per plan, ~93k entries).
  *
  * Store bound (9.5): 10,000 traced decisions put into the REPL trace
  * store: first the real bounded record from the large trace, then records
@@ -213,6 +216,50 @@ static void start_sched(void)
     g_sched->core.trace_ctl = &g_ctl;
 }
 
+static int cmp_res_reason(const void* pa, const void* pb)
+{
+    const xce_res_reason_t* a = *(const xce_res_reason_t* const*)pa;
+    const xce_res_reason_t* b = *(const xce_res_reason_t* const*)pb;
+    int                     c = strcmp(a->host, b->host);
+
+    if (c == 0) {
+        c = strcmp(a->resource, b->resource);
+    }
+    return (c != 0) ? c : (a->reason - b->reason);
+}
+
+/* bug-1927: the pend reason set holds each (host, resource, code) once:
+ * here one (host, slots, NO_RESOURCE) per worker, although each of the 64
+ * plans re-evaluated every host. Sorted copy, adjacent entries compared. */
+static void assert_reasons_distinct(const xce_pendreason_t* reason)
+{
+    const xce_job_preason_t* r;
+    xce_res_reason_t**       sorted;
+    int                      dups = 0;
+    int                      i;
+
+    TEST_ASSERT_NOT_NULL(reason);
+    r = reason->reasons;
+    TEST_ASSERT_NOT_NULL(r);
+    TEST_ASSERT_EQUAL_INT(0, r->nhost_reasons);
+    sorted = (xce_res_reason_t**)calloc((size_t)r->nres_reasons + 1,
+                                        sizeof(*sorted));
+    TEST_ASSERT_NOT_NULL(sorted);
+    memcpy(sorted, r->res_based_reasons,
+           (size_t)r->nres_reasons * sizeof(*sorted));
+    qsort(sorted, (size_t)r->nres_reasons, sizeof(*sorted), cmp_res_reason);
+    for (i = 1; i < r->nres_reasons; i++) {
+        dups += (cmp_res_reason(&sorted[i - 1], &sorted[i]) == 0);
+    }
+    free(sorted);
+    if (getenv("XCE_TRACE_BOUNDS_DUMP") != NULL) {
+        fprintf(stderr, "pend reasons: %d resource entries, %d duplicates\n",
+                r->nres_reasons, dups);
+    }
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, dups, "duplicate pend reason entries");
+    TEST_ASSERT_EQUAL_INT(NWORKERS, r->nres_reasons);
+}
+
 /* Feeds the large job to the allocator thread with line bound `bound` and
  * returns the record its decision carries (a pend). Frees the decision. */
 static xce_sched_trace_rec_t* feed_large(int bound)
@@ -235,6 +282,7 @@ static xce_sched_trace_rec_t* feed_large(int bound)
     TEST_ASSERT_NOT_NULL_MESSAGE(decision, "the allocator pushed nothing");
     TEST_ASSERT_EQUAL_INT_MESSAGE(XCE_DECISION_PEND, decision->op,
                                   "the large job must pend");
+    assert_reasons_distinct(decision->u.reason);
     rec = decision->trace;
     decision->trace = NULL;
     free(decision);
@@ -344,6 +392,11 @@ static void test_large_parallel_job_trace_stays_within_default_bound(void)
                                       "discarded 0\n"));
     TEST_ASSERT_NOT_NULL(strstr(text, "\n  totals: host evaluations: "));
     TEST_ASSERT_NULL(strstr(full_text, "trace lines omitted"));
+    /* bug-1927: one reason per host in the pend outcome */
+    TEST_ASSERT_NOT_NULL(
+        strstr(text, "\n    host bh0000: resource slots: NO_RESOURCE\n"));
+    TEST_ASSERT_NOT_NULL(strstr(
+        text, "\n    omitted hosts' pend reasons: NO_RESOURCE x950\n"));
     TEST_ASSERT_TRUE(count_lines(text) < 2 * XCE_TRACE_LINE_BOUND_DEFAULT);
     TEST_ASSERT_TRUE(count_lines(full_text) >= total_lines);
     free(text);