docs(spec): libxce-alloc-preason-equality tasks approved for implementation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dcf54387ae
commit
094577f245
GPG key ID:
80533536B0EB651C
4 changed files with 118 additions and 104 deletions
|
|
@ -29,7 +29,7 @@
|
|||
- Do not introduce any new comparison call site beyond the existing four consumers.
|
||||
- Do not touch the controller's persistence/resync code paths, or how the controller/worker interpret `from`.
|
||||
- Do not change `make_alloc()`'s signature or its own `time(NULL)` default (controller and many tests call it); the scheduler overwrites `from` after construction (Decision 8).
|
||||
- Do not touch `merge_job_alloc()`/`same_alloc_slot()` (bug-1863); `alloc_eq` neither replaces nor reuses `same_alloc_slot` (see Decision 6). Their NULL-dereference defect is bug-1882.
|
||||
- Do not touch `merge_job_alloc()`/`same_alloc_slot()` (bug-1863); `alloc_eq` neither replaces nor reuses `same_alloc_slot` (see Decision 6). (Their NULL-dereference defect, bug-1882, is already fixed in f9146864.)
|
||||
- Do not remove the test files' white-box `#include "../src/sim_verify.c"`.
|
||||
- Do not make recordings made before this change verify cleanly; their wall-clock `from` values are an accepted limitation.
|
||||
- Do not change the simulator's clock or `--sim-speed`; that is spec `scheduler-sim-discrete-clock`, which this spec depends on.
|
||||
|
|
@ -39,7 +39,7 @@
|
|||
### This Spec Owns
|
||||
- The public equality functions `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` (`xcealloc.{c,h}`) and `job_preason_eq` (`preason.{c,h}`), their equality contracts and their difference-description format.
|
||||
- The shared NULL-safe string-equality helper both modules need (see Decision 5).
|
||||
- The scheduler's stamping of `from` (scheduler.c) and the single normal-mode session-start reading shared by `core->session_start`, the recorded `beginRound`, and `from` (driver.c).
|
||||
- The scheduler's stamping of `from` (`sched_make_primary_alloc_()` in scheduler.c, from the session core) and the single normal-mode session-start reading shared by `core->session_start`, the recorded `beginRound`, and `from` (driver.c).
|
||||
- The migration of `sim_verify.c` and the three white-box test files onto those functions, and the difference text in `sim_log_mismatch`.
|
||||
- libxce unit tests for the new functions (`test_xcealloc.c`, `test_preason.c`) and scheduler tests for `from` stamping.
|
||||
|
||||
|
|
@ -53,15 +53,15 @@
|
|||
### Allowed Dependencies
|
||||
- `src/include/xce/types.h` (the struct definitions — unchanged).
|
||||
- `xcealloc.c`/`xcealloc.h`, `preason.c`/`preason.h`, and `utils.{c,h}` (the modules that already own construction of these structs, plus the shared string helper).
|
||||
- `string.h` (`strcmp`) and `stdio.h` (`vasprintf`) for comparisons and descriptions.
|
||||
- Scheduler: `driver.c`, `scheduler.c`, `sim_verify.c`, `sim_loop.c` (the `sim_decision_eq` call only).
|
||||
- `string.h` (`strcmp`) and `stdio.h` (`vasprintf`, a GNU extension: `xcealloc.c`/`preason.c` define `_GNU_SOURCE` before their first include, as `client_failover.c:14` does) for comparisons and descriptions.
|
||||
- Scheduler: `driver.c`, `scheduler.c`, `sim_verify.c`, `sim_loop.c` (the `sim_decision_eq`/`sim_log_mismatch` call sites only).
|
||||
- Spec `scheduler-sim-discrete-clock`: after it, a replayed session's `core->session_start` equals the recorded `beginRound` exactly.
|
||||
|
||||
### Revalidation Triggers
|
||||
- A change to the `xce_alloc_t`/`xce_host_alloc_t`/`xce_job_alloc_t`/`xce_job_preason_t` struct layouts must re-check the field lists, the exclusion documentation, and the description text.
|
||||
- A change to `same_alloc_slot()`'s matching rule, or a proposal to merge it with `alloc_eq`, must re-check Decision 6 (the two answer different questions and disagree on `instid`, `from`, and `value`).
|
||||
- A change to the pending-reason construction paths (cache-hit vs. full-computation) that alters whether detail lists are populated must re-check the simulator's main-reason-only PEND rationale (bug-1845) and the full-match `job_preason_eq` contract.
|
||||
- Any new path that builds an `xce_alloc_t` in the scheduler outside a session, a second clock read between session start and recording, or a change to `scheduler-sim-discrete-clock`'s clock guarantee, must re-check Decision 8.
|
||||
- Any new allocation path that does not go through `sched_make_primary_alloc_()`, a second clock read between session start and recording, or a change to `scheduler-sim-discrete-clock`'s clock guarantee, must re-check Decision 8.
|
||||
|
||||
## Architecture
|
||||
|
||||
|
|
@ -74,7 +74,9 @@ The four allocation/pending-reason structs are defined in `src/include/xce/types
|
|||
|
||||
Since this design was first drafted, bug-1863 (`aea34738`) added `merge_job_alloc()` to `xcealloc.c` with a private `same_alloc_slot()` helper that matches two `xce_alloc_t` records by `resname`, `instid`, and `reserve`, to decide whether they are the same reservation slot to merge. It is a near neighbour of `alloc_eq` but has a different contract (see Decision 6).
|
||||
|
||||
**How `from` is set today.** `make_alloc()` (`xcealloc.c:30`) stamps `from = time(NULL)`. The scheduler's only production constructor call is `scheduler.c:1893`, inside a scheduling session. `from` is on the wire (`controller.capnp` `from @4: UInt64 # alloc start time`) and `dup_alloc()` copies it.
|
||||
**How `from` is set today.** `make_alloc()` (`xcealloc.c`) stamps `from = time(NULL)`. The scheduler's only production constructor call is in `sched_make_allocs()` (`scheduler.c`), called by the seqjob/parjob allocators' `my_alloc()` inside a scheduling session. `from` is on the wire (`controller.capnp` `from @4: UInt64 # alloc start time`) and `dup_alloc()` copies it. Neither the controller nor the worker reads an allocation's `from` today.
|
||||
|
||||
**Which core `sched_make_allocs()` sees.** Its first argument is `plugin->sched`. In normal mode that is the real scheduler, whose `core` runs the sessions. In simulation it is **not**: `sim_alloc.c` (`sim_bootstrap_seqjob_allocator()`/`..._parjob_...`) hands the allocators a throwaway `xce_scheduler_t` wrapper with its own `xce_sched_core_init()`'d core, aliasing only `resmgr`; that core never begins a session, so its `session_start` is 0. The honest session core is the `core` argument threaded through `sched_make_allocation()` → `sched_make_primary_alloc_(core, ctx)` (`sim->core` in simulation, `sched->core` in normal mode). Every allocation — PRIMARY, each primary of a COMPOUND, each alternative of an ALTER — is produced by one `sched_make_primary_alloc_()` call.
|
||||
|
||||
**How session time is read today.** Two separate clock reads per session:
|
||||
- `xce_core_begin_session()` sets `core->session_start = xce_core_now(core)` (`core_session.c:429`).
|
||||
|
|
@ -130,13 +132,14 @@ flowchart LR
|
|||
- `src/lib/xcealloc.c` / `src/include/xcealloc.h` — add `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` with the optional `char** why` description.
|
||||
- `src/lib/preason.c` / `src/include/preason.h` — add `job_preason_eq` (full match) with the optional description.
|
||||
- `src/scheduler/src/driver.c` — record `beginRound` from `sched->core.session_start` instead of a second `xce_core_now()` read.
|
||||
- `src/scheduler/src/scheduler.c` — after `make_alloc()` at `:1893`, set `allocs[i]->from = sched->core.session_start / 1000000000ULL`.
|
||||
- `src/scheduler/src/sim_verify.c` — delete the five `static` comparators; `sim_decision_eq` gains an optional `char** why` and calls `job_alloc_eq` for DISPATCH; keep a simulator-local `static` main-reason-only helper for PEND; `sim_log_mismatch` asks `sim_decision_eq` for the description, appends it, and frees it. Update the header comments that describe `from` as excluded.
|
||||
- `src/scheduler/src/sim_loop.c` — pass the description through (its `extern` declaration of `sim_decision_eq` gets the new parameters).
|
||||
- `src/scheduler/src/scheduler.c` — in `sched_make_primary_alloc_(core, ctx)`, when the allocator returns `rc > 0` with `ctx->alloc != NULL`, set `from = core->session_start / 1000000000ULL` on every `xce_alloc_t` in `ctx->alloc` (per-host and shared). Not in `sched_make_allocs()`: in simulation its `sched` is `sim_alloc.c`'s wrapper, whose core has no session.
|
||||
- `src/scheduler/src/sim_verify.c` — delete the five `static` comparators; `sim_decision_eq` gains an optional `char** why` and calls `job_alloc_eq` for DISPATCH; keep a simulator-local `static` main-reason-only helper for PEND; `sim_log_mismatch` gains a `const char* why` parameter and appends it. Update the header comments that describe `from` as excluded.
|
||||
- `src/scheduler/src/sim_loop.c` — update both `extern` declarations; in the per-decision loop call `sim_decision_eq(expected, d, &why)`, pass `why` to `sim_log_mismatch`, then `free(why)`; the leftover-recorded loop (recorded decisions with no live counterpart) passes the string literal `"no live decision"`, which it does not free.
|
||||
- `src/lib/testsuite/test_xcealloc.c`, `src/lib/testsuite/test_preason.c` — unit tests.
|
||||
- `src/scheduler/testsuite/test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c` — replace direct calls with `job_alloc_eq(..., NULL)`/`job_preason_eq(..., NULL)` (or take the description, print it on failure, and free it); add `#include "xcealloc.h"`/`#include "preason.h"`; comment at each `job_preason_eq` call why full match is valid (Decision 7); keep the white-box `#include "../src/sim_verify.c"`.
|
||||
- `src/scheduler/testsuite/test_sim_verify.c` — drop the DISPATCH alloc-level tests now owned by libxce; replace the "`from` is ignored" test with "`from` from the same session matches"; add a test that the mismatch log names the differing field; keep PEND/decision-level and integration tests.
|
||||
- A scheduler test (new or in an existing session/allocation test file) — allocations made in one session carry `session_start / 1e9` as `from`, and the recorded `beginRound` equals `session_start`.
|
||||
- `src/scheduler/testsuite/test_sim_verify.c` — add the `why` argument (`NULL`) to its 16 direct `sim_decision_eq()` calls; drop the DISPATCH alloc-level tests now owned by libxce; invert `test_dispatch_decisions_differing_from_timestamp_still_match` (a `from` difference is now a mismatch, and `why` names `from`); add a test that the mismatch description names the differing field; keep PEND/decision-level and integration tests.
|
||||
- Hand-built recorded DISPATCH fixtures that `--sim-verify` or an equivalence test compares against a live replay — they build the recorded allocation with `make_alloc()` (`from = time(NULL)`) or a literal `from`, which no longer matches the replay's `session_start / 1e9`: set the recorded `from` to the fixture session's `begin_round / 1000000000`. Audit every test with a hand-built `ReplayDecision_body_dispatch`: `test_sim_live_allocation.c` (the `make_alloc()` fixture near line 726, `begin_round = 2000` → `from = 0`), `test_sim_contention_parity.c`, `test_sim_resource_contention.c`, `test_sim_round_boundary_reconciliation.c`, `test_sim_reconcile_unaccounted.c`, `test_sim_verify.c`; fixtures that are never compared against a live allocation need no change.
|
||||
- A scheduler test (new or in an existing session/allocation test file) — allocations made in one session carry `session_start / 1e9` as `from`, including in simulation (the `sim_alloc.c` wrapper path), and the recorded `beginRound` equals `session_start`.
|
||||
- `src/scheduler/testsuite/test_sim_live_allocation.c`, `test_sim_reconcile_unaccounted.c`, and the header comments of `test_sim_equivalence.c`/`test_repl_equivalence.c` — update comments that name `sim_verify_*_eq` or describe `from` as excluded.
|
||||
|
||||
## System Flows
|
||||
|
|
@ -149,7 +152,7 @@ flowchart TD
|
|||
B -->|PREEMPT| E["log + report mismatch (unchanged)"]
|
||||
C --> F["host_alloc_eq per host"] --> G["alloc_eq per resource (incl. from, excl. instid)"]
|
||||
D --> H["main-reason only, detail ignored (bug-1845)"]
|
||||
C --> M["sim_log_mismatch appends *why, then free()"]
|
||||
C --> M["sim_loop: sim_log_mismatch(..., why); free(why)"]
|
||||
D --> M
|
||||
```
|
||||
|
||||
|
|
@ -159,12 +162,13 @@ flowchart TD
|
|||
sequenceDiagram
|
||||
participant Drv as driver.c
|
||||
participant Core as core_session.c
|
||||
participant Sch as scheduler.c
|
||||
participant Sch as scheduler.c sched_make_primary_alloc_(core, ctx)
|
||||
participant Rec as recording
|
||||
Drv->>Core: xce_core_begin_session()
|
||||
Core-->>Core: session_start = xce_core_now()<br/>(replay: == recorded beginRound, via scheduler-sim-discrete-clock)
|
||||
Drv->>Rec: beginRound = session_start
|
||||
Sch->>Sch: make_alloc(...); from = session_start / 1e9
|
||||
Sch->>Sch: allocator builds ctx->alloc (make_alloc: from = time(NULL))
|
||||
Sch->>Sch: every alloc in ctx->alloc: from = core->session_start / 1e9
|
||||
```
|
||||
|
||||
## Requirements Traceability
|
||||
|
|
@ -176,11 +180,11 @@ sequenceDiagram
|
|||
| 1.5–1.7 | NULL handling, malformed-record precondition, read-only | all four functions | const params |
|
||||
| 2.1–2.3 | pending-reason full structural equality | `preason.c` | `job_preason_eq` |
|
||||
| 3.1–3.6 | first-difference description, caller-owned | all four functions | `char** why` |
|
||||
| 4.1–4.2, 4.4–4.5 | reproducible `from` (normal mode) | `driver.c`, `scheduler.c` | `core->session_start` |
|
||||
| 4.1–4.2, 4.4–4.5 | reproducible `from` | `driver.c`, `scheduler.c` `sched_make_primary_alloc_()` | `core->session_start` |
|
||||
| 4.3 | replayed session start == recorded `beginRound` | spec `scheduler-sim-discrete-clock` | — |
|
||||
| 5.1 | simulator uses libxce allocation comparison | `sim_verify.c` | `job_alloc_eq` |
|
||||
| 5.2 | simulator-local main-reason-only PEND check | `sim_verify.c` | local static helper |
|
||||
| 5.3 | mismatch log names the difference (bug-1881) | `sim_verify.c`, `sim_loop.c` | `sim_decision_eq(..., why)`, `sim_log_mismatch` |
|
||||
| 5.3 | mismatch log names the difference (bug-1881) | `sim_verify.c`, `sim_loop.c` | `sim_decision_eq(..., why)`, `sim_log_mismatch(..., why)` |
|
||||
| 5.4 | zero-mismatch self-replay | integration tests | — |
|
||||
| 6.1–6.3 | single shared implementation; test files use full-match preason; include stays | `sim_verify.c` + 3 test files | public API |
|
||||
| 6.4–6.6 | libxce + scheduler tests, no other behavior change | test files | — |
|
||||
|
|
@ -243,11 +247,12 @@ extern int str_eq(const char* a, const char* b);
|
|||
- Simulation: no change here. Spec `scheduler-sim-discrete-clock` guarantees `core->session_start == begin_round` for a replayed session (clock frozen between recorded timestamps).
|
||||
|
||||
#### `from` stamping (`scheduler.c`)
|
||||
- After `make_alloc()` at `:1893`: `allocs[i]->from = sched->core.session_start / 1000000000ULL` (seconds, floor).
|
||||
- In `sched_make_primary_alloc_(core, ctx)`, after the allocator returns `rc > 0`: a small `static` helper walks `ctx->alloc` (every `allocs[i]->allocs[j]` and every `allocs_shared[k]`, skipping NULL entries) and sets `from = core->session_start / 1000000000ULL` (seconds, floor). A COMPOUND's primaries are each stamped before `merge_job_alloc()` folds them, and all carry the same session value, so the merged allocation is consistent.
|
||||
- Why here and not in `sched_make_allocs()`: `core` is the session core in both modes; `sched_make_allocs()`'s `sched` is `sim_alloc.c`'s session-less wrapper in simulation.
|
||||
|
||||
#### Simulator verification (`sim_verify.c`)
|
||||
- `int sim_decision_eq(const replay_decision_t* expected, const xce_decision_t* actual, char** why)`: DISPATCH calls `job_alloc_eq(..., why)`; PEND sets `*why` to `jobid A != B` or `main_reason A != B`; kind mismatch sets `kind DISPATCH != PEND`. Same ownership contract as the libxce functions.
|
||||
- `sim_log_mismatch` calls `sim_decision_eq(expected, actual, &why)`, appends ` -- diff: <why>` to its existing line (or ` -- diff: <unavailable>` if `why` is NULL), then `free(why)`.
|
||||
- `int sim_decision_eq(const replay_decision_t* expected, const xce_decision_t* actual, char** why)`: DISPATCH calls `job_alloc_eq(..., why)`; PEND sets `*why` to `jobid A != B` or `main_reason A != B`; kind mismatch sets `kind DISPATCH != PEND`; a NULL side sets `no recorded decision at this index` / `no live decision`; PREEMPT sets `PREEMPT comparison not supported`. Same ownership contract as the libxce functions.
|
||||
- `void sim_log_mismatch(uint64_t begin_round, int decision_index, const replay_decision_t* expected, const xce_decision_t* actual, const char* why)`: appends ` -- diff: <why>` to its existing line when `why != NULL`, otherwise the line is unchanged. It does not compare again and does not free `why`; `sim_loop.c`, which already calls `sim_decision_eq()` right before it, owns and frees the string.
|
||||
|
||||
## Data Models
|
||||
|
||||
|
|
@ -262,18 +267,19 @@ No struct is added or modified. The equality contract and the description format
|
|||
- string: `allocs[0](host=h0).host: "h0" != "h2"`
|
||||
- NULL: `allocs_shared[0]: NULL != non-NULL`
|
||||
- length: `nhosts: 1 != 2`
|
||||
- path labels (`host=`, `resname=`) are taken from the first argument (`a`), or from `b` when `a`'s entry or string is NULL, or `?` when both are.
|
||||
- `jobid` formatted with libxce's own `fmt_jobid()` (`utils.h`), so it reads like the rest of the scheduler log; `from` as integer seconds; `value` with `%.17g` so an exact-compare difference is always visible.
|
||||
|
||||
## Error Handling
|
||||
|
||||
These are read-only functions with no I/O and no failure path for the comparison itself. The only allocation is the optional description; if it fails, `*why` is NULL and the result is unchanged. The only "error" case is NULL input, which is handled by the shared convention (identical pointers including both-NULL → equal; exactly one NULL → not-equal) rather than an error return. A malformed record (non-zero count, NULL array) is a documented precondition violation, not a handled case. No logging is added; the description is built with `vasprintf` (glibc, already used under `_GNU_SOURCE`), and returned to the caller.
|
||||
These are read-only functions with no I/O and no failure path for the comparison itself. The only allocation is the optional description; if it fails, `*why` is NULL and the result is unchanged (`vasprintf` leaves its output undefined on failure, so the implementation sets `*why = NULL` explicitly). A nested call's description is re-wrapped with the parent's path segment and the inner string freed. The only "error" case is NULL input, which is handled by the shared convention (identical pointers including both-NULL → equal; exactly one NULL → not-equal) rather than an error return. A malformed record (non-zero count, NULL array) is a documented precondition violation, not a handled case. No logging is added; the description is built with `vasprintf` (glibc; `_GNU_SOURCE` defined at the top of `xcealloc.c`/`preason.c`) and returned to the caller.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
- **Unit (libxce)**: `test_xcealloc.c` — match/mismatch on each compared field with its expected description, `instid` ignored, `from` compared, ordered-list sensitivity, list-length mismatch, NULL handling at top level, nested entries, and string fields, NULL `why`, `*why == NULL` on equal, a failed description allocation (`--wrap` on `vasprintf` or `malloc` in a dedicated test binary: `*why` NULL, same result); every test frees the returned description, so ASAN/LeakSanitizer proves ownership; an existing `merge_job_alloc` test still passes. `test_preason.c` — full-match: main-reason, host-based and resource-based lists per field + order + length, NULL handling, description cases; plus `str_eq` NULL/NULL and NULL/non-NULL.
|
||||
- **Unit (scheduler)**: `test_sim_verify.c` keeps `sim_decision_eq`-level tests (DISPATCH/PEND match and mismatch, kind mismatch, PREEMPT always-false, NULL defense), asserts the mismatch description, and drops the alloc-level tests now owned by libxce. A session-level test asserts every allocation of one session (including a COMPOUND merge) carries `session_start / 1e9`, and that the recorded `beginRound` equals `session_start`.
|
||||
- **Integration (scheduler)**: `test_sim_verify.c`'s `--sim-verify` round-trips (zero-mismatch self-replay, exact-mismatch altered replay, reconcile cases); the self-replay reports zero mismatches. Until `scheduler-sim-discrete-clock` lands, run them at `--sim-speed 0`.
|
||||
- **Regression**: `test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c` stay green under the stricter checks (full-match preason, `from` compared). A failure there is investigated as a real divergence.
|
||||
- **Unit (scheduler)**: `test_sim_verify.c` keeps `sim_decision_eq`-level tests (DISPATCH/PEND match and mismatch, kind mismatch, PREEMPT always-false, NULL defense), asserts the description for each, and drops the alloc-level tests now owned by libxce. A session-level test asserts every allocation of one session (including a COMPOUND merge and the simulation wrapper path) carries `session_start / 1e9`, and that the recorded `beginRound` equals `session_start`.
|
||||
- **Integration (scheduler)**: `test_sim_verify.c`'s `--sim-verify` round-trips (zero-mismatch self-replay, exact-mismatch altered replay, reconcile cases); the self-replay reports zero mismatches. These tests, and `test_sim_equivalence.c`/`test_repl_equivalence.c`/`test_repl_limit_table.c`/`test_sim_live_allocation.c`, already run the simulator at `sim_speed = 0` (clock frozen), so `session_start == begin_round` holds for them before `scheduler-sim-discrete-clock` lands; only the live `--sim-verify` check at the default speed waits for it.
|
||||
- **Regression**: `test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c` stay green under the stricter checks (full-match preason, `from` compared); the hand-built recorded DISPATCH fixtures listed in the File Structure Plan are updated to carry the session's `from`. A failure after that is investigated as a real divergence.
|
||||
- **Live**: record a session on a local cluster, replay it with `--sim-verify` at the default speed: zero mismatches.
|
||||
|
||||
## Design Decisions
|
||||
|
|
@ -308,7 +314,7 @@ These are read-only functions with no I/O and no failure path for the comparison
|
|||
- **Context**: with `job_preason_eq` now a full match, both `xcealloc.c` (host/resname/reserve strings) and `preason.c` (host/resource strings) need NULL-safe string comparison. Keeping it `static` in one file forces the other to duplicate it.
|
||||
- **Alternatives**: (a) shared `str_eq` in `utils.{c,h}` (selected); (b) duplicate a `static` helper in each of the two files.
|
||||
- **Selected**: add `int str_eq(const char* a, const char* b)` to `utils.{c,h}`, beside the existing `float_equal` helper.
|
||||
- **Rationale**: two consumers and a third on the horizon (controller) justify a shared helper. `utils.h` already centralizes small generic C helpers (`fmt_jobid`, `float_equal`). It could also make `same_alloc_slot()` NULL-safe when bug-1882 is fixed.
|
||||
- **Rationale**: two consumers and a third on the horizon (controller) justify a shared helper. `utils.h` already centralizes small generic C helpers (`fmt_jobid`, `float_equal`).
|
||||
- **Trade-offs**: a new public symbol in `utils.h`; negligible surface.
|
||||
|
||||
### Decision 6: `alloc_eq` and `same_alloc_slot` stay separate
|
||||
|
|
@ -316,7 +322,7 @@ These are read-only functions with no I/O and no failure path for the comparison
|
|||
- **Alternatives**: (a) keep them separate and document the difference (selected); (b) implement `same_alloc_slot` on top of `alloc_eq` or vice versa.
|
||||
- **Selected**: keep them separate. `alloc_eq` answers "do these two records describe the same allocation decision" (resname, reserve, value, duration, from; `instid` excluded). `same_alloc_slot` answers "are these the same reservation slot to accumulate into" (resname, instid, reserve; `value` excluded because merging adds values). The `alloc_eq` header comment names the difference.
|
||||
- **Rationale**: the two predicates disagree on `instid`, `value`, `duration`, and `from`, so neither can be expressed through the other without changing behavior. `merge_job_alloc()` is out of scope.
|
||||
- **Follow-up**: `same_alloc_slot` calls `strcmp(a->resname, …)` without a NULL check, and `resname` is NULL when `make_alloc()`'s unchecked `strdup()` fails; tracked as bug-1882, not part of this spec.
|
||||
- **Follow-up**: none. bug-1882 (a NULL `resname` reaching `same_alloc_slot()`'s `strcmp()` after a failed `strdup()`) is fixed in f9146864: `make_alloc()`/`make_host_alloc()` no longer return records with NULL strings.
|
||||
|
||||
### Decision 7: The three test files adopt the full-match `job_preason_eq`
|
||||
- **Context**: `test_sim_equivalence.c`, `test_repl_equivalence.c`, and `test_repl_limit_table.c` compare two decoded `replay_decision_t` entries and today call the main-reason-only `sim_verify_job_preason_eq`.
|
||||
|
|
@ -328,9 +334,9 @@ These are read-only functions with no I/O and no failure path for the comparison
|
|||
### Decision 8: `from` is the deciding session's start, from one clock reading
|
||||
- **Context**: `alloc_eq` must compare `from`, but `make_alloc()` stamps `time(NULL)`, which a replay cannot reproduce (bug-1838-followup). The scheduler already has a virtualized session start (`core->session_start`), but it is a different clock read from the recorded `beginRound`, and in simulation at `--sim-speed` ≥ 1 it drifts past the recorded `beginRound`.
|
||||
- **Alternatives**: (a) `from = session_start / 1e9`, with the recorded `beginRound` taken from `session_start`; the simulator clock stops drifting (spec `scheduler-sim-discrete-clock`) so a replayed `session_start` is exactly the recorded `beginRound` (selected); (a′) as (a), but with an explicit `xce_core_begin_session_at(core, begin_round)` in the simulator instead of removing the drift (superseded: the user chose to remove `--sim-speed`, which fixes the drift for every clock reader, not just `from`); (b) keep excluding `from` from equality (rejected by the user: `from` is part of what was decided); (c) `from = xce_core_now()` at allocation time (rejected: at `--sim-speed` ≥ 1 it drifts with replay pace, and a second-boundary crossing produces a false mismatch); (d) change `make_alloc()`'s signature to take `from` (rejected: churns the controller and ~20 test callers for no benefit; the scheduler is the only producer that needs a different value).
|
||||
- **Selected**: (a). The recording's `beginRound` is taken from `core->session_start`; `scheduler.c` overwrites `from` after `make_alloc()`; the simulator side comes from `scheduler-sim-discrete-clock`.
|
||||
- **Selected**: (a). The recording's `beginRound` is taken from `core->session_start`; `sched_make_primary_alloc_(core, ctx)` overwrites `from` on the allocator's result using the session core (not `sched_make_allocs()`, whose `sched` is a session-less wrapper in simulation); the simulator side comes from `scheduler-sim-discrete-clock`.
|
||||
- **Rationale**: every allocation of one session gets the same `from`, which a replay reproduces bit-for-bit. `from` changes meaning only slightly — from "second the record was built" to "second the deciding session started", normally the same second and at most one session's duration earlier.
|
||||
- **Trade-offs**: recorded `beginRound` moves a few milliseconds earlier (to `xce_core_begin_session`, before `plugin_begin_session`/`sched_account_jobs`). Recordings made before this change carry wall-clock `from` values and may show a false DISPATCH mismatch when an allocation's `time(NULL)` fell in a later second than its session start; the difference description names `from`, so these are recognisable. `make_alloc()` keeps `time(NULL)` for its other callers.
|
||||
- **Trade-offs**: hand-built test fixtures that stand in for a recording must now carry the session's `from`. Recorded `beginRound` moves a few milliseconds earlier (to `xce_core_begin_session`, before `plugin_begin_session`/`sched_account_jobs`). Recordings made before this change carry wall-clock `from` values and may show a false DISPATCH mismatch when an allocation's `time(NULL)` fell in a later second than its session start; the difference description names `from`, so these are recognisable. `make_alloc()` keeps `time(NULL)` for its other callers.
|
||||
|
||||
### Decision 9: First-difference description returned through `char** why`, owned by the caller
|
||||
- **Context**: a yes/no comparator leaves a mismatch undiagnosable (bug-1881).
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ The decision-level dispatcher (`sim_decision_eq`), its main-reason-only pending-
|
|||
## Boundary Context
|
||||
|
||||
- **In scope**: moving the three allocation comparators from the scheduler simulator into public libxce API with a documented "same allocation decision" contract that compares `from` and excludes only `instid`; adding a new full-structural-match pending-reason comparator to libxce; an optional out-parameter on every comparator that returns a newly allocated description of the first difference, owned by the caller; stamping `from` with the scheduling session's start time, taken from the one timestamp that is also recorded as the session's `beginRound`; using the difference description in `--sim-verify` mismatch logs (resolves bug-1881); migrating the simulator and the three scheduler test files that call the static comparators to the public API; libxce unit tests for the new functions; updating comments that name the removed `sim_verify_*_eq` functions.
|
||||
- **Out of scope**: moving the decision-level comparison dispatcher (`sim_decision_eq`) or mismatch reporting (`sim_log_mismatch` and its describe helpers) out of the scheduler; changing the simulator's main-reason-only pending-decision check (bug-1845); `merge_job_alloc()` and its `same_alloc_slot()` helper in `xcealloc.c` (bug-1863), which answer a different question — whether two records are the same reservation slot to merge, where `instid` *is* significant — and are neither replaced by nor reimplemented on top of `alloc_eq` (their NULL-dereference defect is bug-1882); removing the three test files' white-box `#include "../src/sim_verify.c"`, which they still need because they also white-box-include `sim_loop.c`, which calls `sim_decision_eq`/`sim_log_mismatch`; any new comparison call site beyond the existing consumers, including the controller; any change to how the controller or worker interpret `from`; any change to the `.replay` wire format; removing `--sim-speed` and making the simulator clock move only at recorded event and round timestamps, which is the separate spec `scheduler-sim-discrete-clock`.
|
||||
- **Out of scope**: moving the decision-level comparison dispatcher (`sim_decision_eq`) or mismatch reporting (`sim_log_mismatch` and its describe helpers) out of the scheduler; changing the simulator's main-reason-only pending-decision check (bug-1845); `merge_job_alloc()` and its `same_alloc_slot()` helper in `xcealloc.c` (bug-1863), which answer a different question — whether two records are the same reservation slot to merge, where `instid` *is* significant — and are neither replaced by nor reimplemented on top of `alloc_eq` (their NULL-dereference defect, bug-1882, is already fixed); removing the three test files' white-box `#include "../src/sim_verify.c"`, which they still need because they also white-box-include `sim_loop.c`, which calls `sim_decision_eq`/`sim_log_mismatch`; any new comparison call site beyond the existing consumers, including the controller; any change to how the controller or worker interpret `from`; any change to the `.replay` wire format; removing `--sim-speed` and making the simulator clock move only at recorded event and round timestamps, which is the separate spec `scheduler-sim-discrete-clock`.
|
||||
- **Adjacent expectations**: this feature depends on spec `scheduler-sim-discrete-clock` for Requirement 4.3 (a replayed session starts at exactly the recorded `beginRound`); the `.replay` wire format and the `replay_decision_t`/`xce_decision_t` decision types are unchanged; the `xce/types.h` struct layouts are unchanged, and `from` keeps its unit (seconds since the Unix epoch); libxce's deep-copy and merge helpers (`dup_job_alloc`, `merge_job_alloc`) keep copying `from` unchanged; the controller's Cap'n Proto persistence round-trips for these structures are unchanged. Recordings made before this change carry wall-clock `from` values; replaying them with `--sim-verify` may report a DISPATCH mismatch when an allocation's original `time(NULL)` fell in a later second than its session's start. That is an accepted limitation for old recordings, and the new difference description names `from` as the cause.
|
||||
|
||||
## Requirements
|
||||
|
|
@ -67,7 +67,7 @@ The decision-level dispatcher (`sim_decision_eq`), its main-reason-only pending-
|
|||
#### Acceptance Criteria
|
||||
1. The simulator shall compare a recorded DISPATCH decision against a live one with the libxce allocation comparison (Requirement 1), which now also compares `from`, rather than defining its own.
|
||||
2. The simulator shall keep its pending-decision comparison as a simulator-local main-reason-only check, and shall not use the full-match `job_preason_eq`, because a recorded cache-hit pending reason carries no host/resource detail while a live replayed one always does (bug-1845). This asymmetry is a simulator concern, not a property of pending-reason equality.
|
||||
3. When the simulator reports a verification mismatch, the log line shall keep its existing logical-time and decision context and shall add the description of the first difference (Requirement 3), or, for a PEND mismatch, the two jobids or the two main reason codes. This resolves bug-1881.
|
||||
3. When the simulator reports a verification mismatch, the log line shall keep its existing logical-time and decision context and shall add the description of the first difference (Requirement 3), or, for a PEND mismatch, the two jobids or the two main reason codes, or, when one side has no decision, which side is missing. Only if allocating the description fails (Requirement 3.5) may the line be logged without it. This resolves bug-1881.
|
||||
4. Replaying a recording made after this change against its own previously-recorded output with verification enabled shall report zero mismatches.
|
||||
|
||||
### Requirement 6: Single shared implementation across all consumers
|
||||
|
|
@ -75,7 +75,7 @@ The decision-level dispatcher (`sim_decision_eq`), its main-reason-only pending-
|
|||
|
||||
#### Acceptance Criteria
|
||||
1. The simulator and the three scheduler test files that currently call the static comparators (`test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c`) shall obtain the allocation comparison from the libxce public API, and no scheduler source or test shall retain a copy of it. The simulator's main-reason-only pending-decision check (Requirement 5, criterion 2) is the single documented simulator-local pending-reason comparison.
|
||||
2. The three test files shall compare pending reasons with the full-match `job_preason_eq`. This makes their pending-decision checks stricter than today's main-reason-only comparison. The change is valid because both sides of each of their comparisons are produced without the scheduler's unsatisfiable-allocation cache (none of these tests registers `unsatisfiable_lookup_cb`), so the detail lists are built by the same full per-host computation on both sides and are expected to match; each test file shall document this reasoning where it calls `job_preason_eq`. Their allocation comparisons also become stricter (`from` is compared). If a test fails after either change, the failure shall be investigated as a real divergence, not resolved by loosening the comparison.
|
||||
2. The three test files shall compare pending reasons with the full-match `job_preason_eq`. This makes their pending-decision checks stricter than today's main-reason-only comparison. The change is valid because both sides of each of their comparisons are produced without the scheduler's unsatisfiable-allocation cache (none of these tests registers `unsatisfiable_lookup_cb`), so the detail lists are built by the same full per-host computation on both sides and are expected to match; each test file shall document this reasoning where it calls `job_preason_eq`. Their allocation comparisons also become stricter (`from` is compared). Hand-built test fixtures that stand in for a recording shall carry the recorded session's start time as `from` (Requirement 4.1), like a real recording. If a test fails after these changes, the failure shall be investigated as a real divergence, not resolved by loosening the comparison.
|
||||
3. A consumer shall be able to call the public comparators by including `xcealloc.h`/`preason.h` and linking libxce, without white-box-including any scheduler source. The three test files may keep their existing white-box `#include "../src/sim_verify.c"`, since they need it for the decision-level functions that `sim_loop.c` calls, but shall no longer call any comparator that is private to `sim_verify.c`.
|
||||
4. The library shall provide unit test coverage for each new equality function, covering: a match; a mismatch on each compared field, with the expected difference description; that an `instid` difference does not affect allocation equality; that a `from` difference does; that list order matters (the same entries in a different order compare not-equal); a list-length mismatch; the NULL-handling cases at the top level and at nested entries and string fields; a NULL `why`; `*why` set to NULL on equal; and a failed description allocation (`*why` NULL, same result).
|
||||
5. The scheduler shall have tests showing that allocations made in one session carry that session's start time as `from`, and that the recorded `beginRound` equals that start time.
|
||||
|
|
|
|||
|
|
@ -1,24 +1,24 @@
|
|||
{
|
||||
"feature_name": "libxce-alloc-preason-equality",
|
||||
"created_at": "2026-09-24T04:42:20Z",
|
||||
"updated_at": "2026-09-28T08:40:00Z",
|
||||
"updated_at": "2026-09-28T09:10:25Z",
|
||||
"language": "en",
|
||||
"phase": "tasks-generated",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
},
|
||||
"design": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
},
|
||||
"tasks": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
}
|
||||
},
|
||||
"ready_for_implementation": false,
|
||||
"ready_for_implementation": true,
|
||||
"gitea": {
|
||||
"milestone_id": 30,
|
||||
"issues": {
|
||||
|
|
|
|||
|
|
@ -1,78 +1,86 @@
|
|||
# Implementation Plan
|
||||
|
||||
## Task Format
|
||||
Test-first throughout (red → green → refactor). Every sub-task leaves the build and all test suites green: consumers move to the public API before the private comparators are deleted, test fixtures are brought in line before `from` starts being compared, and each change updates the tests it affects.
|
||||
|
||||
Work is sequenced test-first (TDD: red → green → refactor). New library API gets its unit tests first; the `from` stamping change gets a failing scheduler test first; the scheduler-side migration is then verified by the existing and extended `--sim-verify` tests.
|
||||
- [ ] 1. Foundation: shared NULL-safe string equality
|
||||
- [ ] 1.1 Provide a NULL-safe string-equality helper in libxce
|
||||
- Write the tests first: two NULL strings are equal; one NULL and one non-NULL are not equal in either order; equal strings are equal; different strings are not.
|
||||
- Add the helper beside the existing floating-point comparison helper in the shared utilities, as described in design.md (Decision 5).
|
||||
- Done when the libxce utility tests include these cases and the libxce suite passes under ASAN.
|
||||
- _Requirements: 1.5, 2.3_
|
||||
|
||||
---
|
||||
- [ ] 2. Core: public equality API in libxce
|
||||
- [ ] 2.1 Allocation equality for single records, host allocations and job allocations, with an optional first-difference description
|
||||
- Write the tests first: a match; a mismatch on each compared field (resource name, reserve, value, duration, reservation start time) with the expected description; an allocation-instance-id difference still compares equal; host name, entry order and list length matter; job id, ordered hosts, ordered shared resources and list-length mismatches; NULL handling at the top level, in list entries and in string fields; no description requested; no description returned when equal.
|
||||
- Add a wrapped description-allocation failure to the existing allocation OOM test binary (bug-1882) and test that a failed description leaves no description and the same result.
|
||||
- Implement the three comparisons in the allocation module with the field order, exact value comparison, description path format and ownership contract of design.md (Data Models, Components: Allocation equality, Decision 9), enabling the GNU extension the description formatter needs.
|
||||
- Document in the public header: "same allocation decision" equality, reservation start time compared and instance id excluded (and why), the difference from the merge helper's slot matching, NULL/order semantics, the malformed-record precondition, and that the caller frees the description.
|
||||
- Leave the allocation constructors and the merge helper unchanged.
|
||||
- Done when the new tests pass, the existing allocation and allocation-OOM tests still pass, and every test frees the descriptions it receives (clean under ASAN/LeakSanitizer).
|
||||
- _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 3.1, 3.2, 3.3, 3.4, 3.5, 3.6, 6.4, 6.6_
|
||||
- _Boundary: libxce allocation module_
|
||||
- [ ] 2.2 Full-match pending-reason equality with an optional first-difference description
|
||||
- Write the tests first: a match; main-reason mismatch; host-based reason list mismatch per field, order and length; resource-based reason list mismatch per field, order and length; NULL handling at the top level, in list entries and in string fields; each with the expected description; no description requested; no description returned when equal; a failed description allocation (in the allocation OOM test binary, which already wraps the formatter after 2.1) leaves no description and the same result.
|
||||
- Implement the full structural match in the pending-reason module per design.md (Components: Pending-reason equality, Decision 3), enabling the GNU extension the description formatter needs.
|
||||
- Document in the public header the full-match contract, NULL/order semantics, the malformed-record precondition, the description ownership, and that the simulator deliberately uses a main-reason-only check instead (bug-1845).
|
||||
- Done when the new tests pass and the libxce suite passes under ASAN with leak detection.
|
||||
- _Requirements: 2.1, 2.2, 2.3, 3.1, 3.2, 3.3, 3.4, 3.5, 3.6, 6.4_
|
||||
- _Boundary: libxce pending-reason module; the allocation OOM test binary (shared test harness, sequenced after 2.1)_
|
||||
- _Depends: 2.1_
|
||||
|
||||
- [ ] 1. Add a shared NULL-safe string helper to libxce (`utils.c`/`utils.h`)
|
||||
- Add `int str_eq(const char* a, const char* b)` beside the existing `float_equal` helper: returns non-zero iff both arguments are NULL, or both non-NULL and `strcmp(a, b) == 0`.
|
||||
- Test it in `test_preason.c` or a `utils` test: NULL/NULL, NULL/non-NULL, equal, different.
|
||||
- _Requirements: 1.5, 2.3 (supporting)_
|
||||
- _Boundary: utils.c, utils.h_
|
||||
- [ ] 3. Core: reproducible reservation start time in the scheduler
|
||||
- [ ] 3.1 (P) Record each session's start from the same clock reading as the session start
|
||||
- Write the test first, in its own test function (not shared with 3.2): with recording enabled, a recorded session's begin time equals the session's start time exactly.
|
||||
- Take the recorded begin time from the session start instead of a second clock reading (design.md, Components: Recording).
|
||||
- Done when the test passes and the existing recording tests still pass.
|
||||
- _Requirements: 4.2, 6.5_
|
||||
- _Boundary: scheduler normal-mode driver_
|
||||
- [ ] 3.2 (P) Stamp every allocation with its deciding session's start time, from the session core
|
||||
- Write the tests first, in test functions separate from 3.1's: every allocation of one normal-mode session, including every primary of a merged COMPOUND request, carries the session start in whole seconds; the same holds in simulation mode, where the allocators' scheduler handle is the simulator's session-less wrapper, using a realistic epoch-scale recorded session start so a stamp taken from the wrong core (0) fails.
|
||||
- Stamp the reservation start time on every per-host and shared allocation record where the allocator result is received with the session core, not in the record-building helper (design.md, Components: `from` stamping, Decision 8).
|
||||
- Confirm no allocation path bypasses that point; if one does, stop and raise it.
|
||||
- The simulator still ignores the reservation start time at this point, so no fixture changes are needed yet.
|
||||
- Done when the new tests pass and the full scheduler suite passes.
|
||||
- _Requirements: 4.1, 4.4, 4.5, 6.5, 6.6_
|
||||
- _Boundary: scheduler allocation chain_
|
||||
|
||||
- [ ] 2. Allocation equality API with first-difference description (`xcealloc.c`/`xcealloc.h`), test-first
|
||||
- RED: in `test_xcealloc.c`, add tests for `alloc_eq`/`host_alloc_eq`/`job_alloc_eq`: match; mismatch per field (`resname`, `reserve`, `value`, `duration`, `from`) with the expected description in `*why` (freed by the test); `instid` difference still equal; host name, order and list-length sensitivity; `jobid`, ordered hosts, ordered shared, length mismatch; NULL at top level, nested entries and string fields; NULL `why`; `*why == NULL` on equal. In a dedicated test binary with `-Wl,--wrap=vasprintf` (bug-1843's `--wrap` idiom), a failed description allocation leaves `*why` NULL with the same result.
|
||||
- GREEN: implement the three functions with signature `(a, b, char** why)`, comparing in the order given in design.md Data Models, `value` with exact `==` (not `float_equal`), strings via `str_eq`, descriptions via `vasprintf` in the design's path format (`jobid` via `fmt_jobid`, `value` with `%.17g`); a nested call's description is prefixed with the parent's path segment and the inner string freed.
|
||||
- Declare them in `src/include/xcealloc.h` with a doc comment covering: "same allocation decision" equality, `from` compared and `instid` excluded (and why), the difference from `merge_job_alloc()`'s slot matching, NULL/order semantics, the malformed-record precondition, and the `why` ownership contract (caller frees with `free()`; `*why` NULL on equal or on allocation failure).
|
||||
- Do not change `make_alloc()`, `merge_job_alloc()` or `same_alloc_slot()`; confirm existing `test_xcealloc.c` tests still pass.
|
||||
- _Requirements: 1.1–1.7, 3.1–3.5, 6.4, 6.6_
|
||||
- _Boundary: xcealloc.c, xcealloc.h, test_xcealloc.c_
|
||||
- _Depends: 1_
|
||||
|
||||
- [ ] 3. Pending-reason equality API with description (`preason.c`/`preason.h`), test-first
|
||||
- RED: in `test_preason.c`, add tests for `job_preason_eq`: match; main-reason mismatch; host-based list mismatch per field, order and length; resource-based list mismatch per field, order and length; NULL handling; each with the expected description (freed by the test); NULL `why`; `*why == NULL` on equal.
|
||||
- GREEN: implement `job_preason_eq(a, b, char** why)` as a full structural match with the shared NULL convention.
|
||||
- Declare it in `src/include/preason.h` with a doc comment covering the full-match contract (nothing excluded), NULL/order semantics, malformed-record precondition, the `why` ownership contract, and that the simulator deliberately uses a main-reason-only check instead (bug-1845).
|
||||
- _Requirements: 2.1–2.3, 3.1–3.5, 6.4_
|
||||
- _Boundary: preason.c, preason.h, test_preason.c_
|
||||
- _Depends: 1_
|
||||
|
||||
- [ ] 4. Reproducible `from`: one session-start reading, stamped on every allocation, test-first
|
||||
- RED: scheduler test(s) asserting (a) every allocation produced in one normal-mode session, including a COMPOUND request merged by `merge_job_alloc()`, has `from == session_start / 1000000000`; (b) with recording on, the recorded `beginRound == session_start`.
|
||||
- GREEN:
|
||||
- `driver.c`: pass `sched->core.session_start` to `sched_record_session_begin()` instead of a second `xce_core_now()` read.
|
||||
- `scheduler.c:1893`: after `make_alloc()`, set `allocs[i]->from = sched->core.session_start / 1000000000ULL`.
|
||||
- Check there is no other scheduler path that builds an `xce_alloc_t` for a decision outside a session; if one exists, stop and raise it (Revalidation Trigger).
|
||||
- Simulation side (Requirement 4.3) is spec `scheduler-sim-discrete-clock`; once it lands, add a test that a replayed allocation's `from` equals the recorded one.
|
||||
- _Requirements: 4.1, 4.2, 4.4, 4.5, 6.5_
|
||||
- _Boundary: driver.c, scheduler.c, new/extended scheduler test_
|
||||
|
||||
- [ ] 5. Migrate the simulator to the public API and put the difference in the mismatch log (`sim_verify.c`, `sim_loop.c`)
|
||||
- Delete the five `static` comparators (`sim_verify_str_eq`, `sim_verify_alloc_eq`, `sim_verify_host_alloc_eq`, `sim_verify_job_alloc_eq`, `sim_verify_job_preason_eq`) and their `from`-exclusion comments.
|
||||
- Add a `static` simulator-local `sim_verify_pend_outcome_eq(a, b, char** why)` comparing `main_reason` only, documented with the bug-1845 rationale.
|
||||
- Change `sim_decision_eq` to `(expected, actual, char** why)`: DISPATCH → `job_alloc_eq(..., why)`; PEND → jobid equality plus the local main-reason check, describing whichever differs; kind mismatch described too. Update `sim_loop.c`'s `extern` declaration and call.
|
||||
- `sim_log_mismatch`: get the description from `sim_decision_eq`, append ` -- diff: <why>` to the existing line, then `free()` it (resolves bug-1881).
|
||||
- Add `#include "xcealloc.h"`; keep the `describe_*` helpers.
|
||||
- _Requirements: 5.1, 5.2, 5.3_
|
||||
- _Boundary: sim_verify.c, sim_loop.c_
|
||||
- _Depends: 2, 4_
|
||||
|
||||
- [ ] 6. Migrate the three white-box test consumers to the public API
|
||||
- `test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c`: replace `sim_verify_job_alloc_eq` → `job_alloc_eq` and `sim_verify_job_preason_eq` → `job_preason_eq` (take the description, print it in the failure message and free it, or pass `NULL`); add the two headers.
|
||||
- Keep each file's white-box `#include "../src/sim_verify.c"`: the files also include `sim_loop.c`, which calls `sim_decision_eq`/`sim_log_mismatch`.
|
||||
- The switch to full-match `job_preason_eq` is valid because none of these tests registers `unsatisfiable_lookup_cb`, so both sides build detail lists by the same full per-host computation; add a short comment at each `job_preason_eq` call stating this. `from` is now compared too. If a test fails after either change, investigate it as a real divergence; do not loosen the comparison.
|
||||
- Update comments that name `sim_verify_*_eq` or describe `from` as excluded in these files and in `test_sim_live_allocation.c` (~702, ~718) and `test_sim_reconcile_unaccounted.c` (~550).
|
||||
- [ ] 4. Integration: move the consumers to the public API
|
||||
- [ ] 4.1 Move the three white-box equivalence tests to the public comparators
|
||||
- Switch the three test files that call the simulator's private comparators to the public allocation and full-match pending-reason comparisons, printing the description in their failure messages and freeing it.
|
||||
- Keep their white-box inclusion of the simulator's verification source, which they still need for the decision-level functions.
|
||||
- At each full-match pending-reason call, add a short comment on why full match is valid there (no unsatisfiable-allocation cache on either side).
|
||||
- Update their header comments that name the private comparators.
|
||||
- Done when the three tests pass unchanged in outcome under the stricter checks; a failure is investigated as a real divergence, not loosened.
|
||||
- _Requirements: 6.1, 6.2, 6.3, 6.7_
|
||||
- _Boundary: test_sim_equivalence.c, test_repl_equivalence.c, test_repl_limit_table.c, test_sim_live_allocation.c, test_sim_reconcile_unaccounted.c_
|
||||
- _Depends: 2, 3, 4, 5_
|
||||
- _Depends: 2.1, 2.2, 3.1, 3.2_ (3.1 as well as 3.2: a normal-mode recording's begin time and its allocations' start time must come from one clock reading, or a recorded-vs-replayed comparison can differ by one second)
|
||||
- [ ] 4.2 Bring hand-built recorded dispatch fixtures in line with the session start time
|
||||
- In every test that hand-builds a recorded dispatch later compared against a live replay, set the recorded allocation's reservation start time to the fixture session's recorded start in whole seconds (design.md, File Structure Plan: the six listed test files).
|
||||
- Update the comments in those files that describe the reservation start time as excluded or name the private comparators.
|
||||
- Harmless before 4.3, because the simulator still ignores the field; required after it.
|
||||
- Done when every listed fixture carries the session-start value and the scheduler suite still passes.
|
||||
- _Requirements: 6.2, 6.7_
|
||||
- _Depends: 3.2_
|
||||
- [ ] 4.3 Migrate simulator verification to the public API and report the difference in mismatch logs
|
||||
- Write the tests first: add the new description argument to the verification unit test's direct decision-comparison calls; test that the description names the differing field and both values for a dispatch difference, a pending job-id difference, a pending main-reason difference, a kind difference, a missing side and an unsupported preempt; test that the mismatch log line appends a provided description; invert the "reservation start time difference still matches" test so a difference is now reported and named.
|
||||
- Delete the simulator's private comparators; compare dispatch decisions with the public allocation comparison; keep a simulator-local main-reason-only pending check with the bug-1845 rationale (design.md, Components: Simulator verification).
|
||||
- In the simulator's replay loop (the only code caller of the decision comparison and the mismatch log), pass the description it already computes to the mismatch log and free it, and pass a fixed "no live decision" text for recorded decisions left over; the signature changes and these callers land in this same sub-task.
|
||||
- Done when the verification tests and the self-replay, altered-replay and reconcile round trips pass (they already run with the simulator clock frozen), and every mismatch line carries a diff.
|
||||
- _Requirements: 5.1, 5.2, 5.3, 5.4, 6.1, 6.4_
|
||||
- _Depends: 2.1, 3.1, 4.1, 4.2_
|
||||
|
||||
- [ ] 7. Reconcile `test_sim_verify.c` with the relocated comparators
|
||||
- Remove the DISPATCH alloc-level unit tests that only re-prove the allocation comparators — now owned by libxce tests from task 2. Replace the "`from` difference is ignored" test with one that shows same-session allocations verify cleanly and a changed `from` is reported as a mismatch naming `from`.
|
||||
- Keep the PEND unit tests (identical-content match, jobid/main-reason mismatch, detail-ignored) — they pin `sim_decision_eq`'s simulator-local main-reason-only behavior.
|
||||
- Add a test that `sim_log_mismatch`'s line contains the differing field and both values.
|
||||
- Run the `--sim-verify` round-trip tests (zero-mismatch self-replay, altered replay, reconcile cases). Until `scheduler-sim-discrete-clock` lands, at `--sim-speed 0`.
|
||||
- _Requirements: 5.2, 5.3, 5.4, 6.4_
|
||||
- _Boundary: test_sim_verify.c_
|
||||
- _Depends: 5_
|
||||
|
||||
- [ ] 8. Full build, test and live verification
|
||||
- `/xce-build lib` (debug + ASAN + tests) green.
|
||||
- `/xce-build scheduler --incremental` (debug + ASAN + tests) green, including `test_sim_verify.c`, `test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c`.
|
||||
- `/xce-build controller --incremental` green (consumes `xcealloc.h`; confirms nothing else depended on the removed names).
|
||||
- Live (after `scheduler-sim-discrete-clock`): record a session on a local deployment, replay it with `--sim-verify`: zero mismatches.
|
||||
- `grep -rn 'sim_verify_\(host_\|job_\)\?\(alloc\|preason\)_eq' src` returns no matches.
|
||||
- After merge: `buglog.py status bug-1881 fixed --commit <sha>` with the Verification section filled in.
|
||||
- [ ] 5. Validation
|
||||
- [ ] 5.1 Remove duplicated coverage from the verification unit tests
|
||||
- Remove the dispatch allocation-level tests that only re-prove the allocation comparisons now owned by libxce; keep the pending-decision, decision-level and integration tests, the inverted reservation-start-time test from 4.3, and the description-text tests from 4.3.
|
||||
- Done when the verification test file no longer duplicates libxce coverage and still passes.
|
||||
- _Requirements: 6.1, 6.4_
|
||||
- [ ] 5.2 Full build and regression
|
||||
- Run the full ASAN build with the libxce, scheduler and controller suites.
|
||||
- Confirm no source still names any of the five removed private comparators (the string, allocation, host-allocation, job-allocation and pending-reason ones); the decision-level comparison keeps its name.
|
||||
- Done when every suite passes and the search finds no match.
|
||||
- _Requirements: 5.4, 6.6, 6.7_
|
||||
- _Depends: 2, 3, 4, 5, 6, 7_
|
||||
- [ ] 5.3 Replay reproduces the reservation start time (after spec scheduler-sim-discrete-clock)
|
||||
- Add a test that a replayed allocation's reservation start time equals the recorded one at the default simulation settings.
|
||||
- Record a session on a local deployment and replay it with verification at the default settings.
|
||||
- Done when the test passes and the live replay reports zero mismatches; then mark bug-1881 fixed through the buglog skill with the fix commit.
|
||||
- _Requirements: 4.3, 5.4_
|
||||
- _Depends: spec scheduler-sim-discrete-clock_
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue