A-batch admin-plane fixes: rusage unit name, XCE_JOBID identity, select WARN, CLI env, docs
rusage collection (merge cb0c7fe9 regression, root cause of 0 counters):
- collect_job_rusages builds the systemd unit name with
make_job_sd_unit_name (four-segment fmt_jobid_ex form) instead of a bare
%lu of the packed uint64, so the D-Bus property lookups hit the real
per-job unit again. E2E on t1: DONE job reports max_mem/avg_mem/cpu_time
non-zero (previously all 0).
XCE_JOBID written identity:
- exdata sets XCE_JOBID via fmt_jobid (four-segment
cluster-apiserver-timestamp-seq) instead of a bare packed decimal;
XCE_JOBINDEX handling unchanged.
- xrun parses XCE_JOBID with parse_jobid (accepts the four-segment form);
a bare decimal no longer round-trips through the written identity.
- E2E on t1: job sees XCE_JOBID=<submit id>; xrun remote-start round-trip
to another host matches the job.
select WARN (silent degradation):
- parse_resreq_primary WARNs when a select references an unregistered
resource ("...references an unregistered resource -- dropping the select
and skipping any rusage[] clause of this primary") instead of failing
silently; scheduler log shows the WARN on t1 and the job schedules
without the constraint.
CLI env handling (unit-tested; subEnvVars is not flag-bound so this is
not reachable from the production CLI):
- getEnvironmentVariables keeps B=b=c (split at first '=') and warns +
drops a bare key or an '=value' fragment instead of silently mis-parsing.
Docs:
- src/docs/xce.md: document the dead-clause semantics (select/rusage
primary drop behavior) matching the WARN.
bug-1972 (test-only UAF): test_job_live_gc asserted job destruction by
reading refcount from the freed job; rewrite the assertion to observe
destruction without touching freed memory. controller_unit_test is green
again (1354 tests).
Regenerate 3 repl_trace_baseline fixtures whose baselines shifted with the
select-WARN logging; replay tests green.
Spec naming: external-license-resources design.md/brief.md fix the
self-referential daemon-name note (old drafts said xceextresd/extresd).
Verified on t1: rusage_burn job (counters non-zero in DONE), echo_env job
(four-segment XCE_JOBID), select[nosuchres] job (WARN + schedules),
xrun t2 remote-start round-trip, CLI env unit tests, xcelib 762 /
controller 1354 / worker exdata 8+11 / sched repl_trace 4+14+4+10+17
tests green.
Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
parent
68eabd8a4a
commit
f780a1660a
17 changed files with 329 additions and 44 deletions
|
|
@ -160,13 +160,21 @@ static void test_sweep_purges_expired_done_job(void)
|
|||
|
||||
TEST_ASSERT_EQUAL_INT(3, jlg_job_count());
|
||||
|
||||
/* Refcount before the sweep: seed gives the table 1, and nothing else
|
||||
* holds the job. Read NOW — a correct sweep dereferences down to
|
||||
* job_destroy and FREES the whole RTTI object, so touching `expired`
|
||||
* after the sweep is a use-after-free (the assertion that proved
|
||||
* "refcount reached 0" by reading freed memory was itself the bug;
|
||||
* destruction is verified by absence below instead). */
|
||||
TEST_ASSERT_EQUAL_UINT32(1, job_get_refcount(expired));
|
||||
|
||||
job_live_gc_sweep(&g_jlg_c);
|
||||
|
||||
/* The expired job is gone from the table... */
|
||||
/* The expired job is gone from the table — and, with the fixture
|
||||
* holding no other reference, a correct sweep also destroyed it
|
||||
* (refcount 1 → 0 through its two dereferences). `expired` must not
|
||||
* be dereferenced past this point. */
|
||||
TEST_ASSERT_NULL(jlg_find_job(81001));
|
||||
/* ...and its refcount ran down to destruction (seed refcount 1, sweep
|
||||
* added 1, dereference after unlink leaves 0 → job_destroy ran). */
|
||||
TEST_ASSERT_EQUAL_UINT32(0, job_get_refcount(expired));
|
||||
|
||||
/* The others are untouched. */
|
||||
TEST_ASSERT_NOT_NULL(jlg_find_job(81002));
|
||||
|
|
|
|||
|
|
@ -122,8 +122,9 @@ supports a subset of the LSF array syntax.
|
|||
- **Element number in the job's environment.** A running element finds its
|
||||
own number in `XCE_JOBINDEX` (1..N). An ordinary job gets
|
||||
`XCE_JOBINDEX=0`. A user-supplied `XCE_JOBINDEX` is overridden, with a
|
||||
warning in the worker log. `XCE_JOBID` is the element's own job number, as
|
||||
before.
|
||||
warning in the worker log. `XCE_JOBID` is the element's own job number in
|
||||
its written form (`cluster-apiserver-timestamp-sequence`, the same form
|
||||
listings and control commands accept).
|
||||
- **Test harness.** `xce_test jobsub -a N` submits elements 1 through N
|
||||
(2 <= N <= 32767, regardless of `max_array_size`; `-a 1` submits an
|
||||
ordinary job). `$jobid` is then the array identity and `$jobid[k]` names
|
||||
|
|
@ -156,6 +157,48 @@ These LSF features are not available either:
|
|||
- **`LSB_JOBINDEX` / `LSB_JOBID` variables** are not set. Scripts read
|
||||
`XCE_JOBINDEX` (element number) and `XCE_JOBID` (job number) instead.
|
||||
|
||||
## Resource requirement (resreq)
|
||||
|
||||
A job's resource request is given at submission with
|
||||
`xcecli jobs submit -R "<request>"` (REST: `resreq`). The grammar accepts
|
||||
`select[...]`, `rusage[...]`, `span[...]`, `same[...]` and `affinity[...]`
|
||||
clauses in any order, compound requests (`N*{...} + M*{...}`) and
|
||||
alternatives (`{...} || {...}`). A job with no request (or an empty one)
|
||||
is scheduled as `type == any` (any host) — the API layer does not apply a
|
||||
default; the "default: type == local" note in the API model is historical.
|
||||
|
||||
What the scheduler actually enforces today:
|
||||
|
||||
- **`select[<expression>]`** — enforced. The expression is evaluated
|
||||
against each candidate host's registered resources (`resources.yaml`:
|
||||
`slots`, `mem`, load indexes, host attributes `type`/`model`/... and any
|
||||
static resources). Workers report `type` as `LINUX64` and `model` as
|
||||
`Intel64`.
|
||||
- An unknown resource name in the expression is **tolerated, not
|
||||
rejected**: the select is dropped and the job is scheduled without it,
|
||||
with a warning in the scheduler log. Everything after the dropped
|
||||
select in the same request (e.g. a trailing `rusage[...]`) is dropped
|
||||
with it. A malformed select (syntax error, type mismatch) rejects the
|
||||
whole request at submission or scheduling.
|
||||
- **`rusage[<res>=<value>[:duration=<d>[:decay=<0|1>]] ...]`** — the
|
||||
value is enforced as a per-task request (multiplied by the job's slot
|
||||
count; `mem` also feeds memory limits and accounting). The optional
|
||||
`per` suffix (`value/task`, `value/job`, `value/host`) is parsed and
|
||||
kept, and `value/job` / `value/host` are honored when computing
|
||||
job-wide totals, but the allocation request itself is always built
|
||||
**per task**. `duration` and `decay` are parsed and stored but **not
|
||||
consumed** — no historical-usage decay is implemented (see gaps.md).
|
||||
- **`span[...]`, `same[...]`, `affinity[...]`** — parsed for syntax and
|
||||
**dropped**: a multi-host job may be split across hosts by even
|
||||
splitting only, with no placement control (see gaps.md #15).
|
||||
- **Queue-level `res_req` / `gpu_req`** (`queues.yaml`) — the queue's
|
||||
`res_req` string is carried to the scheduler's queue object but is
|
||||
**never merged into job allocation**; jobs get only their own request.
|
||||
`gpu_req` has no wire field at all.
|
||||
- **Ordering** — requests are evaluated first-fit in submission order
|
||||
within a queue; there is no `order[...]` clause and no load-based
|
||||
ordering.
|
||||
|
||||
## shell
|
||||
|
||||
An interactive bash shell used for troubleshooting and inspection. Not
|
||||
|
|
|
|||
|
|
@ -896,10 +896,20 @@ func (s *stringValue) Set(val string) error {
|
|||
func (s *stringValue) Type() string { return "string" }
|
||||
|
||||
func (js *JobSubmitter) getEnvironmentVariables() []string {
|
||||
// The downstream wire path (apiserver ConvertStringToJsonb) splits each
|
||||
// entry at the FIRST '=' (strings.SplitN 2), so values containing '='
|
||||
// (A=b=c -> A = "b=c") round-trip fine -- dropping them here used to be
|
||||
// a silent data loss, not a necessary restriction. A nameless entry
|
||||
// (=value) cannot become a map key, so it is the one case that must be
|
||||
// rejected -- say so instead of dropping it silently.
|
||||
var envVars []string
|
||||
for _, env := range js.subEnvVars {
|
||||
// Skip environment variables with multiple equal signs
|
||||
if strings.Count(env, "=") > 1 {
|
||||
if strings.HasPrefix(env, "=") {
|
||||
fmt.Printf("warning: ignoring environment variable '%s': empty name -- name=value entries only\n", env)
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(env, "=") {
|
||||
fmt.Printf("warning: ignoring environment variable '%s': no '=' -- name=value entries only\n", env)
|
||||
continue
|
||||
}
|
||||
envVars = append(envVars, env)
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
package jobs
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
|
|
@ -133,6 +135,48 @@ func TestSubmitHelpShowsArrayForms(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// getEnvironmentVariables must keep entries whose value contains '=' (the
|
||||
// apiserver splits at the FIRST '=', so A=b=c is A -> "b=c") -- dropping
|
||||
// them used to be silent data loss. A nameless or '='-less entry cannot
|
||||
// become a map key, so it is warned about and dropped.
|
||||
func TestGetEnvironmentVariables_KeepsMultiEqualSign(t *testing.T) {
|
||||
js := NewJobSubmitter(nil)
|
||||
js.subEnvVars = []string{"A=1", "B=b=c", "C=", "=novalue", "novalue"}
|
||||
|
||||
r, w, pipeErr := os.Pipe()
|
||||
if pipeErr != nil {
|
||||
t.Fatalf("pipe: %v", pipeErr)
|
||||
}
|
||||
old := os.Stdout
|
||||
os.Stdout = w
|
||||
done := make(chan string)
|
||||
go func() {
|
||||
var buf bytes.Buffer
|
||||
_, _ = io.Copy(&buf, r)
|
||||
done <- buf.String()
|
||||
}()
|
||||
got := js.getEnvironmentVariables()
|
||||
os.Stdout = old
|
||||
_ = w.Close()
|
||||
warn := <-done
|
||||
|
||||
want := []string{"A=1", "B=b=c", "C="}
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("env = %q, want %q", got, want)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Errorf("env[%d] = %q, want %q", i, got[i], want[i])
|
||||
}
|
||||
}
|
||||
if !strings.Contains(warn, "=novalue") || !strings.Contains(warn, "warning:") {
|
||||
t.Errorf("warning output = %q, want it to name the nameless entry", warn)
|
||||
}
|
||||
if !strings.Contains(warn, "novalue") {
|
||||
t.Errorf("warning output = %q, want it to name the '='-less entry", warn)
|
||||
}
|
||||
}
|
||||
|
||||
// Attach marks the terminal-attach kind of TTY job so the worker may reap
|
||||
// ttyd when no client ever connects; --browse keeps the web-viewed
|
||||
// semantics and must not set it.
|
||||
|
|
|
|||
|
|
@ -473,6 +473,23 @@ int parse_resreq_primary(xce_primary_resreq_t* primary, char* resreqString,
|
|||
if (data.selectString != NULL && data.selectString[0] != '\0') {
|
||||
rc = parse_select(primary, data.selectString, resmgr, errmsg);
|
||||
if (rc < 0) {
|
||||
/* -ENOENT (select names an unregistered resource) is tolerated
|
||||
* by the resreq grammar -- xce_resreq.peg keeps the primary
|
||||
* without the clause (jobs rely on it, the scheduler's own
|
||||
* "type == any" default among them when no "type" resource is
|
||||
* registered). That tolerance is silent, and the early return
|
||||
* below also skips every clause that follows in this primary,
|
||||
* so say so: a job whose select and rusage are both gone is
|
||||
* scheduled with none of the constraints it asked for.
|
||||
* -EINVAL/-ENOMEM failures need no WARN here -- the grammar
|
||||
* turns them into a whole-resreq rejection the caller logs. */
|
||||
if (rc == -ENOENT) {
|
||||
WARN("resreq: select '%s' references an unregistered "
|
||||
"resource -- dropping the select and skipping any "
|
||||
"rusage[] clause of this primary; the job is scheduled "
|
||||
"without these constraints",
|
||||
data.selectString);
|
||||
}
|
||||
kill_resreq_data(&data);
|
||||
return rc;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -128,6 +128,33 @@ test_select_parse_unknown_resource(void)
|
|||
sel_optimize_test_reset();
|
||||
}
|
||||
|
||||
static void
|
||||
test_select_unknown_resource_skips_rusage(void)
|
||||
{
|
||||
/* The -ENOENT tolerance (bug-1887) drops the select AND the primary's
|
||||
* rusage clause with it (parse_resreq_primary returns before parsing
|
||||
* rusage) -- now with a WARN. Assert the documented drop semantics:
|
||||
* rc carries -ENOENT and no rusage survived, so callers can never
|
||||
* mistake the tolerated failure for a fully-parsed primary. */
|
||||
char * errmsg = NULL;
|
||||
xce_primary_resreq_t resreq;
|
||||
|
||||
var resmgr = make_test_resmgr();
|
||||
|
||||
sel_optimize_set(0);
|
||||
|
||||
int rc = parse_resreq_primary(&resreq, "unknownres==linux rusage[mem=2]",
|
||||
resmgr, &errmsg);
|
||||
TEST_ASSERT_EQUAL_INT(-ENOENT, rc);
|
||||
TEST_ASSERT_NULL(resreq.select);
|
||||
TEST_ASSERT_EQUAL_INT(0, resreq.rusages.count);
|
||||
|
||||
kill_primary_resreq(&resreq);
|
||||
FREEUP(errmsg);
|
||||
free_test_resmgr(resmgr);
|
||||
sel_optimize_test_reset();
|
||||
}
|
||||
|
||||
static void
|
||||
test_select_parse_type_mismatch(void)
|
||||
{
|
||||
|
|
@ -2346,6 +2373,7 @@ test_select(void)
|
|||
RUN_TEST(test_select_parse_valid_expr);
|
||||
RUN_TEST(test_select_parse_valid_any);
|
||||
RUN_TEST(test_select_parse_unknown_resource);
|
||||
RUN_TEST(test_select_unknown_resource_skips_rusage);
|
||||
RUN_TEST(test_select_parse_type_mismatch);
|
||||
|
||||
RUN_TEST(test_select_input_len_overflow);
|
||||
|
|
|
|||
Binary file not shown.
Binary file not shown.
Binary file not shown.
|
|
@ -1450,7 +1450,12 @@ xce_job_rusages_t* collect_job_rusages(uint64_t job_id)
|
|||
return NULL;
|
||||
}
|
||||
|
||||
snprintf(unit_name, sizeof(unit_name), "xcecd-worker-%lu.service", job_id);
|
||||
/* Build the unit name the same way start_systemd_job() did (four-segment
|
||||
* fmt_jobid_ex form) -- a bare %lu of the packed jobid never matches the
|
||||
* real unit, so every property lookup below missed and all four counters
|
||||
* stayed 0 (merge cb0c7fe9 regression). */
|
||||
make_job_sd_unit_name(unit_name, sizeof(unit_name) - 1, "xcecd-worker",
|
||||
job_id);
|
||||
char* escaped_unit_name = replace_chars(unit_name);
|
||||
if (!escaped_unit_name) {
|
||||
XCECD_LOG(XCE_LOG_ERROR, "Failed to escape unit name.");
|
||||
|
|
|
|||
|
|
@ -452,8 +452,12 @@ void apply_env_vars(env_var_t** env_vars, uint64_t jobid)
|
|||
XCECD_WORKER_LOG(XCE_LOG_DEBUG, "Applying environment variables...");
|
||||
|
||||
// Set some default XCE environment variables
|
||||
char jobid_str[32];
|
||||
snprintf(jobid_str, sizeof(jobid_str), "%lu", jobid);
|
||||
// XCE_JOBID is the four-segment written identity (cluster-apiserver-
|
||||
// timestamp-sequence, fmt_jobid) -- a bare decimal of the packed uint64
|
||||
// is unusable in scripts (output-file names, xcecli job selection all
|
||||
// speak the written form; same convention as every log line).
|
||||
char jobid_str[64];
|
||||
fmt_jobid(jobid_str, sizeof(jobid_str), jobid);
|
||||
if (setenv("XCE_JOBID", jobid_str, 1) != 0) {
|
||||
XCECD_WORKER_LOG(XCE_LOG_ERROR,
|
||||
"Error setting environment variable: %s",
|
||||
|
|
|
|||
|
|
@ -69,7 +69,16 @@ int xrun_ipc_send_remote_start(const char* hostname, const char* command)
|
|||
|
||||
char* jobid_str = getenv("XCE_JOBID");
|
||||
if (jobid_str) {
|
||||
request_msg.data.remote_start.jobid = strtoull(jobid_str, NULL, 10);
|
||||
/* XCE_JOBID is the four-segment written identity (fmt_jobid);
|
||||
* parse_jobid() accepts exactly that form (a bare decimal of the
|
||||
* packed uint64 no longer round-trips through the written form). */
|
||||
if (parse_jobid(&request_msg.data.remote_start.jobid, jobid_str)
|
||||
!= 1) {
|
||||
XCECD_WORKER_LOG(XCE_LOG_WARN,
|
||||
"xrun_ipc: cannot parse XCE_JOBID '%s'; "
|
||||
"remote start will not match the job",
|
||||
jobid_str);
|
||||
}
|
||||
}
|
||||
|
||||
if (send(sock_fd, &request_msg, sizeof(request_msg), 0) < 0) {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,8 @@
|
|||
* the submission's own variables. XCE_JOBINDEX must be the element number of
|
||||
* an array-tagged job ID (a one-element array included) and 0 for an
|
||||
* ordinary job; a submission-supplied XCE_JOBINDEX is ignored with a WARN
|
||||
* naming the job and the variable; XCE_JOBID handling is unchanged.
|
||||
* naming the job and the variable; XCE_JOBID is the four-segment written
|
||||
* identity (fmt_jobid), unchanged in override behavior.
|
||||
*
|
||||
* White-box #include of ../src/xcecd_exdata.c (same convention as the other
|
||||
* tests in this suite). XCECD_WORKER_LOG is redefined to a capture hook
|
||||
|
|
@ -203,20 +204,20 @@ static void test_similar_names_still_applied(void)
|
|||
unsetenv("xce_jobindex");
|
||||
}
|
||||
|
||||
/* 6.5: XCE_JOBID is the decimal job ID; a submission's own XCE_JOBID still
|
||||
* overrides it, as before, with no WARN. */
|
||||
/* 6.5: XCE_JOBID is the four-segment written job identity (fmt_jobid); a
|
||||
* submission's own XCE_JOBID still overrides it, as before, with no WARN. */
|
||||
static void test_jobid_unchanged(void)
|
||||
{
|
||||
env_var_t* vars = NULL;
|
||||
char dec[32];
|
||||
char idstr[64];
|
||||
|
||||
snprintf(dec, sizeof(dec), "%lu", (unsigned long)ARRAY_ELEMENT(4));
|
||||
fmt_jobid(idstr, sizeof(idstr), ARRAY_ELEMENT(4));
|
||||
apply_env_vars(&vars, ARRAY_ELEMENT(4));
|
||||
expect_env("XCE_JOBID", dec);
|
||||
expect_env("XCE_JOBID", idstr);
|
||||
|
||||
snprintf(dec, sizeof(dec), "%lu", (unsigned long)ORDINARY_JOB);
|
||||
fmt_jobid(idstr, sizeof(idstr), ORDINARY_JOB);
|
||||
apply_env_vars(&vars, ORDINARY_JOB);
|
||||
expect_env("XCE_JOBID", dec);
|
||||
expect_env("XCE_JOBID", idstr);
|
||||
|
||||
captured_count = 0;
|
||||
vars = push_var(vars, "XCE_JOBID", "user-value");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue