fix(scheduler): restore slots-based global limit enforcement (bug-1790 regression)

The limit-plugin-ownership refactor (fa34b038) broke gitea#46's exact
original scenario. calc_wanted_job_total_resval() scans the job's
rusages by name, but "slots" is never a declared rusage entry --
seqjob_prepare_allocation()/parjob_prepare_allocation() derive its want
directly from the job's own task count via their own unconditional
key_find("slots", ...) + store, entirely outside the rusages loop. The
refactor's on-demand lookup therefore always missed for "slots",
silently leaving the computed want at 0 -- reintroducing bug-1789's
exact original symptom (matching worked, but caps never bound).

Live-confirmed against a fresh deploy of gitea#46's own repro (9 plain
sleep jobs, user1, queue normal, no -R flag): limit1/ug1_slots/
ug1_normal_slots all matched correctly but zero unsatisfy_ lines
appeared and all 9 jobs dispatched uncapped. The refactor's own new
unit test had masked this -- it artificially added a "slots" rusages
entry that doesn't reflect production reality.

Fix: sched_fill_limit_requests() now falls back to `nslots` directly
(the same value both allocators already store as "slots"'s own
per-host want) whenever the rusages lookup misses and the resource is
named "slots" -- the one documented exception to the rusages-lookup
rule, matching this codebase's existing "slots" special-casing
convention.

New test test_fill_limit_requests_global_limit_on_slots_uses_nslots
proves the fallback with no "slots" entry in rusages at all. The
original test reworked to use "mem" instead, so it now genuinely
exercises the general on-demand path.

Full xce-build.sh all asan --incremental: 94/94 Ok. Live re-verified
against both the original gitea#46 scenario (byte-identical to the
pre-refactor trace) and the ug1_global_mem scenario -- both correct.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GLfB5uWRdmx2WiZSFC7Fm7

src/scheduler/src/resource.c

@@ -967,8 +967,30 @@ int sched_fill_limit_requests(xce_sched_alloc_req_t* alloc_req,
         dst = dst_is_global ? &(alloc_req->shared) : &(alloc_req->host);
 
         if (dst_is_global && !src_is_global) {
-            calc_wanted_job_total_resval(rusages, PER_TASK, nhosts, nslots,
+            int found = calc_wanted_job_total_resval(
-                                         vres->res->name, &val);
+                rusages, PER_TASK, nhosts, nslots, vres->res->name, &val);
+
+            /* "slots" is synthetic, never a declared rusage entry: seqjob_
+             * prepare_allocation()/parjob_prepare_allocation() derive its
+             * want directly from the job's own task count (1.0 for
+             * seqjob -- always maxTasks==1 -- or (float)ctx->nslots for
+             * parjob), via their own unconditional key_find("slots", ...)
+             * + store, entirely outside the normal rusages loop -- so the
+             * lookup above always misses for it (bug-1790 refactor
+             * regression, live-confirmed against gitea#46's original
+             * ug1_slots/ug1_normal_slots/limit1 scenario: matching worked,
+             * but the cap never bound, because "wanted" silently stayed
+             * 0). Its job-wide total is exactly `nslots` itself -- the
+             * SAME value those allocators already store as its per-host
+             * want -- mirrored here as the one documented exception to
+             * the rusages-lookup rule, matching this codebase's own
+             * existing "slots" special-casing convention (both allocators
+             * already look it up by literal name, outside their normal
+             * rusages-driven computation). */
+            if (!found && (vres->res->name != NULL)
+                && (strcmp(vres->res->name, "slots") == 0)) {
+                val = (float)nslots;
+            }
         } else {
             xvec_idval_t* src
                 = src_is_global ? &(alloc_req->shared) : &(alloc_req->host);

src/scheduler/testsuite/test_resource_accounting.c

@@ -830,24 +830,27 @@ static void test_ctx_uncommit_host_reverses_provisional_debit(void)
  * (scope, underlying-resource) combinations read straight out of alloc_req
  * (already populated by whichever allocator plugin ran). This test proves
  * the fourth, different-in-kind combination (bug-1789's actual reported
- * case): a GLOBAL limit (write target alloc_req->shared) on a per-host
+ * case, generalized in bug-1790): a GLOBAL limit (write target
- * underlying resource like "slots" (limits.yaml's ug1_normal_slots,
+ * alloc_req->shared) on a per-host underlying resource that IS a normal
- * summing slots across the whole cluster) -- alloc_req->host never holds
+ * declared rusage -- e.g. "mem" (a future global mem cap, bug-1790's own
- * a job-WIDE total for this (only per-host/per-bucket-specific data for
+ * regression coverage) -- alloc_req->host never holds a job-WIDE total for
- * the real per-host capacity check), so the want is computed directly
+ * this (only per-host/per-bucket-specific data for the real per-host
- * from the job's own rusages instead, via calc_wanted_job_total_resval()
+ * capacity check), so the want is computed directly from the job's own
- * (resreq.c) -- PER_TASK: value * nslots, using nslots=3 here specifically
+ * rusages instead, via calc_wanted_job_total_resval() (resreq.c) --
- * to prove the job's WHOLE total is used, not any one host's/bucket's
+ * PER_TASK: value * nslots, using nslots=3 here specifically to prove the
- * share (this is the limit-evaluation layer, allocator-agnostic -- no
+ * job's WHOLE total is used, not any one host's/bucket's share (this is
- * seqjob/parjob-specific test needed for this any more).
+ * the limit-evaluation layer, allocator-agnostic -- no seqjob/parjob-
+ * specific test needed for this any more). "slots" is deliberately NOT
+ * used as the underlying resource here -- see the sibling test below for
+ * why it needs its own, separate coverage.
  */
 static void
 test_fill_limit_requests_global_limit_on_host_resource_reads_host(void)
 {
     xce_scheduler_t        sched;
-    xce_resource_t          slots_res_in = { 0 };
+    xce_resource_t          mem_res_in   = { 0 };
     xce_resource_t          limit_res_in = { 0 };
-    xce_resource_t*         slots_res;
+    xce_resource_t*         mem_res;
     xce_resource_t*         limit_res;
     xce_sched_alloc_req_t   alloc_req;
     xlist_t                 limits;
@@ -859,23 +862,26 @@ test_fill_limit_requests_global_limit_on_host_resource_reads_host(void)
     memset(&sched, 0, sizeof(sched));
     TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
 
-    /* "slots" -- deliberately NOT XCE_RESF_GLOBAL, matching the real
+    /* "mem" -- deliberately NOT XCE_RESF_GLOBAL, matching the real
      * makeResource() call every host-capacity resource in this codebase
-     * goes through. */
+     * goes through, AND (unlike "slots") a resource that genuinely
-    slots_res_in.name = strdup("slots");
+     * appears as a normal declared rusage entry when requested via
-    slots_res         = makeResource(sched.core.resmgr, &slots_res_in);
+     * -R "rusage[mem=...]" -- the real production shape this test's
-    TEST_ASSERT_NOT_NULL(slots_res);
+     * on-demand rusages lookup needs to match. */
+    mem_res_in.name = strdup(SCHED_MEM_RESOURCE_NAME);
+    mem_res         = makeResource(sched.core.resmgr, &mem_res_in);
+    TEST_ASSERT_NOT_NULL(mem_res);
 
     /* the LIMIT's own virtual resource -- GLOBAL, exactly as
      * instantiate_limit()->add_vres() would flag it for a limit like
-     * ug1_normal_slots (is_host_based_limit() == 0). */
+     * ug1_global_mem (is_host_based_limit() == 0). */
-    limit_res_in.name  = strdup("limit=ug1_normal_slots");
+    limit_res_in.name  = strdup("limit=ug1_global_mem");
     limit_res_in.flags = XCE_RESF_GLOBAL;
     limit_res          = makeResource(sched.core.resmgr, &limit_res_in);
     TEST_ASSERT_NOT_NULL(limit_res);
 
     memset(&vres, 0, sizeof(vres));
-    vres.res  = slots_res;
+    vres.res  = mem_res;
     vres.vres = limit_res;
 
     rc = xlist_init(&limits, 4);
@@ -883,12 +889,12 @@ test_fill_limit_requests_global_limit_on_host_resource_reads_host(void)
     rc = xlist_store(&limits, &vres);
     TEST_ASSERT_TRUE(rc >= 0);
 
-    /* the job's own declared "slots" rusage -- 2.0/task, 3 tasks total
+    /* the job's own declared "mem" rusage -- 2.0/task, 3 tasks total
      * (nslots below), for an expected job-wide total of 6.0. */
     rc = xlist_init(&rusages, 4);
     TEST_ASSERT_EQUAL_INT(0, rc);
     memset(&rv, 0, sizeof(rv));
-    rv.res      = strdup("slots");
+    rv.res      = strdup(SCHED_MEM_RESOURCE_NAME);
     rv.value    = 2.0f;
     rv.per_flag = PER_TASK;
     rc          = xlist_store(&rusages, &rv);
@@ -914,6 +920,82 @@ test_fill_limit_requests_global_limit_on_host_resource_reads_host(void)
     xlist_fini(&limits);
 }
 
+/*
+ * bug-1790 regression (found live, gitea#46): "slots" is NEVER a declared
+ * rusage entry -- seqjob_prepare_allocation()/parjob_prepare_allocation()
+ * derive its want directly from the job's own task count (1.0 for seqjob,
+ * always maxTasks==1; (float)ctx->nslots for parjob), via their own
+ * unconditional key_find("slots", ...) + store, entirely outside the
+ * normal rusages loop. The general on-demand path above (calc_wanted_job_
+ * total_resval(), keyed by scanning rusages for a matching entry) always
+ * misses for "slots" as a result -- confirmed live against a fresh deploy
+ * of limits.yaml's actual ug1_slots/ug1_normal_slots/limit1 (all global,
+ * slots-resname'd): matching worked, but every job's "wanted" silently
+ * stayed 0, so the caps never bound at all, reintroducing bug-1789's
+ * exact original symptom. sched_fill_limit_requests() (resource.c) now
+ * falls back to `nslots` directly (the SAME value those allocators
+ * already store as "slots"'s own per-host want) whenever the rusages
+ * lookup misses AND the resource is named "slots" -- this test proves
+ * that fallback with NO "slots" entry in rusages at all, matching the
+ * real production shape exactly (unlike this test's own first version,
+ * which artificially added a "slots" rusages entry and masked this exact
+ * gap).
+ */
+static void test_fill_limit_requests_global_limit_on_slots_uses_nslots(void)
+{
+    xce_scheduler_t        sched;
+    xce_resource_t          slots_res_in = { 0 };
+    xce_resource_t          limit_res_in = { 0 };
+    xce_resource_t*         slots_res;
+    xce_resource_t*         limit_res;
+    xce_sched_alloc_req_t   alloc_req;
+    xlist_t                 limits;
+    xce_vres_t              vres;
+    xlist_t                 rusages;
+    int                     rc;
+
+    memset(&sched, 0, sizeof(sched));
+    TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
+
+    slots_res_in.name = strdup("slots");
+    slots_res         = makeResource(sched.core.resmgr, &slots_res_in);
+    TEST_ASSERT_NOT_NULL(slots_res);
+
+    limit_res_in.name  = strdup("limit=ug1_normal_slots");
+    limit_res_in.flags = XCE_RESF_GLOBAL;
+    limit_res          = makeResource(sched.core.resmgr, &limit_res_in);
+    TEST_ASSERT_NOT_NULL(limit_res);
+
+    memset(&vres, 0, sizeof(vres));
+    vres.res  = slots_res;
+    vres.vres = limit_res;
+
+    rc = xlist_init(&limits, 4);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    rc = xlist_store(&limits, &vres);
+    TEST_ASSERT_TRUE(rc >= 0);
+
+    /* deliberately EMPTY -- "slots" never appears here in real production
+     * either. */
+    rc = xlist_init(&rusages, 4);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    rc = xvec_idval_init(&(alloc_req.shared), 8);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    rc = xvec_idval_init(&(alloc_req.host), 8);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    rc = sched_fill_limit_requests(&alloc_req, &limits, &rusages, 1, 4);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    TEST_ASSERT_EQUAL_INT(1, alloc_req.shared.count);
+    TEST_ASSERT_EQUAL_INT(limit_res->idx, ((int*)alloc_req.shared.idxs.data)[0]);
+    TEST_ASSERT_EQUAL_FLOAT(4.0f, ((float*)alloc_req.shared.vals.data)[0]);
+
+    xlist_fini(&rusages);
+    xlist_fini(&limits);
+}
+
 /*
  * bug-1789 (already-working sibling case, unaffected by the fix): a
  * HOST-scoped limit (its own vres NOT GLOBAL-flagged, so its write
@@ -1062,6 +1144,7 @@ int main(void)
     RUN_TEST(test_reconcile_job_resource_usage_null_args_return_einval);
     RUN_TEST(test_ctx_uncommit_host_reverses_provisional_debit);
     RUN_TEST(test_fill_limit_requests_global_limit_on_host_resource_reads_host);
+    RUN_TEST(test_fill_limit_requests_global_limit_on_slots_uses_nslots);
     RUN_TEST(test_fill_limit_requests_host_limit_on_host_resource_reads_host);
     RUN_TEST(test_fill_limit_requests_host_limit_on_global_resource_reads_shared);
     return UNITY_END();