myxce/.kiro/specs/scheduler-controller-msg-gap/tasks.md
Rongsong Shen 080ff78a80 docs(scheduler-controller-msg-gap): reconcile sched_op_name layering (bug-765 G-7)
The spec/design placed the scheduler-op name lookup as xce_sched_op_name in
lib/helper, but xce_sched_op_t is a scheduler-only type and src/lib must not
depend on scheduler headers — so it is correctly a static sched_op_name() in
scheduler.c. Code kept as-is (correct); docs amended to match:
- Task 1 intro + 1.1/1.3 + 5.7 + 9.1: scheduler-layer placement + layering note.
- design.md Component 8/9, file table, risks: same reconciliation.
Only xce_notify_event_name (int-typed, lib-level) lives in lib/helper.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-19 03:51:01 +00:00

16 KiB

Tasks: scheduler-controller-msg-gap

Generated: 2026-05-11T19:30:00-07:00 Feature: scheduler-controller-msg-gap Status: Tasks Generated

Overview

Implementation tasks for filling gaps in the scheduler's handling of controller messages. The work follows the design's build order: shared helpers first, then header definitions, plugin hooks, business logic, handler wiring, and finally tests.


Task 1: Add name-lookup helpers

Requirements: 8

Add the op/event name-lookup helpers, following the xce_dataset_table() pattern (char* return, "UNKNOWN(%d)" fallback).

Layering note (bug-765 G-7 reconciliation, 2026-06-18): the two helpers live in DIFFERENT layers by necessity. xce_notify_event_name(int) is lib-level (it takes an int NotifyData event) and lives in src/lib/helper.c (declared in src/include/helper.h). The scheduler-op name lookup operates on xce_sched_op_t, which is a scheduler-only type (src/scheduler/include/scheduler.h); src/lib must not depend on scheduler headers (no layering inversion), so it is implemented as a static sched_op_name(xce_sched_op_t) in src/scheduler/src/scheduler.c — NOT as xce_sched_op_name in lib/helper. The original design/tasks text placing it in lib/helper did not account for this layering; the as-built scheduler-layer placement is correct.

  • 1.1 Declare xce_notify_event_name(int event) in src/include/helper.h. (The scheduler-op name lookup is scheduler-internal — see the layering note — so it is a static sched_op_name in scheduler.c, not declared in helper.h.)
  • 1.2 Implement xce_notify_event_name() in src/lib/helper.c — switch on all NotifyData_data_* enum values; return symbolic string; fall back to "UNKNOWN(%d)"
  • 1.3 Implement the scheduler-op name lookup as static sched_op_name(xce_sched_op_t op) in src/scheduler/src/scheduler.c — switch on all XCE_SCHED_OP_* enum values; return symbolic string; fall back to "UNKNOWN(%d)". (Kept in the scheduler layer per the layering note above, not in lib/helper.)

Task 2: Extend scheduler.h with new op types and request fields

Requirements: 1, 3, 4, 5, 6

Add five new xce_sched_op_t enum values and five corresponding union members in xce_sched_update_request_t to carry the payloads for queue update, resource update, full resval update, incremental resval update, and job rusage.

  • 2.1 Add XCE_SCHED_OP_UPDATE_QUEUE, XCE_SCHED_OP_UPDATE_RESOURCES, XCE_SCHED_OP_UPDATE_RESVALS_FULL, XCE_SCHED_OP_UPDATE_RESVALS, and XCE_SCHED_OP_JOB_RUSAGE to the xce_sched_op_t enum in src/scheduler/include/scheduler.h
  • 2.2 Add the corresponding named union members (queueupdate, resourceupdate, resvalfullupdate, resvalupdate, job_rusage) to xce_sched_update_request_t

Task 3: Add plugin rusage hook

Requirements: 6

Add the plugin_job_rusage_hook_t typedef, the rusage_hook field in plugin_hooks_t, and the plugin_job_rusage_update() global function. This follows the identical pattern used by plugin_job_state_change().

  • 3.1 Add plugin_job_rusage_hook_t typedef and rusage_hook field to plugin_hooks_t in src/scheduler/include/plugin.h
  • 3.2 Implement plugin_job_rusage_update() in src/scheduler/src/plugin.c — iterate registered plugins and call their rusage_hook when non-NULL

Task 4: Add queue update function in schedobj.c

Requirements: 4

Implement sched_update_existing_queue() in src/scheduler/src/schedobj.c. The function locates a queue by name via sched_get_queue(), updates its mutable fields (status, priority, accept_interval, sched_delay, resreq string), calls plugin_queue_update(), and returns -ENOENT if the queue is unknown.

  • 4.1 Implement sched_update_existing_queue(xce_scheduler_t*, xce_queue_t*) in src/scheduler/src/schedobj.c with correct lookup, field update, string reallocation for resreq, plugin call, and error returns

Task 5: Extend scheduler.c dequeue processing and error logging

Requirements: 1, 3, 4, 5, 6, 7

Wire the five new op types into the switch (req->op) in sched_update(), implement sched_update_existing_job(), and add WARN-level error logging after the switch when a handler returns non-zero.

  • 5.1 Add XCE_SCHED_OP_UPDATE_QUEUE case — delegate to sched_update_existing_queue(), return -ENOENT when queue not found
  • 5.2 Add XCE_SCHED_OP_UPDATE_RESOURCES case — iterate resdata->resources[], call makeResource(sched->resmgr, r) for each non-NULL entry
  • 5.3 Add XCE_SCHED_OP_UPDATE_RESVALS_FULL case — iterate resvaldata->vals[], find resource in sched->resmgr by name, update value; log WARN and skip unknown resources
  • 5.4 Add XCE_SCHED_OP_UPDATE_RESVALS case — iterate resvals->vals[], find worker by host name via sched_get_worker(), update worker's resvals zarray; silently skip unknown hosts
  • 5.5 Add XCE_SCHED_OP_JOB_RUSAGE case — find job by jobid, store rusage data ON THE JOB (xce_job_rusages_t/xce_sched_job_rusages_t field added to xce_sched_job_t, replacing any prior, freed in kill_sched_job), call plugin_job_rusage_update(); return -ENOENT if job not found. Remediation (bug-765, gap G-5): rusage was never stored — xce_sched_job_t has no rusage field; Req 6 UB-6.1 unmet. See research.md §Gap Re-Audit.
  • 5.6 Implement AND WIRE sched_update_existing_job() — find job by jobid via sched_get_job(), update ALL mutable fields per Req 3 (state, priority, queue, resource request — resreq string realloc like sched_update_existing_queue), call plugin_job_update(); return -ENOENT when jobid unknown, -EINVAL on NULL. CRITICAL routing fix: the notify schedUpdateJob handler (sched_handle_sched_update_job, handler.c:200) and the dataset bulk-load (sched_on_jobs, handler.c:31) BOTH enqueue XCE_SCHED_OP_UPDATE_JOB, and the dequeue (scheduler.c:755) routes BOTH to sched_update_jobs() (create/replace). Distinguish the notify path (introduce a distinct op, e.g. XCE_SCHED_OP_UPDATE_JOB_FIELDS, enqueued only by the notify handler, dequeued to sched_update_existing_job) so a notify update mutates-in-place / returns -ENOENT and NEVER creates a job (Req 3: jobs created only via the dataset path). Remediation (bug-765, gap G-8+G-4): sched_update_existing_job was dead code (0 callers) and copied only state; UB-3.1/UB-3.2 unmet. See research.md §Gap Re-Audit.
  • 5.7 Add WARN log in sched_update() after switch block for non-zero rc — include symbolic op name via the static sched_op_name() (scheduler.c, see Task 1 layering note) and strerror(-rc)
  • 5.8 Free new-op payloads in sched_pend_update_killer (and any dequeue early-exit) for all new op types (UPDATE_QUEUE/UPDATE_RESOURCES/UPDATE_RESVALS_FULL/UPDATE_RESVALS/JOB_RUSAGE/the new UPDATE_JOB_FIELDS) — ownership IS transferred to the request (handler nullifies the notify/dataset payload pointers), so the empty break; cases leak. Use the lib deep-free helpers; validate ASAN-clean (no double-free). Remediation (bug-765, gap G-3): killer had no-op break cases → confirmed per-message leak. See research.md §Gap Re-Audit.

Task 6: Wire notify handlers in handler.c

Requirements: 3, 4, 5, 6, 8

Add four static handler functions for the currently-stubbed NotifyData events, wire them into sched_handle_notify(), and update the TRACE log calls in sched_handle_notify() and sched_handle_dataset() to use symbolic name helpers.

  • 6.1 Implement sched_handle_sched_update_job() — validate non-NULL, enqueue XCE_SCHED_OP_UPDATE_JOB; return -EINVAL on NULL, -ENOMEM on alloc failure
  • 6.2 Implement sched_handle_sched_update_queue() — validate non-NULL, enqueue XCE_SCHED_OP_UPDATE_QUEUE; return -EINVAL on NULL, -ENOMEM on alloc failure
  • 6.3 Implement sched_handle_sched_update_resvals() — skip if NULL or count==0, enqueue XCE_SCHED_OP_UPDATE_RESVALS; return -ENOMEM on alloc failure
  • 6.4 Implement sched_handle_sched_job_rusage() — validate non-NULL, enqueue XCE_SCHED_OP_JOB_RUSAGE; return -EINVAL on NULL, -ENOMEM on alloc failure
  • 6.5 Wire all four handlers into sched_handle_notify() switch — replace stub break / TRACE-only cases
  • 6.6 Update TRACE log in sched_handle_dataset() from %d table integer to %s via xce_dataset_table() (8.2), and update notify event logs to use xce_notify_event_name() (8.3)

Task 7: Add dataset handlers in handler.c

Requirements: 1, 8

Add sched_handle_resources() and sched_handle_resvals() static functions and wire them into the sched_handle_dataset() switch for the two currently-empty Data table cases. Update the existing opcode log in scheduler_req_handler to use xce_request_op_name().

  • 7.1 Implement sched_handle_resources() — skip if NULL or count==0, enqueue XCE_SCHED_OP_UPDATE_RESOURCES; return -ENOMEM on alloc failure
  • 7.2 Implement sched_handle_resvals() — skip if NULL or count==0, enqueue XCE_SCHED_OP_UPDATE_RESVALS_FULL; return -ENOMEM on alloc failure
  • 7.3 Wire sched_handle_resources() and sched_handle_resvals() into sched_handle_dataset() switch replacing the empty break cases
  • 7.4 Update opcode log in scheduler_req_handler to call xce_request_op_name() instead of printing the numeric value (8.1)

Task 8: Remove dead code pushmsg branches

Requirements: 2

Delete the Request_body_pushmsg case block from scheduler_req_handler() and the static functions sched_handle_jobpush() and sched_handle_workerpush(). These paths are unreachable because vtask_entry() rewrites all pushmsg into datamsg before invoking the handler.

  • 8.1 Remove the Request_body_pushmsg case and its two nested branches (job/worker) from scheduler_req_handler() in src/scheduler/src/handler.c
  • 8.2 Remove the static function definitions sched_handle_jobpush() and sched_handle_workerpush() from src/scheduler/src/handler.c

Task 9: Write unit tests for all new message handlers

Requirements: 1, 2, 3, 4, 5, 6, 7, 8

Extend src/scheduler/testsuite/test.c with unit tests covering every acceptance criterion. Tests must run without a live server — use stub schedulers, table-driven cases, and the existing testsuite harness.

  • 9.1 Test xce_notify_event_name() (lib helper) — verify known values return correct strings, unknown values return "UNKNOWN(%d)" format. (The scheduler-op name lookup is a static sched_op_name in scheduler.c — scheduler-internal, not directly unit-testable from test.c; it is exercised indirectly via the Req 7 WARN path.)
  • 9.2 Test sched_handle_resources() and sched_handle_resvals() handler functions — verify enqueue on valid input, skip on NULL/empty, -ENOMEM path
  • 9.3 Test sched_handle_sched_update_job(), sched_handle_sched_update_queue(), sched_handle_sched_update_resvals(), sched_handle_sched_job_rusage() — verify -EINVAL on NULL, enqueue on valid input
  • 9.4 Test sched_update_existing_job() — verify -ENOENT for unknown jobid, correct field update and plugin call for known jobid
  • 9.5 Test sched_update_existing_queue() — verify -ENOENT for unknown queue, correct field update and plugin call for known queue; verify -EINVAL on NULL payload
  • 9.6 Test dequeue path for XCE_SCHED_OP_UPDATE_RESVALS — verify unknown host skipped, known host updated
  • 9.7 Test dequeue path for XCE_SCHED_OP_JOB_RUSAGE — verify -ENOENT for unknown jobid, rusage stored and plugin called for known jobid; verify -EINVAL on NULL rusage
  • 9.8 Verify dead code symbols are absent — compile-time check that sched_handle_jobpush / sched_handle_workerpush are not defined (2.1, 2.2)
  • [~] 9.9 (gated: log capture infrastructure) Verify WARN log is emitted in sched_update() when any handler returns a non-zero error code (optional — may require log capture infrastructure)

Implementation Notes

  • 5.6 (bug-765 remediation, gaps G-8+G-4, done 2026-06-18): Introduced a distinct op XCE_SCHED_OP_UPDATE_JOB_FIELDS (+ jobfieldsupdate.job union member) so the notify schedUpdateJob handler routes to sched_update_existing_job (now in schedobj.c) which mutates state/priority/queue/resreq IN PLACE and returns -ENOENT for unknown jobid — never creating a job. The dataset bulk-create path (sched_on_jobs → XCE_SCHED_OP_UPDATE_JOB → sched_update_jobs) is unchanged. Killer frees the new op's payload via free_Job_ptr. New white-box test testsuite/test_job_update.c (separate meson executable sched_job_update_test, white-box #include "../src/schedobj.c" + stubs for scheduler-internal externs): 3/3 PASS, regression sched_test 15/15, ASAN clean (leaks-off per suite convention). VERIFY NOTES: (1) the handler→dequeue ROUTING is verified by code review, NOT an automated integration test (an end-to-end test through sched_handle_notify needs NotifyData capnp plumbing — follow-up); (2) free(job->resreq) on replace is shallow but consistent with the codebase — kill_sched_job is a no-op stub, so sched jobs are not deep-freed anywhere; the inner-resreq leak is pre-existing, not introduced here. (3) test_job_update.c originally had an accidental */ inside a comment (make_*/kill_*) + non-ASCII chars + a make_sched_obj stub conflicting with the static def in schedobj.c — rewritten ASCII-clean with correct stub set. Task 9.4 (test sched_update_existing_job) is now genuinely satisfied by this real test.
  • 5.5 (bug-765 remediation, gap G-5, done 2026-06-18): Added xce_job_rusages_t* rusages to xce_sched_job_t (sched/types.h). New sched_apply_job_rusage() (schedobj.c) stores the report ON the job (ownership transferred), replacing any prior via the generated free_JobRusages_ptr; returns -ENOENT unknown / -EINVAL NULL. The dequeue JOB_RUSAGE case calls it and NULLs req->u.job_rusage.rusages on success so the killer doesn't double-free; kill_sched_job (was a no-op stub) now releases job->rusages. NOTE: used the generated capnp deep-freer free_JobRusages_ptr (NOT a hand-rolled freer — it correctly frees pids/pgids/usages incl. resname per the capnp allocation model; an earlier hand-rolled free_job_rusages was removed). 4 new white-box tests (store / replace-frees-prior / -ENOENT / -EINVAL): sched_job_update_test 7/7.
  • 5.8 (bug-765 remediation, gap G-3, done 2026-06-18): sched_pend_update_killer now frees all five previously-leaked op payloads with the exact generated deep-freers — free_QueueConf_ptr (xce_queue_t), free_Resource_ptr[]+free(arr) (xce_resource_t), free_ResourceVal_ptr[]+free(arr) (xce_resval_t), free_ResValList_ptr (xce_resval_list_t), free_JobRusages_ptr (xce_job_rusages_t). Safe because: ownership is transferred (notify sched_handle_notify and dataset sched_handle_dataset nullify their pointers on enqueue), makeResource constructs a fresh instance via makeInstance (no alias), and the dequeue copies values (no alias). VERIFY NOTE: the JOB_RUSAGE killer path interacts with 5.5's ownership; the other four killer frees are review-verified (type-exact generated freers + confirmed transfer/non-alias) but NOT runtime-ASAN-exercised — those notify/dataset ops are not emitted to the scheduler in production today (research R-3 dormant paths) and constructing capnp payloads for a dequeue test is a follow-up. regression sched_test 15/15; no bad-free/double-free under ASAN.
  • G-7 (bug-765 doc drift, resolved 2026-06-18, DOC-ONLY): the scheduler-op name lookup is static sched_op_name(xce_sched_op_t) in scheduler.c, NOT xce_sched_op_name in lib/helper. RESOLUTION = fix the docs (code is correct): xce_sched_op_t is a scheduler-only type (scheduler.h) and src/lib has zero scheduler-header dependencies, so placing the helper in lib/helper would be a layering inversion. Only xce_notify_event_name(int) (lib-level) belongs in lib/helper. Updated Task 1 (intro + 1.1/1.3 + 5.7 + 9.1) and design.md Component 8/9 + file table + risks to reflect the scheduler-layer placement. No code change (kept sched_op_name in origin per user direction).