fix(scheduler): REPL iterator leaks, affected jobids in round summaries, raw ids for trace on job
bug-1928: repl_count_jobs() and repl_info_jobs() never destroyed their job-table iterators (16 B per continue round); both now do, and the baseline and trace wiring tests run with leak detection on. bug-1929: continue-mode round summaries listed only pending jobs, so a dispatch-only round printed an empty jobids= and rounds dispatching different jobs collapsed together; rounds now record and list every affected job (scheduler-sim-repl-verbosity Req 2.1, 3.2). The baseline transcript changes only in its two jobids= lines. bug-1930: trace on job now accepts the raw decimal jobid form that info trace and info job accept, through a shared parser. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5ca59dc9a8
commit
ba6f42cdc2
GPG key ID:
80533536B0EB651C
13 changed files with 455 additions and 67 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 <dashed>` and `trace on job <decimal>` 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);
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue