diff --git a/src/scheduler/src/repl.c b/src/scheduler/src/repl.c index c00dac5b..f755fdfa 100644 --- a/src/scheduler/src/repl.c +++ b/src/scheduler/src/repl.c @@ -198,6 +198,7 @@ int repl_count_jobs(xce_sched_core_t* core) } count++; } + iter_destroy(&it); return count; } diff --git a/src/scheduler/src/repl_break.c b/src/scheduler/src/repl_break.c index 9bc947a0..e86282be 100644 --- a/src/scheduler/src/repl_break.c +++ b/src/scheduler/src/repl_break.c @@ -916,6 +916,7 @@ void xce_repl_on_begin_round(xce_sched_sim_driver_t* sim, uint64_t begin_round) rs->last_n_dispatch = 0; rs->last_n_pend = 0; rs->last_n_preempt = 0; + repl_round_reset_decided(rs); bp = repl_find_begin_round_breakpoint(rs); if (bp != NULL) { @@ -951,12 +952,14 @@ void xce_repl_on_decision(xce_sched_sim_driver_t* sim, xce_decision_t* d) switch (d->op) { case XCE_DECISION_DISPATCH: rs->last_n_dispatch++; + repl_round_note_decided(rs, decision_jobid(d)); break; case XCE_DECISION_PEND: rs->last_n_pend++; break; case XCE_DECISION_PREEMPT: rs->last_n_preempt++; + repl_round_note_decided(rs, decision_jobid(d)); break; default: break; diff --git a/src/scheduler/src/repl_info.c b/src/scheduler/src/repl_info.c index 5bd971dd..07686db0 100644 --- a/src/scheduler/src/repl_info.c +++ b/src/scheduler/src/repl_info.c @@ -190,6 +190,7 @@ static void repl_info_jobs(xce_sched_sim_driver_t* sim) repl_job_state_name(job->state), (job->queue != NULL) ? job->queue->name : "-"); } + iter_destroy(&it); fprintf(stdout, "[sim] info jobs: %d job(s)\n", count); } diff --git a/src/scheduler/src/repl_internal.h b/src/scheduler/src/repl_internal.h index b281895f..099e0f7b 100644 --- a/src/scheduler/src/repl_internal.h +++ b/src/scheduler/src/repl_internal.h @@ -303,6 +303,20 @@ struct xce_sched_repl_state_s { int collapse_preempt_count; uint64_t* collapse_pend_jobids; int collapse_n_pend_jobids; + /* bug-1929: the jobs the range's dispatch/preempt decisions affected + * (heap, sorted, unique), part of the fingerprint and of the listing + * alongside the pend set; freed and reset with it. */ + uint64_t* collapse_decided_jobids; + int collapse_n_decided_jobids; + /* bug-1929: the current round's dispatch/preempt jobids, appended by + * xce_repl_on_decision() (repl_rollup.c's repl_round_note_decided()), + * released by xce_repl_on_begin_round() and taken over by + * repl_round_rollup_process(). `round_decided_lost` records a failed + * append, so that round's listing is reported as unavailable. */ + uint64_t* round_decided_jobids; + int round_n_decided; + int round_cap_decided; + bool round_decided_lost; /* * Resource Value Formatter field (scheduler-sim-repl-verbosity spec, @@ -571,6 +585,17 @@ extern int repl_count_jobs(xce_sched_core_t* core); */ extern void repl_round_rollup_process(xce_sched_sim_driver_t* sim); +/* + * repl_round_note_decided / repl_round_reset_decided (repl_rollup.c, + * bug-1929) - the per-round dispatch/preempt jobid list the Round Rollup + * lists with the pend set: note_decided appends one decision's jobid + * (xce_repl_on_decision()), reset_decided frees the list at the start of a + * round (xce_repl_on_begin_round()). + */ +extern void repl_round_note_decided(xce_sched_repl_state_t* rs, + uint64_t jobid); +extern void repl_round_reset_decided(xce_sched_repl_state_t* rs); + /* * repl_trace_render (repl_trace_render.c, scheduler-sim-decision-trace * spec, task 4.1, design.md "TraceRenderer") - writes one decision trace @@ -621,6 +646,17 @@ extern void repl_trace_store_fini(xce_repl_trace_state_t* ts); extern bool repl_cmd_info_trace(xce_sched_sim_driver_t* sim, char tokens[][XCE_REPL_TOKEN_LEN], int ntok); +/* + * repl_parse_jobsel_arg (repl_trace_store.c, bug-1930) - the job selector + * forms `info trace` and `trace on job` both take: parse_jobsel()'s dashed + * id, `X[k]` and `X[a-b]`, plus (like `info job`) a raw packed decimal id, + * returned as JOBSEL_BARE with `key` = the number and `*raw` = true (a + * decimal too wide for 64 bits is JOBSEL_EWIDTH). `*raw` is false for + * every other form; `raw` may be NULL. + */ +extern jobsel_err_t repl_parse_jobsel_arg(const char* arg, jobsel_t* out, + bool* raw); + /* * ReplTraceControl (repl_trace.c, scheduler-sim-decision-trace spec, task * 4.3, design.md "ReplTraceControl"): diff --git a/src/scheduler/src/repl_rollup.c b/src/scheduler/src/repl_rollup.c index 553ac7cd..78e458a7 100644 --- a/src/scheduler/src/repl_rollup.c +++ b/src/scheduler/src/repl_rollup.c @@ -121,13 +121,8 @@ static bool repl_query_pend_jobids(xce_sched_core_t* core, uint64_t** out_ids, } } - /* Unlike repl_count_jobs()/repl_info_jobs() (whose single call-per- - * command lifetime makes their own missing iter_destroy() a - * pre-existing, low-impact quirk), this function runs once per - * completed round for as long as `continue` keeps running -- the same - * safer iter_destroy() convention repl_info_queue_one() already - * establishes elsewhere in this file family is worth following here to - * avoid accumulating a real leak across a long replay. */ + /* Runs once per completed round for as long as `continue` keeps + * running, so a missing iter_destroy() here leaks per round. */ iter_destroy(&it); qsort(ids, (size_t)n, sizeof(*ids), repl_jobid_u64_cmp); @@ -160,6 +155,103 @@ static bool repl_jobid_arrays_equal(const uint64_t* a, int na, return true; } +/* + * repl_round_note_decided / repl_round_reset_decided - see repl_internal.h + * (bug-1929). The list lives on the heap so a round that dispatches many + * jobs is listed in full; an append that cannot grow the list sets + * `round_decided_lost` instead of dropping the id silently. + */ +void repl_round_note_decided(xce_sched_repl_state_t* rs, uint64_t jobid) +{ + if (jobid == 0) { /* a decision without its alloc/preempt body */ + return; + } + if (rs->round_n_decided == rs->round_cap_decided) { + int cap = (rs->round_cap_decided > 0) + ? (rs->round_cap_decided * 2) : 16; + uint64_t* grown = (uint64_t*)realloc(rs->round_decided_jobids, + (size_t)cap * sizeof(*grown)); + + if (grown == NULL) { + rs->round_decided_lost = true; + return; + } + rs->round_decided_jobids = grown; + rs->round_cap_decided = cap; + } + rs->round_decided_jobids[rs->round_n_decided++] = jobid; +} + +void repl_round_reset_decided(xce_sched_repl_state_t* rs) +{ + free(rs->round_decided_jobids); + rs->round_decided_jobids = NULL; + rs->round_n_decided = 0; + rs->round_cap_decided = 0; + rs->round_decided_lost = false; +} + +/* repl_sort_unique - sorts `ids` ascending and drops repeats (a job both + * preempted and dispatched in one round is listed once); returns the new + * count. */ +static int repl_sort_unique(uint64_t* ids, int n) +{ + int i; + int out = 0; + + if (n <= 0) { + return 0; + } + qsort(ids, (size_t)n, sizeof(*ids), repl_jobid_u64_cmp); + for (i = 0; i < n; i++) { + if ((out == 0) || (ids[out - 1] != ids[i])) { + ids[out++] = ids[i]; + } + } + return out; +} + +/* repl_fmt_affected - renders the union of two sorted, unique jobid sets + * (the pend set and the dispatch/preempt set) with repl_fmt_jobid_range() + * into `buf`. An empty union on a non-idle range only happens when the + * listing could not be built, so it says so instead of printing nothing. */ +static void repl_fmt_affected(char* buf, size_t bufsz, const uint64_t* a, + int na, const uint64_t* b, int nb) +{ + uint64_t* all; + int i = 0; + int j = 0; + int n = 0; + + if ((na + nb) == 0) { + snprintf(buf, bufsz, "(unavailable)"); + return; + } + all = (uint64_t*)malloc((size_t)(na + nb) * sizeof(*all)); + if (all == NULL) { + snprintf(buf, bufsz, "(unavailable)"); + return; + } + while ((i < na) || (j < nb)) { + uint64_t next; + + if ((j >= nb) || ((i < na) && (a[i] <= b[j]))) { + next = a[i++]; + } else { + next = b[j++]; + } + if ((n == 0) || (all[n - 1] != next)) { + all[n++] = next; + } + } + if (repl_fmt_jobid_range(buf, bufsz, all, n) < 0) { + /* design.md's Error Handling: buffer too small -- a placeholder + * rather than a truncated or partial listing. */ + snprintf(buf, bufsz, "(too many to list)"); + } + free(all); +} + /* * repl_round_emit - the Round Rollup's actual print, given an already- * decided range (bounds/fingerprint) rather than reading rs->collapse_* @@ -169,7 +261,8 @@ static bool repl_jobid_arrays_equal(const uint64_t* a, int na, * documents (test_repl_round_rollup.c's "Assumed output contract" header * comment): idle ranges (zero dispatch/preempt AND an empty pend-jobid set) * render as "... : idle"; every other range renders decision counts plus - * the pend-jobid listing via repl_fmt_jobid_range() (design.md's Jobid + * the affected-jobid listing (the pend set plus the dispatched and + * preempted jobs, bug-1929) via repl_fmt_jobid_range() (design.md's Jobid * Range Compressor, reused rather than reimplemented -- Requirement 4.5). * Callers decide whether an idle range's round count clears Requirement * 3.1's ">= 2" threshold before calling this -- this function always prints @@ -177,12 +270,14 @@ static bool repl_jobid_arrays_equal(const uint64_t* a, int na, */ static void repl_round_emit(uint64_t start_round, uint64_t end_round, uint64_t start_ts, uint64_t end_ts, - int dispatch_count, int preempt_count, - const uint64_t* pend_jobids, int n_pend_jobids, - bool idle) + int dispatch_count, int pend_count, + int preempt_count, const uint64_t* pend_jobids, + int n_pend_jobids, const uint64_t* decided_jobids, + int n_decided_jobids, bool idle) { char ts_start[32]; char ts_end[32]; + char jobids_buf[1024]; repl_format_ts_human(start_ts, ts_start, sizeof(ts_start)); repl_format_ts_human(end_ts, ts_end, sizeof(ts_end)); @@ -196,27 +291,19 @@ static void repl_round_emit(uint64_t start_round, uint64_t end_round, return; } - { - char jobids_buf[1024]; - int rc = repl_fmt_jobid_range(jobids_buf, sizeof(jobids_buf), - pend_jobids, n_pend_jobids); + /* bug-1929: "jobids=" names every job the range's decisions affected + * (Requirement 2.1) -- the dispatched/preempted jobs as well as the + * pend set; before, it listed the pend set alone, so a round whose + * only decision was a dispatch printed "jobids=" with nothing after. */ + repl_fmt_affected(jobids_buf, sizeof(jobids_buf), pend_jobids, + n_pend_jobids, decided_jobids, n_decided_jobids); - if (rc < 0) { - /* design.md's Error Handling: buffer too small -- fall back to - * a placeholder rather than truncating or crashing; this - * never fabricates or silently drops a jobid, it just can't - * render the full listing here. */ - snprintf(jobids_buf, sizeof(jobids_buf), "(too many to list)"); - } - - fprintf(stdout, - "[sim] round %llu-%llu (%llu round(s), t=%s..%s): %d " - "dispatch, %d pend, %d preempt -- jobids=%s\n", - (unsigned long long)start_round, (unsigned long long)end_round, - (unsigned long long)(end_round - start_round + 1), ts_start, - ts_end, dispatch_count, n_pend_jobids, preempt_count, - jobids_buf); - } + fprintf(stdout, + "[sim] round %llu-%llu (%llu round(s), t=%s..%s): %d " + "dispatch, %d pend, %d preempt -- jobids=%s\n", + (unsigned long long)start_round, (unsigned long long)end_round, + (unsigned long long)(end_round - start_round + 1), ts_start, + ts_end, dispatch_count, pend_count, preempt_count, jobids_buf); } /* @@ -258,13 +345,19 @@ void repl_round_collapse_flush(xce_sched_sim_driver_t* sim) repl_round_emit(rs->collapse_start_round, rs->collapse_end_round, rs->collapse_start_ts, rs->collapse_end_ts, rs->collapse_dispatch_count, + rs->collapse_n_pend_jobids, rs->collapse_preempt_count, rs->collapse_pend_jobids, - rs->collapse_n_pend_jobids, idle); + rs->collapse_n_pend_jobids, + rs->collapse_decided_jobids, + rs->collapse_n_decided_jobids, idle); } free(rs->collapse_pend_jobids); - rs->collapse_pend_jobids = NULL; - rs->collapse_n_pend_jobids = 0; + free(rs->collapse_decided_jobids); + rs->collapse_pend_jobids = NULL; + rs->collapse_n_pend_jobids = 0; + rs->collapse_decided_jobids = NULL; + rs->collapse_n_decided_jobids = 0; rs->collapse_dispatch_count = 0; rs->collapse_preempt_count = 0; rs->collapse_start_round = 0; @@ -287,23 +380,38 @@ void repl_round_rollup_process(xce_sched_sim_driver_t* sim) xce_sched_repl_state_t* rs = sim->repl_state; uint64_t* pend_jobids = NULL; int n_pend_jobids = 0; + uint64_t* decided = rs->round_decided_jobids; + int n_decided = rs->round_n_decided; + bool decided_lost = rs->round_decided_lost; int dispatch_count = rs->last_n_dispatch; int preempt_count = rs->last_n_preempt; uint64_t round_no = rs->session_count; uint64_t round_ts = rs->last_begin_round; - if (!repl_query_pend_jobids(&(sim->core), &pend_jobids, &n_pend_jobids)) { + /* bug-1929: this round's dispatch/preempt jobids now belong to this + * function -- freed below or handed to the collapsed range. */ + rs->round_decided_jobids = NULL; + repl_round_reset_decided(rs); + n_decided = repl_sort_unique(decided, n_decided); + + if (decided_lost + || !repl_query_pend_jobids(&(sim->core), &pend_jobids, + &n_pend_jobids)) { /* design.md's Error Handling: allocation failure while building * this round's fingerprint -- rather than silently losing this * round's information, flush whatever was already active * (preserving print ordering) and report this round on its own, - * uncollapsed. Never starts a new active range on this degraded - * path -- the next round gets a fair, fully-informed fingerprint - * attempt again. */ + * uncollapsed, its listing marked unavailable. Never starts a new + * active range on this degraded path -- the next round gets a + * fair, fully-informed fingerprint attempt again. */ repl_round_collapse_flush(sim); repl_round_emit(round_no, round_no, round_ts, round_ts, - dispatch_count, preempt_count, NULL, 0, - (dispatch_count == 0) && (preempt_count == 0)); + dispatch_count, n_pend_jobids, preempt_count, NULL, 0, + NULL, 0, + (dispatch_count == 0) && (preempt_count == 0) + && (n_pend_jobids == 0)); + free(pend_jobids); + free(decided); return; } @@ -315,11 +423,12 @@ void repl_round_rollup_process(xce_sched_sim_driver_t* sim) * so a pend-only round is not idle. */ repl_round_collapse_flush(sim); repl_round_emit(round_no, round_no, round_ts, round_ts, - dispatch_count, preempt_count, pend_jobids, - n_pend_jobids, + dispatch_count, n_pend_jobids, preempt_count, + pend_jobids, n_pend_jobids, decided, n_decided, (dispatch_count == 0) && (preempt_count == 0) && (n_pend_jobids == 0)); free(pend_jobids); + free(decided); return; } @@ -328,12 +437,16 @@ void repl_round_rollup_process(xce_sched_sim_driver_t* sim) && (rs->collapse_preempt_count == preempt_count) && repl_jobid_arrays_equal(rs->collapse_pend_jobids, rs->collapse_n_pend_jobids, pend_jobids, - n_pend_jobids)) { + n_pend_jobids) + && repl_jobid_arrays_equal(rs->collapse_decided_jobids, + rs->collapse_n_decided_jobids, decided, + n_decided)) { /* Identical outcome -- extend the active range silently (never * printed until it ends, Requirement 3.2). */ rs->collapse_end_round = round_no; rs->collapse_end_ts = round_ts; free(pend_jobids); + free(decided); return; } @@ -343,13 +456,15 @@ void repl_round_rollup_process(xce_sched_sim_driver_t* sim) * range of its own (print deferred until IT, in turn, ends). */ repl_round_collapse_flush(sim); - rs->collapse_active = true; - rs->collapse_start_round = round_no; - rs->collapse_end_round = round_no; - rs->collapse_start_ts = round_ts; - rs->collapse_end_ts = round_ts; - rs->collapse_dispatch_count = dispatch_count; - rs->collapse_preempt_count = preempt_count; - rs->collapse_pend_jobids = pend_jobids; /* ownership transferred */ - rs->collapse_n_pend_jobids = n_pend_jobids; + rs->collapse_active = true; + rs->collapse_start_round = round_no; + rs->collapse_end_round = round_no; + rs->collapse_start_ts = round_ts; + rs->collapse_end_ts = round_ts; + rs->collapse_dispatch_count = dispatch_count; + rs->collapse_preempt_count = preempt_count; + rs->collapse_pend_jobids = pend_jobids; /* ownership transferred */ + rs->collapse_n_pend_jobids = n_pend_jobids; + rs->collapse_decided_jobids = decided; /* ownership transferred */ + rs->collapse_n_decided_jobids = n_decided; } diff --git a/src/scheduler/src/repl_trace.c b/src/scheduler/src/repl_trace.c index fed19e57..6ecba91d 100644 --- a/src/scheduler/src/repl_trace.c +++ b/src/scheduler/src/repl_trace.c @@ -207,7 +207,9 @@ static bool parse_sel(xce_repl_trace_settings_t* s, word_t w) out_of_memory(); return false; } - err = parse_jobsel(text, &js); + /* bug-1930: the forms `info trace` takes, including a raw packed + * decimal id, which selects what its dashed form would */ + err = repl_parse_jobsel_arg(text, &js, NULL); free(text); if (err != JOBSEL_OK) { invalid("job id", w, jobsel_strerror(err)); diff --git a/src/scheduler/src/repl_trace_store.c b/src/scheduler/src/repl_trace_store.c index 86fd4cb8..e9c9de65 100644 --- a/src/scheduler/src/repl_trace_store.c +++ b/src/scheduler/src/repl_trace_store.c @@ -443,24 +443,50 @@ static bool all_digits(const char* s) return true; } +jobsel_err_t repl_parse_jobsel_arg(const char* arg, jobsel_t* out, bool* raw) +{ + jobsel_err_t err = parse_jobsel(arg, out); + unsigned long long v; + + if (raw != NULL) { + *raw = false; + } + if ((err == JOBSEL_OK) || (arg == NULL) || !all_digits(arg)) { + return err; + } + errno = 0; + v = strtoull(arg, NULL, 10); + if (errno == ERANGE) { + return JOBSEL_EWIDTH; + } + memset(out, 0, sizeof(*out)); + out->form = JOBSEL_BARE; + out->key = (uint64_t)v; + if (raw != NULL) { + *raw = true; + } + return JOBSEL_OK; +} + /* One `info trace` argument: a job, `array[k]`, `array[a-b]`, an array id, * or (like `info job`) a raw packed jobid. */ static void info_trace_sel(xce_sched_sim_driver_t* sim, const xce_repl_trace_state_t* ts, const char* arg) { jobsel_t sel; - jobsel_err_t err = parse_jobsel(arg, &sel); + bool raw; + jobsel_err_t err = repl_parse_jobsel_arg(arg, &sel, &raw); char idbuf[64]; if (err != JOBSEL_OK) { - if (all_digits(arg)) { - info_trace_one(sim, ts, strtoull(arg, NULL, 10)); - return; - } fprintf(stdout, "[sim] info trace: invalid job id %s: %s\n", arg, jobsel_strerror(err)); return; } + if (raw) { /* like `info job`: a raw packed id names that job */ + info_trace_one(sim, ts, sel.key); + return; + } switch (sel.form) { case JOBSEL_SINGLE: diff --git a/src/scheduler/testsuite/fixtures/repl_trace_baseline/transcript.txt b/src/scheduler/testsuite/fixtures/repl_trace_baseline/transcript.txt index 2dc87e54..2dd51d59 100644 --- a/src/scheduler/testsuite/fixtures/repl_trace_baseline/transcript.txt +++ b/src/scheduler/testsuite/fixtures/repl_trace_baseline/transcript.txt @@ -160,7 +160,7 @@ Trace: allocs=1 [sim] breakpoint 1 deleted [sim] breakpoint 2 set: BEGIN_ROUND -[sim] round 5-5 (1 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 3 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-107 +[sim] round 5-5 (1 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 3 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-104,0-0-0-105,0-0-0-106,0-0-0-107 [sim] hit breakpoint 2 @ BEGIN_ROUND round=7000 [sim] session: ts=6000 (1970-01-01T00:00:00Z) event_index=6 sessions=6 [sim] last session: round=7000 dispatch=0 pend=0 preempt=0 @@ -170,7 +170,7 @@ Trace: [sim] round 6-7 (2 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 0 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-107 [sim] t=8500 (1970-01-01T00:00:00Z) session=8 [sim] event: JOB_PUSH njobs=1 jobids=[0-0-0-108] -[sim] round 8-8 (1 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 1 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-107 +[sim] round 8-8 (1 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 1 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-107,0-0-0-108 [sim] round 9-9 (1 round(s), t=1970-01-01T00:00:00Z..1970-01-01T00:00:00Z): 0 dispatch, 1 pend, 0 preempt -- jobids=0-0-0-107 [sim] replay complete. 9 session(s) processed. [sim] output written to: set_output.replay diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index d2bb1dd3..4040b67b 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -2542,9 +2542,10 @@ sched_repl_trace_baseline_test = executable( + '"'], install: false) +# Leak detection stays on: the child xce_scheduler inherits it, so a REPL +# leak makes the scripted session exit non-zero (bug-1928). test('scheduler repl trace baseline test', sched_repl_trace_baseline_test, suite: 'scheduler', - env: ['ASAN_OPTIONS=detect_leaks=0'], depends: scheduler) # scheduler-sim-decision-trace task 6.1 (Requirements 1-8; design.md @@ -2884,8 +2885,7 @@ test('scheduler repl trace emit test', # `set output` rejecting an open trace file, quit/EOF shutdown, the # traced-round rollup and the interactive-only gates. Includes repl.c (for # its static help, dispatch and quit) and links every other scheduler -# source except main.c. Leak detection is off like the other rollup tests: -# repl_count_jobs() (repl.c) never destroys its iterator. +# source except main.c. Leak detection stays on. sched_repl_trace_wiring_test_srcs = ['test_repl_trace_wiring.c'] foreach f : scheduler_srcs if f != 'src/main.c' and f != 'src/repl.c' @@ -2904,8 +2904,7 @@ sched_repl_trace_wiring_test = executable( install: false) test('scheduler repl trace wiring test', - sched_repl_trace_wiring_test, suite: 'scheduler', - env: ['ASAN_OPTIONS=detect_leaks=0']) + sched_repl_trace_wiring_test, suite: 'scheduler') # scheduler-sim-decision-trace task 5.3 (Requirements 1.6, 2.5, 7.1-7.3, # 7.5, 10.1, 11.3): the trace wired into the simulation driver's session and diff --git a/src/scheduler/testsuite/test_repl_round_rollup.c b/src/scheduler/testsuite/test_repl_round_rollup.c index f24df586..56bb5093 100644 --- a/src/scheduler/testsuite/test_repl_round_rollup.c +++ b/src/scheduler/testsuite/test_repl_round_rollup.c @@ -1244,6 +1244,116 @@ test_active_collapsed_range_flushed_before_replay_complete_banner(void) "banner: PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* Golden: a continue-mode round summary lists the jobs its dispatch and */ +/* preempt decisions affected, not only the jobs left pending, and two */ +/* rounds that dispatched different jobs never collapse (Requirements 2.1, */ +/* 3.2). Before the fix a lone dispatch printed "jobids=" with nothing */ +/* after it. */ +/* ---------------------------------------------------------------------- */ + +#define GOLDEN_TS1 2000000000ULL +#define GOLDEN_TS2 3000000000ULL + +static xce_sched_sim_driver_t* g_golden_sim; + +static void golden_round(uint64_t ts, uint64_t dispatch_id, uint64_t preempt_id) +{ + xce_job_alloc_t alloc; + xce_preempt_t preempt; + xce_decision_t d; + + xce_repl_on_begin_round(g_golden_sim, ts); + memset(&alloc, 0, sizeof(alloc)); + alloc.jobid = dispatch_id; + memset(&d, 0, sizeof(d)); + d.op = XCE_DECISION_DISPATCH; + d.u.alloc = &alloc; + xce_repl_on_decision(g_golden_sim, &d); + if (preempt_id != 0) { + memset(&preempt, 0, sizeof(preempt)); + preempt.jobid = preempt_id; + memset(&d, 0, sizeof(d)); + d.op = XCE_DECISION_PREEMPT; + d.u.preempt = &preempt; + xce_repl_on_decision(g_golden_sim, &d); + } + repl_round_rollup_process(g_golden_sim); +} + +static void golden_two_dispatch_rounds(void) +{ + golden_round(GOLDEN_TS1, 5101ULL, 0); + golden_round(GOLDEN_TS2, 5102ULL, 0); + repl_round_collapse_flush(g_golden_sim); +} + +static void golden_dispatch_and_preempt_round(void) +{ + golden_round(GOLDEN_TS1, 5103ULL, 5104ULL); + repl_round_collapse_flush(g_golden_sim); +} + +static char* golden_run(void (*fn)(void)) +{ + xce_sched_sim_driver_t sim; + char* captured; + + memset(&sim, 0, sizeof(sim)); + sim.interactive = true; + TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sim.core))); + sim.repl_state = repl_state_ensure(&sim); + TEST_ASSERT_NOT_NULL(sim.repl_state); + sim.repl_state->mode = XCE_REPL_MODE_CONTINUE; + + g_golden_sim = ∼ + captured = capture_stdout_around(fn); + g_golden_sim = NULL; + + TEST_ASSERT_NULL(sim.repl_state->collapse_pend_jobids); + TEST_ASSERT_NULL(sim.repl_state->collapse_decided_jobids); + TEST_ASSERT_NULL(sim.repl_state->round_decided_jobids); + free(sim.repl_state); /* the core is not torn down, as in + * build_fixture_scheduler() */ + return captured; +} + +static void test_round_summary_lists_dispatched_jobids_golden(void) +{ + char t1[32]; + char t2[32]; + char x[64]; + char y[64]; + char z[64]; + char expected[512]; + char* captured; + + repl_format_ts_human(GOLDEN_TS1, t1, sizeof(t1)); + repl_format_ts_human(GOLDEN_TS2, t2, sizeof(t2)); + + fmt_jobid(x, sizeof(x), 5101ULL); + fmt_jobid(y, sizeof(y), 5102ULL); + snprintf(expected, sizeof(expected), + "[sim] round 1-1 (1 round(s), t=%s..%s): 1 dispatch, 0 pend, " + "0 preempt -- jobids=%s\n" + "[sim] round 2-2 (1 round(s), t=%s..%s): 1 dispatch, 0 pend, " + "0 preempt -- jobids=%s\n", + t1, t1, x, t2, t2, y); + captured = golden_run(golden_two_dispatch_rounds); + TEST_ASSERT_EQUAL_STRING(expected, captured); + free(captured); + + fmt_jobid(x, sizeof(x), 5103ULL); + fmt_jobid(z, sizeof(z), 5104ULL); + snprintf(expected, sizeof(expected), + "[sim] round 1-1 (1 round(s), t=%s..%s): 1 dispatch, 0 pend, " + "1 preempt -- jobids=%s,%s\n", + t1, t1, x, z); + captured = golden_run(golden_dispatch_and_preempt_round); + TEST_ASSERT_EQUAL_STRING(expected, captured); + free(captured); +} + int main(void) { UNITY_BEGIN(); @@ -1259,6 +1369,7 @@ int main(void) RUN_TEST(test_round_collapse_flush_is_a_safe_noop_when_nothing_active); RUN_TEST( test_active_collapsed_range_flushed_before_replay_complete_banner); + RUN_TEST(test_round_summary_lists_dispatched_jobids_golden); return UNITY_END(); } diff --git a/src/scheduler/testsuite/test_repl_trace_cmd.c b/src/scheduler/testsuite/test_repl_trace_cmd.c index efb0cd7c..cdbd9c89 100644 --- a/src/scheduler/testsuite/test_repl_trace_cmd.c +++ b/src/scheduler/testsuite/test_repl_trace_cmd.c @@ -322,6 +322,88 @@ static void test_on_job_selectors(void) TEST_ASSERT_FALSE(sched_trace_ctl_wants(ctl, ELEM2(1))); } +/* Copies `next`'s selector list for comparing two parses. */ +static int sel_copy(xce_sched_trace_sel_t* out, int max) +{ + const xce_sched_trace_ctl_t* ctl = next_ctl(); + int i; + + TEST_ASSERT_TRUE(ctl->nsel <= max); + for (i = 0; i < ctl->nsel; i++) { + out[i] = ctl->sel[i]; + } + return ctl->nsel; +} + +/* Runs `trace on job ` and `trace on job ` for `id` and + * checks both produce the same selectors. */ +static void assert_raw_matches_dashed(uint64_t id) +{ + char line[256]; + xce_sched_trace_sel_t dashed[4]; + int ndashed; + int i; + + snprintf(line, sizeof(line), "trace on job %s", fmt_id(id)); + run_ok(line); + ndashed = sel_copy(dashed, 4); + + snprintf(line, sizeof(line), "trace on job %llu", (unsigned long long)id); + run_ok(line); + TEST_ASSERT_EQUAL_INT_MESSAGE(ndashed, next_ctl()->nsel, line); + for (i = 0; i < ndashed; i++) { + TEST_ASSERT_TRUE_MESSAGE(dashed[i].key == next_ctl()->sel[i].key, + line); + TEST_ASSERT_EQUAL_INT_MESSAGE(dashed[i].whole_array, + next_ctl()->sel[i].whole_array, line); + } +} + +/* bug-1930: `trace on job` takes the raw packed decimal id `info trace` + * and `info job` accept, mapped to the selector its dashed form gives + * (an aligned id also selects the whole array, Implementation Notes 4.3), + * and mixes with the dashed and element forms on one line. */ +static void test_on_job_raw_decimal_ids(void) +{ + char line[512]; + const xce_sched_trace_ctl_t* ctl; + + assert_raw_matches_dashed(PLAIN_JOB); + assert_raw_matches_dashed(ARRAY_BASE); + assert_raw_matches_dashed((uint64_t)104); + + snprintf(line, sizeof(line), "trace on job %llu", + (unsigned long long)PLAIN_JOB); + run_ok(line); + ctl = next_ctl(); + TEST_ASSERT_EQUAL_INT(1, ctl->nsel); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, PLAIN_JOB)); + TEST_ASSERT_FALSE(sched_trace_ctl_wants(ctl, PLAIN_JOB2)); + + snprintf(line, sizeof(line), "trace on job %llu", + (unsigned long long)ARRAY_BASE); + run_ok(line); + ctl = next_ctl(); + TEST_ASSERT_EQUAL_INT(2, ctl->nsel); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, ARRAY_BASE)); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, ELEM(1))); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, ELEM(32767))); + + run_ok("trace on job 104"); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(next_ctl(), (uint64_t)104)); + + snprintf(line, sizeof(line), "trace on job %llu %s[5-6] %s level full", + (unsigned long long)PLAIN_JOB, fmt_id(ARRAY2_BASE), + fmt_id(ARRAY_BASE)); + run_ok(line); + ctl = next_ctl(); + TEST_ASSERT_EQUAL_INT(XCE_TRACE_LEVEL_FULL, ctl->level); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, PLAIN_JOB)); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, ELEM2(5))); + TEST_ASSERT_FALSE(sched_trace_ctl_wants(ctl, ELEM2(7))); + TEST_ASSERT_TRUE(sched_trace_ctl_wants(ctl, ELEM(9))); +} + /* 1.12: 20 job selectors on one line (more than the tokenizer's 16). */ static void test_twenty_selectors(void) { @@ -434,10 +516,15 @@ static void test_invalid_arguments_leave_settings(void) snprintf(want, sizeof(want), "[sim] trace: invalid job id: 1-2-x (%s)\n", jobsel_strerror(JOBSEL_ESYNTAX)); assert_rejected("trace on job 1-2-x", want); - snprintf(line, sizeof(line), "trace on job %s 7", fmt_id(PLAIN_JOB)); - snprintf(want, sizeof(want), "[sim] trace: invalid job id: 7 (%s)\n", + snprintf(line, sizeof(line), "trace on job %s 7x", fmt_id(PLAIN_JOB)); + snprintf(want, sizeof(want), "[sim] trace: invalid job id: 7x (%s)\n", jobsel_strerror(JOBSEL_ESYNTAX)); assert_rejected(line, want); + /* bug-1930: a raw decimal id too wide for 64 bits */ + snprintf(want, sizeof(want), + "[sim] trace: invalid job id: 18446744073709551616 (%s)\n", + jobsel_strerror(JOBSEL_EWIDTH)); + assert_rejected("trace on job 18446744073709551616", want); assert_rejected("trace on all level verbose", "[sim] trace: invalid level: verbose\n"); assert_rejected("trace on all level", @@ -677,6 +764,7 @@ int main(void) RUN_TEST(test_on_all_defaults); RUN_TEST(test_on_all_options); RUN_TEST(test_on_job_selectors); + RUN_TEST(test_on_job_raw_decimal_ids); RUN_TEST(test_twenty_selectors); RUN_TEST(test_file_paths); RUN_TEST(test_off); diff --git a/src/scheduler/testsuite/test_repl_trace_emit.c b/src/scheduler/testsuite/test_repl_trace_emit.c index 970d54a2..cf3daf30 100644 --- a/src/scheduler/testsuite/test_repl_trace_emit.c +++ b/src/scheduler/testsuite/test_repl_trace_emit.c @@ -78,6 +78,9 @@ void tearDown(void) repl_trace_shutdown(&g_sim); free(g_rs.collapse_pend_jobids); g_rs.collapse_pend_jobids = NULL; + free(g_rs.collapse_decided_jobids); /* bug-1929 */ + g_rs.collapse_decided_jobids = NULL; + repl_round_reset_decided(&g_rs); g_sim.repl_state = NULL; unlink("a.trace"); unlink("b.trace"); diff --git a/src/scheduler/testsuite/test_repl_trace_wiring.c b/src/scheduler/testsuite/test_repl_trace_wiring.c index 31dfd839..c9515d4e 100644 --- a/src/scheduler/testsuite/test_repl_trace_wiring.c +++ b/src/scheduler/testsuite/test_repl_trace_wiring.c @@ -96,6 +96,9 @@ static void fixture_fini(void) repl_trace_shutdown(&g_sim); free(g_rs.collapse_pend_jobids); g_rs.collapse_pend_jobids = NULL; + free(g_rs.collapse_decided_jobids); /* bug-1929 */ + g_rs.collapse_decided_jobids = NULL; + repl_round_reset_decided(&g_rs); if (g_sim.record_file != NULL) { fclose(g_sim.record_file); g_sim.record_file = NULL;