feat(scheduler-sim-repl-verbosity): task 1.2 - implement job identifier range compression
Implements repl_fmt_jobid_range() in repl.c (per design.md's "no new production .c file" commitment), grouping job-array identifiers by base and compressing contiguous index runs into [lo-hi] ranges. All 5 of task 1.1's RED-phase tests now pass. Went through one review round: an initial GREEN implementation placed the function in a new file (repl_jobid_range.c) to sidestep a linker issue in the lean test target from task 1.1. Review found and verified a cleaner fix: link the test against the full scheduler_srcs object list (mirroring the existing sched_scheduler_connect_test precedent), which resolves the same symbols without a new file. Remediated accordingly; bug-1762 in buglog.json documents the investigation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
parent
f165e3a751
commit
b639fae162
5 changed files with 788 additions and 434 deletions
|
|
@ -6,7 +6,7 @@
|
||||||
- Observable: the new test cases execute and fail against the current codebase, which has no compression behavior yet
|
- Observable: the new test cases execute and fail against the current codebase, which has no compression behavior yet
|
||||||
- _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5_
|
- _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5_
|
||||||
|
|
||||||
- [ ] 1.2 Implement job identifier range compression so the new tests pass
|
- [x] 1.2 Implement job identifier range compression so the new tests pass
|
||||||
- Group identifiers by their shared array submission and render contiguous index runs as a single compressed range, comma-joining non-contiguous runs
|
- Group identifiers by their shared array submission and render contiguous index runs as a single compressed range, comma-joining non-contiguous runs
|
||||||
- Never drop or fabricate an identifier, and fail safely (without truncating or crashing) when the output buffer is too small
|
- Never drop or fabricate an identifier, and fail safely (without truncating or crashing) when the output buffer is too small
|
||||||
- Observable: all tests from 1.1 pass; the helper is ready for the event-echo and round-summary work to adopt
|
- Observable: all tests from 1.1 pass; the helper is ready for the event-echo and round-summary work to adopt
|
||||||
|
|
|
||||||
File diff suppressed because one or more lines are too long
|
|
@ -1,4 +1,5 @@
|
||||||
#include <ctype.h>
|
#include <ctype.h>
|
||||||
|
#include <stdarg.h>
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include <stdlib.h>
|
#include <stdlib.h>
|
||||||
#include <string.h>
|
#include <string.h>
|
||||||
|
|
@ -529,3 +530,265 @@ void xce_repl_on_replay_complete(xce_sched_sim_driver_t* sim)
|
||||||
rs->replay_complete = true;
|
rs->replay_complete = true;
|
||||||
repl_command_loop(sim);
|
repl_command_loop(sim);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* ---- Job-identifier range compression (scheduler-sim-repl-verbosity spec,
|
||||||
|
* task 1.2, design.md's "Jobid Range Compressor" component, Req 4, ACs
|
||||||
|
* 4.1-4.5) ---- A pure, read-only formatter with zero dependency on
|
||||||
|
* xce_sched_sim_driver_t/xce_sched_core_t or any of this file's other REPL
|
||||||
|
* machinery, kept here per design.md's File Structure Plan ("... in repl.c,
|
||||||
|
* following the file's existing shared-helper convention" -- same
|
||||||
|
* convention repl_format_ts_human()/repl_split_tokens() above establish).
|
||||||
|
* Reuses fmt_jobid_ex()'s own jobid bit-layout macros (utils.h, already
|
||||||
|
* #included above) rather than re-deriving the layout.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/*
|
||||||
|
* xce_repl_jobid_group_t - one grouping unit while repl_fmt_jobid_range()
|
||||||
|
* walks its input: either a standalone non-array jobid (`is_array` false)
|
||||||
|
* or one array submission's (cluster, apiserver, timestamp, arraybase)
|
||||||
|
* identity plus every index seen for it so far. Kept in first-seen order
|
||||||
|
* (design.md: "distinct array bases appear in input order"). `idxs` grows
|
||||||
|
* by one realloc'd slot per newly-seen index, same idiom repl_break.c's
|
||||||
|
* repl_breakpoints_append() establishes for this file family.
|
||||||
|
*/
|
||||||
|
typedef struct {
|
||||||
|
bool is_array;
|
||||||
|
uint32_t cluster;
|
||||||
|
uint32_t apiserver;
|
||||||
|
uint32_t timestamp;
|
||||||
|
uint32_t arraybase; /* array groups only; already fmt_jobid_ex()-shifted */
|
||||||
|
uint32_t plain_seq; /* non-array groups only */
|
||||||
|
uint32_t* idxs;
|
||||||
|
int nidxs;
|
||||||
|
} xce_repl_jobid_group_t;
|
||||||
|
|
||||||
|
static void repl_jobid_groups_free(xce_repl_jobid_group_t* groups,
|
||||||
|
int ngroups)
|
||||||
|
{
|
||||||
|
int i;
|
||||||
|
|
||||||
|
for (i = 0; i < ngroups; i++) {
|
||||||
|
free(groups[i].idxs);
|
||||||
|
}
|
||||||
|
free(groups);
|
||||||
|
}
|
||||||
|
|
||||||
|
static bool repl_jobid_group_add_idx(xce_repl_jobid_group_t* g, uint32_t idx)
|
||||||
|
{
|
||||||
|
uint32_t* grown = (uint32_t*)realloc(
|
||||||
|
g->idxs, (size_t)(g->nidxs + 1) * sizeof(*(g->idxs)));
|
||||||
|
|
||||||
|
if (grown == NULL) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
g->idxs = grown;
|
||||||
|
g->idxs[g->nidxs] = idx;
|
||||||
|
g->nidxs++;
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
static int repl_jobid_idx_cmp(const void* a, const void* b)
|
||||||
|
{
|
||||||
|
uint32_t ua = *(const uint32_t*)a;
|
||||||
|
uint32_t ub = *(const uint32_t*)b;
|
||||||
|
|
||||||
|
if (ua < ub) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
if (ua > ub) {
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* repl_jr_appendf - appends one snprintf()-formatted fragment onto a
|
||||||
|
* realloc'd dynamic string (`*out`/`*outlen` track the allocation and its
|
||||||
|
* used length; `*out` stays NUL-terminated), growing by exactly the
|
||||||
|
* fragment's length each time -- same realloc idiom as
|
||||||
|
* repl_jobid_group_add_idx() above. Every fragment this formats (a decimal
|
||||||
|
* number, dash, comma, or bracket) fits well inside the 63-byte local
|
||||||
|
* scratch below; exceeding that is a defensive failure (returns false,
|
||||||
|
* `*out`/`*outlen` unchanged), as is an allocation failure.
|
||||||
|
*/
|
||||||
|
static bool repl_jr_appendf(char** out, size_t* outlen, const char* fmt, ...)
|
||||||
|
{
|
||||||
|
va_list ap;
|
||||||
|
char frag[64];
|
||||||
|
int n;
|
||||||
|
char* grown;
|
||||||
|
|
||||||
|
va_start(ap, fmt);
|
||||||
|
n = vsnprintf(frag, sizeof(frag), fmt, ap);
|
||||||
|
va_end(ap);
|
||||||
|
|
||||||
|
if ((n < 0) || ((size_t)n >= sizeof(frag))) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
grown = (char*)realloc(*out, *outlen + (size_t)n + 1);
|
||||||
|
if (grown == NULL) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
memcpy(grown + *outlen, frag, (size_t)n + 1); /* fragment + its NUL */
|
||||||
|
*out = grown;
|
||||||
|
*outlen += (size_t)n;
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* repl_fmt_jobid_range - design.md's "Jobid Range Compressor" (Req 4, ACs
|
||||||
|
* 4.1-4.5): formats `njobids` raw jobids into `buf` (bufsz-bounded),
|
||||||
|
* grouping by array submission (cluster, apiserver, timestamp, array base)
|
||||||
|
* and compressing contiguous [idx] runs into "[lo-hi]" form, comma-joining
|
||||||
|
* non-contiguous runs within one base; non-array jobids are listed
|
||||||
|
* individually. Distinct groups appear in the order each is FIRST seen in
|
||||||
|
* `jobids`; indices within a compressed range always ascend regardless of
|
||||||
|
* input order. Returns bytes written to `buf` (excluding the NUL), or
|
||||||
|
* negative if `buf` was too small -- mirroring fmt_jobid_ex()'s own pattern
|
||||||
|
* (src/lib/utils.c): the full rendering is built into a heap scratch buffer
|
||||||
|
* first, and `buf` is touched only once it is proven to fit -- never a
|
||||||
|
* partial/truncated write.
|
||||||
|
*/
|
||||||
|
int repl_fmt_jobid_range(char* buf, size_t bufsz, const uint64_t* jobids,
|
||||||
|
int njobids)
|
||||||
|
{
|
||||||
|
xce_repl_jobid_group_t* groups = NULL;
|
||||||
|
int ngroups = 0;
|
||||||
|
char* out = NULL;
|
||||||
|
size_t outlen = 0;
|
||||||
|
bool ok = true;
|
||||||
|
int i;
|
||||||
|
int rc;
|
||||||
|
|
||||||
|
if ((buf == NULL) || (bufsz == 0)) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
if (njobids <= 0) {
|
||||||
|
buf[0] = '\0';
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
if (jobids == NULL) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
|
groups = (xce_repl_jobid_group_t*)calloc((size_t)njobids,
|
||||||
|
sizeof(*groups));
|
||||||
|
if (groups == NULL) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
|
for (i = 0; (i < njobids) && ok; i++) {
|
||||||
|
uint64_t id = jobids[i];
|
||||||
|
uint32_t tag = EXTRACT_ID_TAG(id);
|
||||||
|
uint32_t cluster = EXTRACT_ID_CLUSTER(id);
|
||||||
|
uint32_t apiserver = EXTRACT_ID_APISERVER(id);
|
||||||
|
uint32_t timestamp = EXTRACT_ID_TIMESTAMP(id);
|
||||||
|
|
||||||
|
if ((tag & TAG_JOB_ARRAY) != 0) {
|
||||||
|
uint32_t arraybase = EXTRACT_ARRAY_JOB(id);
|
||||||
|
uint32_t idx = EXTRACT_ARRAY_IDX(id);
|
||||||
|
int g = -1;
|
||||||
|
int j;
|
||||||
|
|
||||||
|
for (j = 0; j < ngroups; j++) {
|
||||||
|
if (groups[j].is_array && (groups[j].cluster == cluster)
|
||||||
|
&& (groups[j].apiserver == apiserver)
|
||||||
|
&& (groups[j].timestamp == timestamp)
|
||||||
|
&& (groups[j].arraybase == arraybase)) {
|
||||||
|
g = j;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (g < 0) {
|
||||||
|
g = ngroups++;
|
||||||
|
groups[g].is_array = true;
|
||||||
|
groups[g].cluster = cluster;
|
||||||
|
groups[g].apiserver = apiserver;
|
||||||
|
groups[g].timestamp = timestamp;
|
||||||
|
groups[g].arraybase = arraybase;
|
||||||
|
}
|
||||||
|
ok = repl_jobid_group_add_idx(&groups[g], idx);
|
||||||
|
} else {
|
||||||
|
int g = ngroups++;
|
||||||
|
|
||||||
|
groups[g].is_array = false;
|
||||||
|
groups[g].cluster = cluster;
|
||||||
|
groups[g].apiserver = apiserver;
|
||||||
|
groups[g].timestamp = timestamp;
|
||||||
|
groups[g].plain_seq = EXTRACT_ID_SEQUENCE(id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (i = 0; (i < ngroups) && ok; i++) {
|
||||||
|
xce_repl_jobid_group_t* g = &groups[i];
|
||||||
|
|
||||||
|
if (i > 0) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, ",");
|
||||||
|
if (!ok) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!g->is_array) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, "%u-%u-%u-%u", g->cluster,
|
||||||
|
g->apiserver, g->timestamp, g->plain_seq);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
qsort(g->idxs, (size_t)g->nidxs, sizeof(*(g->idxs)),
|
||||||
|
repl_jobid_idx_cmp);
|
||||||
|
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, "%u-%u-%u-%u[", g->cluster,
|
||||||
|
g->apiserver, g->timestamp, g->arraybase);
|
||||||
|
|
||||||
|
{
|
||||||
|
int j = 0;
|
||||||
|
|
||||||
|
while (ok && (j < g->nidxs)) {
|
||||||
|
uint32_t run_lo = g->idxs[j];
|
||||||
|
uint32_t run_hi = run_lo;
|
||||||
|
int k = j + 1;
|
||||||
|
|
||||||
|
while ((k < g->nidxs) && (g->idxs[k] == run_hi + 1)) {
|
||||||
|
run_hi = g->idxs[k];
|
||||||
|
k++;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (j > 0) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, ",");
|
||||||
|
}
|
||||||
|
if (ok && (run_lo == run_hi)) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, "%u", run_lo);
|
||||||
|
} else if (ok) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, "%u-%u", run_lo,
|
||||||
|
run_hi);
|
||||||
|
}
|
||||||
|
|
||||||
|
j = k;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (ok) {
|
||||||
|
ok = repl_jr_appendf(&out, &outlen, "]");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
repl_jobid_groups_free(groups, ngroups);
|
||||||
|
|
||||||
|
if (!ok || (out == NULL) || ((outlen + 1) > bufsz)) {
|
||||||
|
free(out);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
|
memcpy(buf, out, outlen + 1);
|
||||||
|
rc = (int)outlen;
|
||||||
|
free(out);
|
||||||
|
|
||||||
|
return rc;
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -234,4 +234,39 @@ extern bool repl_cmd_set(xce_sched_sim_driver_t* sim, const char* line);
|
||||||
* a given state. */
|
* a given state. */
|
||||||
extern const char* repl_job_state_name(uint32_t state);
|
extern const char* repl_job_state_name(uint32_t state);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* repl_fmt_jobid_range (repl.c, scheduler-sim-repl-verbosity spec, task 1.2,
|
||||||
|
* Requirement 4 "Job Identifier Range Compression", ACs 4.1-4.5) - formats
|
||||||
|
* `njobids` raw jobids into `buf` (bufsz-bounded): groups by array
|
||||||
|
* submission (cluster, apiserver, timestamp, array base) and compresses
|
||||||
|
* contiguous [idx] runs into "[lo-hi]" form, comma-joining non-contiguous
|
||||||
|
* runs within one base; non-array jobids are listed individually, each at
|
||||||
|
* its own first-seen position. Distinct groups appear in the order each is
|
||||||
|
* first seen in `jobids`. Returns the number of bytes written to `buf`
|
||||||
|
* (excluding the NUL terminator), or a negative value if `buf` was too
|
||||||
|
* small to hold the full listing -- `buf` is left completely untouched in
|
||||||
|
* that case (never a partial/truncated write; see design.md's Error
|
||||||
|
* Handling section). Pure, read-only formatting -- never mutates `jobids`
|
||||||
|
* or any scheduler/job state. Shared by repl_print_event_summary()'s
|
||||||
|
* job-arrival listings, the Round Rollup's pend-jobid display, and
|
||||||
|
* repl_info_worker()'s assigned-job listing (design.md's Jobid Range
|
||||||
|
* Compressor component, Requirement 4.5) -- one helper, no per-caller
|
||||||
|
* variant.
|
||||||
|
*
|
||||||
|
* Lives in repl.c itself, exactly as design.md's File Structure Plan
|
||||||
|
* directs ("implements the new shared jobid-range-compression helper ... in
|
||||||
|
* repl.c"), even though it has zero dependency on
|
||||||
|
* xce_sched_sim_driver_t/xce_sched_core_t or any of repl_internal.h's OTHER
|
||||||
|
* extern cross-references (repl_cmd_break/repl_cmd_info/repl_cmd_set/...
|
||||||
|
* implemented in the repl_break.c/repl_info.c/repl_params.c siblings).
|
||||||
|
* test_repl_output_compression.c (task 1.1) still declares this function as
|
||||||
|
* a plain extern rather than #include-ing repl.c -- see that test file's
|
||||||
|
* own header comment -- and its meson target links the real scheduler_srcs
|
||||||
|
* object list instead (sched_scheduler_connect_test's own established
|
||||||
|
* technique), so it exercises this real implementation without pulling
|
||||||
|
* repl.c's other undefined cross-references into a directly-linked .o.
|
||||||
|
*/
|
||||||
|
extern int repl_fmt_jobid_range(char* buf, size_t bufsz,
|
||||||
|
const uint64_t* jobids, int njobids);
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
|
||||||
|
|
@ -974,21 +974,40 @@ sched_repl_equivalence_test = executable(
|
||||||
dependencies: [scheduler_test_deps, libedit_dep],
|
dependencies: [scheduler_test_deps, libedit_dep],
|
||||||
install: false)
|
install: false)
|
||||||
|
|
||||||
# RED-phase contract test for the REPL's job-identifier range compressor
|
# Contract test for the REPL's job-identifier range compressor
|
||||||
# (scheduler-sim-repl-verbosity spec, task 1.1, Requirement 4). Unlike every
|
# (scheduler-sim-repl-verbosity spec, task 1.1/1.2, Requirement 4).
|
||||||
# other sched_repl_*_test above, this is NOT a white-box #include of
|
# repl_fmt_jobid_range() lives in ../src/repl.c (design.md's File Structure
|
||||||
# ../src/repl.c and friends -- repl_fmt_jobid_range() is a pure formatter
|
# Plan: "implements the new shared jobid-range-compression helper ... in
|
||||||
# with no xce_sched_sim_driver_t/xce_sched_core_t dependency, so this file
|
# repl.c"), but repl.c's own dependency surface is too large for the lean
|
||||||
# declares it as a plain extern instead (see test_repl_output_compression.c's
|
# "#include the one file under test" pattern the other lightweight REPL
|
||||||
# own header comment for the full rationale). As of task 1.1, the function
|
# tests use (test_repl_output_compression.c declares repl_fmt_jobid_range()
|
||||||
# has no definition anywhere in this codebase (task 1.2 adds it), so this
|
# as a plain extern rather than #include-ing repl.c -- see that file's own
|
||||||
# target is EXPECTED to fail at link time -- that is task 1.1's own
|
# header comment: repl.c's repl_dispatch() references repl_cmd_break/
|
||||||
# Observable evidence, not a defect in this build file.
|
# repl_cmd_info/repl_cmd_set/... from sibling files repl_break.c/
|
||||||
|
# repl_info.c/repl_params.c, and a directly-linked .o -- unlike an archive
|
||||||
|
# member -- requires ALL its undefined symbols resolved regardless of
|
||||||
|
# whether the calling code path is ever exercised). So instead this target
|
||||||
|
# links the real scheduler_srcs object list, the same technique
|
||||||
|
# sched_scheduler_connect_test below established for the identical "real
|
||||||
|
# production dependency surface, avoid textual inclusion" problem.
|
||||||
|
# scheduler_srcs (parent meson.build) minus src/main.c (this test file
|
||||||
|
# supplies its own main()) -- built via a loop, same as
|
||||||
|
# scheduler_connect_test_srcs below, so it can never drift from the
|
||||||
|
# production source list. No `-Wl,--wrap=` or `-DXCE_SCHEDULER_C_PATH` is
|
||||||
|
# needed here: this test never calls sched_init() or reaches networking
|
||||||
|
# code, it only exercises the pure repl_fmt_jobid_range() formatter.
|
||||||
|
repl_output_compression_test_srcs = ['test_repl_output_compression.c']
|
||||||
|
foreach f : scheduler_srcs
|
||||||
|
if f != 'src/main.c'
|
||||||
|
repl_output_compression_test_srcs += ['../' + f]
|
||||||
|
endif
|
||||||
|
endforeach
|
||||||
|
|
||||||
sched_repl_output_compression_test = executable(
|
sched_repl_output_compression_test = executable(
|
||||||
'sched_repl_output_compression_test',
|
'sched_repl_output_compression_test',
|
||||||
['test_repl_output_compression.c'],
|
repl_output_compression_test_srcs,
|
||||||
include_directories:[incdir],
|
include_directories:[incdir],
|
||||||
dependencies: [scheduler_test_deps],
|
dependencies: [scheduler_deps, unity_dep],
|
||||||
install: false)
|
install: false)
|
||||||
|
|
||||||
# Tests for the scheduler's unified candidate-list connect/rejoin pipeline
|
# Tests for the scheduler's unified candidate-list connect/rejoin pipeline
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue