From f780a1660a370bffe2265a74b190b3c8b0ea6a79 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Mon, 26 Oct 2026 14:02:53 +0000 Subject: [PATCH] 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=; 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 --- .../specs/external-license-resources/brief.md | 9 +- .../external-license-resources/design.md | 44 +++++---- .wolf/buglog.json | 25 ++++- .wolf/bugs/bug-1972.md | 86 ++++++++++++++++++ src/controller/testsuite/test_job_live_gc.c | 16 +++- src/docs/xce.md | 47 +++++++++- src/frontend/xcecli/cmd/jobs/submit.go | 14 ++- src/frontend/xcecli/cmd/jobs/submit_test.go | 44 +++++++++ src/lib/resreq.c | 17 ++++ src/lib/testsuite/test_select.c | 28 ++++++ .../repl_trace_baseline/contention.replay | Bin 5384 -> 5528 bytes .../repl_trace_baseline/set_output.replay | Bin 13632 -> 13952 bytes .../repl_trace_baseline/sim_output.replay | Bin 4400 -> 4544 bytes src/worker/src/xcecd_cgroups.c | 7 +- src/worker/src/xcecd_exdata.c | 8 +- src/worker/src/xrun.c | 11 ++- src/worker/testsuite/test_exdata_jobindex.c | 17 ++-- 17 files changed, 329 insertions(+), 44 deletions(-) create mode 100644 .wolf/bugs/bug-1972.md diff --git a/.kiro/specs/external-license-resources/brief.md b/.kiro/specs/external-license-resources/brief.md index d15d266c..0daf8973 100644 --- a/.kiro/specs/external-license-resources/brief.md +++ b/.kiro/specs/external-license-resources/brief.md @@ -12,7 +12,7 @@ XCE 无法调度依赖外部 license server（FlexLM/lmstat 等）的 EDA 应用 XCE_RESF_EXTERNAL 是死 flag）。 ## Desired Outcome -- **xceextresd**：独立采集 daemon（避免 lmstat 阻塞连坐 controller）， +- **xceesd**：独立采集 daemon（避免 lmstat 阻塞连坐 controller）， 以新 `HostRole` 成员 join controller，采集器插件：flexlm（lmstat）、 script（任意外部资源）。wire 零 schema 变更（复用 ResValList 管线）， 唯一变更是 HostRole 加成员。 @@ -38,7 +38,7 @@ share 机制）。 - **elastic-shared-resources**：同管线两种记账模式按资源分流 （EXTERNAL=server-authoritative，无 flag=elastic base+borrowed）—— opupd 守卫"SHARED 即收"预留其写入路径。即对其决策日志 P9 的回答。 -- **PR-2 proxy**（resource-questions.md）：xceextresd 定位为该 proxy 的 +- **PR-2 proxy**（resource-questions.md）：xceesd 定位为该 proxy 的 第一片（只读 report 能力），HostRole 只加一个成员。 - **project-config**：per-app 配额→per-project share（project tree 是 唯一并发上限处，limits.yaml 退役）。需要其扩展：集群级 pool location。 @@ -48,7 +48,7 @@ share 机制）。 ## Boundary Candidates - HostRole_extres join/leave + opupd 准入守卫（controller） - unsatisfy_ EXTERNAL 分支（scheduler） -- xceextresd daemon 与采集器插件（新 src/extresd/） +- xceesd daemon 与采集器插件（新 src/esd/） - mapping.py `location: [external]` 路由 ## Out of Boundary @@ -64,6 +64,7 @@ share 机制）。 provisioning-demand（headroom 需求源）、project-config（集群级池） ## Open（待用户/跨团队决策） -1. 命名：xceextresd（推荐）vs xcelsd。 +1. ~~命名~~ 已定（2026-10-27）：**xceesd**（src/esd/，esd.yaml；HostRole + 枚举成员 extres）。 2. 断联策略：fail-open（推荐）vs fail-closed。 3. P9 模式拆分与三个扩展点需与 Rongsong 确认（design §3.6）。 diff --git a/.kiro/specs/external-license-resources/design.md b/.kiro/specs/external-license-resources/design.md index 7f16e2c9..5dff37ce 100644 --- a/.kiro/specs/external-license-resources/design.md +++ b/.kiro/specs/external-license-resources/design.md @@ -110,17 +110,23 @@ license 服务器的"当前空闲数"是外部真值；本地只需补一个窗 ### 3.1 架构决策：独立 daemon + 采集器插件 -**xceextresd（外部资源守护进程）**，新 `src/extresd/`，C 实现， -照 scheduler 的 join 方式以新 `HostRole_extres` 角色接入 controller。 -内部采集器插件接口： +**xceesd（外部资源守护进程；XCE External-resources Daemon）**，新 +`src/esd/`，C 实现，照 scheduler 的 join 方式以新 `HostRole_extres` +角色接入 controller。内部采集器插件接口： ``` -xceextresd +xceesd ├─ collector/flexlm.c — lmstat -c port@host -f feature 解析（EDA license 主路径） ├─ collector/script.c — 用户脚本输出 key=value/JSON（任意外部资源：云配额、网络带宽…） └─ collector/http.c —（预留） ``` +> **命名**（2026-10-27 定）：daemon 名 `xceesd`，目录 `src/esd/`，二进制 +> `xceesd`，配置 `esd.yaml`，代码标识符 `esd`。wire 层 HostRole 枚举成员 +> 仍用 `extres`（角色语义，与 reporter/provisioner 讨论对齐）。 +> 注意 `esd` 是高频子串（closed/parsed…），全文检索认准全名。 +> 旧稿中的 `xceextresd`/`extresd` 均指本 daemon（改名前写法）。 + - **为什么不是 controller 内插件**：lmstat 是秒级阻塞调用，违反 controller async I/O 红线（delaymgr/vtask 不得阻塞，HA/心跳会被连坐）；license server 故障必须与 controller 故障域隔离。 @@ -133,7 +139,7 @@ xceextresd ### 3.2 数据流（wire 零 schema 变更，除 HostRole 加成员） ``` -xceextresd (join: HostRole_extres) +xceesd (join: HostRole_extres) │ OPCode_update / Update_data_resvals（现 wire：ResourceVal{name, host:"", val}） ▼ opupd.c do_update_resvals ── 放行 extres 角色；flag 守卫只许 SHARED 资源（§3.6） @@ -185,7 +191,7 @@ EXTERNAL：F ≥ uncomit + wanted（unsatisfy_ 循环内一个 flag 分支； 与 Slurm `_handle_consumed` 的 deficit 算法同构（都是"避开外部消费者份额"）， 但更简：不需要 LastConsumed/external 推导，因为 lmstat 直接给 free。 -### 3.3b elim（LSF load index）兼容性 —— 已盘点，归 xcecd 不归 extresd +### 3.3b elim（LSF load index）兼容性 —— 已盘点，归 xcecd 不归 esd **LSF elim 模型**：用户在主机上部署脚本，周期 echo `资源名 数量`，lim 收集后 变成可调度资源。per-host 语义。License Scheduler 历史上正是从 elim 派生—— @@ -217,15 +223,15 @@ posix_spawn 路径；读 stdout `name value` 行 elim 原格式；调现有 regi API 进 registry；脚本超时看门狗 kill）。MAX_LOAD_INDICES=64 够用。 工作量 D 类（中型），与 E2 独立可并行。 -**归属决策：elim 支持放 xcecd，不放 xceextresd**。理由： +**归属决策：elim 支持放 xcecd，不放 xceesd**。理由： 1. 作用域——elim 值 per-host（"这台机器有多少"），license 值集群全局 - （"全池还剩多少"）；worker 本来就站在每台主机上，让 extresd 管主机值 + （"全池还剩多少"）；worker 本来就站在每台主机上，让 esd 管主机值 是数据绕远路。 2. 故障域——elim 脚本挂只影响一台主机上报；license server 断联影响全集群 准入。xcecd 采集器继承前者的隔离性。 3. LSF 自己也是分开的（lim vs lsld/blcollect），复刻这个分界。 4. XCE 架构两条腿天然对齐：worker→per-worker resvals→host 判式； - extresd→sharedinst→全局判式。同一资源模型（定义/rusage/pend reason/ + esd→sharedinst→全局判式。同一资源模型（定义/rusage/pend reason/ CLI 展示）。 **统一记账规则（两条腿共享，同一 flag 位决定）**：EXTERNAL 资源的满足性 @@ -252,13 +258,13 @@ count) 聚合——无法判定 A=2/B=3 还是 A=3/B=2。LSF License Scheduler daemon 每轮对账：Σ(派发中) vs 服务器 used，差值 = 外部消费者 + 泄漏， 异常 alert。漂移有界且方向安全：少拿→配额保守不超卖；多拿→服务器 total 暴露。 -3. **报告层 = 证据分级 + 残差桶**（matcher 为 extresd 插件，不猜、显式 +3. **报告层 = 证据分级 + 残差桶**（matcher 为 esd 插件，不猜、显式 化"不知道"）： - Tier 0：派发记账（app→授予数），精确，永远可用； - Tier 1：vendor checkout 属性（如 Synopsys LM_PROJECT 类，lmstat 可见； XCE 作业 env 已注入 XCE_JOBID，vendor 走 env 即可盖入），精确； - Tier 1'：PID 匹配（部分 vendor daemon 报客户端 PID）——jobPidInfo - 已有 jobid→pid 映射（3545aa0d），extresd 从 controller 拉即可，精确； + 已有 jobid→pid 映射（3545aa0d），esd 从 controller 拉即可，精确； - Tier 2：(user, host) + checkout start 时间窗 匹配作业记录（LSF 基线 法升级：两个独立 XCE 作业通常可按 start 时间拆开），user+host 相同时 只能给出"这批作业合计持有 K"的界； @@ -287,7 +293,7 @@ resources: ``` ```yaml -# extresd.yaml（daemon 侧：操作） +# esd.yaml（daemon 侧：操作） poll_interval: 30s resources: - name: Verilog_XL # 必须与 resources.yaml 同名（控制器侧 unknown-resource 拒） @@ -300,7 +306,7 @@ resources: stale_limit: 150s # 连续取不到 → WARN 保旧值（fail-open，可配 fail-closed 清零） ``` -职责分界：resources.yaml 管声明（作业能否 rusage），extresd.yaml 管采集 +职责分界：resources.yaml 管声明（作业能否 rusage），esd.yaml 管采集 （值从哪来）。`seed_sharedinst_from_config` 对 EXTERNAL 资源**不播种** （值只来自 daemon；daemon 未上线时 get_resvals 读 0 → PEND NO_RESOURCE detail 带资源名）。 @@ -317,7 +323,7 @@ detail 带资源名）。 3. resource.c unsatisfy_：EXTERNAL 跳过 inuse 项（+trace 行） 4. cluster.c seed：EXTERNAL 不播种 5. mapping.py：`location: [external]` 路由 -6. xceextresd 骨架：join/rejoin/failover 重连即全量重推（worker 状态补报 +6. xceesd 骨架：join/rejoin/failover 重连即全量重推（worker 状态补报 9d7628e 同款语义）+ script/flexlm 采集器 + 轮询去抖 7. TDD：testsuite 红→绿（opupd 守卫、unsatisfy_ 分支、采集器解析）； t1 E2E 用 script 采集器 + 假 license 脚本（不必有真 FlexLM） @@ -335,7 +341,7 @@ PEND detail 含 lic 名）；daemon kill/重启后值全量补推；controller daemon 重新 join 新主并补推；`--sim` 录制含 daemon 推送事件且回放一致。 **elim 归属（与 E2 并行的独立 D 类项）**：LSF elim（per-host 外部资源脚本） -归 xcecd（worker）不归 xceextresd——详见 §3.3b。license（全局）与 elim +归 xcecd（worker）不归 xceesd——详见 §3.3b。license（全局）与 elim （主机）是两条腿、同一资源模型、同一记账规则（EXTERNAL 跳 inuse）。 ### 3.6 与 Rongsong 统一资源模型 spec 群的合并性审查（2026-10-26） @@ -379,13 +385,13 @@ daemon 推 free 直接覆盖 sharedInst。两套各写各的 → 同一资源 sh 唯一并发上限处，limits.yaml 退役"（332f86ea）；§3.3c Tier-0 配额写成 project share/member_quota 由 limit plugin 编译，不发明新机制。 - staleness/interval 进**资源描述符**（其 Phase 1 descriptor 字段 - interval/last_report 已预留），不放 extresd.yaml；daemon 只执行。 + interval/last_report 已预留），不放 esd.yaml；daemon 只执行。 - 持久化分级：daemon 推的 F 是 dynamic reported value → 非 WAL fact、 进 scheduler snapshot + daemon 重连全量补推即安全；将来 elastic 模式 的 total 才是 WAL fact。与 persistence.md 一致。 - 角色收敛：PR-2 proxy（leaning）统一 reporter+provisioner 为一个 daemon role；**HostRole 只加一个成员**（避免将来再出现第二个同义 enum）， - xceextresd 定位为 PR-2 proxy 的第一片（只读 report 能力）。 + xceesd 定位为 PR-2 proxy 的第一片（只读 report 能力）。 - 归因 Tier 1' PID 匹配引用其 jobPidInfo（我们已实施，3545aa0d）。 **我们的设计对其 spec 的三个扩展点**（他目前装不下，需带回讨论）： @@ -407,8 +413,8 @@ daemon 推 free 直接覆盖 sharedInst。两套各写各的 → 同一资源 sh "or reuse the provisioner role"）。 ### 3.7 提请决策（未决） -1. daemon 命名：`xceextresd`（通用外部资源，推荐）vs `xcelsd`（贴 LSF - license 语义）；定位为 PR-2 proxy 第一片（见 §3.6）。 +1. ~~daemon 命名~~ 已定（2026-10-27）：`xceesd` / `src/esd/` / `esd.yaml`； + HostRole 枚举成员仍为 `extres`；定位为 PR-2 proxy 第一片（见 §3.6）。 2. 断联默认策略：fail-open（保旧值+stale WARN，推荐）vs fail-closed（清零停派）。 3. P9 模式拆分与三个扩展点需与 Rongsong 确认（§3.6）。 diff --git a/.wolf/buglog.json b/.wolf/buglog.json index d4f2c157..9985afdf 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -6,7 +6,7 @@ "id": "bug-1971", "status": "fixed", "status_updated": "2026-10-22", - "status_note": "", + "status_note": "two-choke archive in xce-build.sh; test harness green", "component": "build", "severity": "wrong-result", "timestamp": "2026-10-22T07:50:00Z", @@ -27,6 +27,29 @@ "68c3ccd7" ], "record": ".wolf/bugs/bug-1971.md" + }, + { + "id": "bug-1972", + "status": "open", + "status_updated": "2026-10-26", + "status_note": "fix in working tree, commit pending with A-batch", + "component": "controller", + "severity": "crash", + "timestamp": "2026-10-26T09:46:58Z", + "last_seen": "2026-10-26T09:46:58Z", + "occurrences": 4, + "error_message": "test_job_live_gc asserted destruction by reading freed memory\n\n**Observed:**\n`controller_unit_test` aborts under ASAN at `test_sweep_purges_expired_done_job`\n(test_job_live_gc.c:169):\n\n```\n==27834==ERROR: AddressSanitizer: heap-use-after-free on address 0x5110000da290\nREAD of size 8 at 0x5110000da290 thread T0\n #0 job_get_refcount ../lib/jobj.c:715\n #1 test_sweep_purges_expired_done_job test_job_live_gc.c:169\nfreed by:\n #3 job_dereference ../lib/jobj.c:705\n #5 job_live_gc_sweep ../controller/job.c:4777\n```\n\n**Expected:** a correct sweep dereferences the purged job down to refcount 0\nand destroys it; the test must verify destruction without touching the freed\nobject.\n\n**Impact:** controller suite aborts (killed by SIGABRT); the whole\n`controller test` meson entry fails. Test-only — the production sweep\n(job.c:4765-4777, double dereference: collection ref + table's makeJob ref)\nis correct.\n\n**Found by:** ASAN, first full-suite run after d028c8c3 (the archives' earlier\nruns all predate that commit).", + "file": "src/controller/testsuite/test_job_live_gc.c", + "root_cause": "The assertion `TEST_ASSERT_EQUAL_UINT32(0, job_get_refcount(expired))` runs\nAFTER `job_live_gc_sweep` has already dereferenced the job to 0 and freed the\nentire RTTI object (header + private + data in one calloc block). Reading the\nrefcount through the dangling `job_t*` is itself the use-after-free: the\nstronger the sweep does its job, the more surely the test crashes. The same\npattern is absent from test_apply_pump.c's refcount test because there the\njob is never destroyed.", + "fix": "Move the refcount probe BEFORE the sweep and invert its meaning: assert\nrefcount == 1 (table's seed reference, nothing else) before `job_live_gc_sweep`,\nthen verify destruction by absence — `jlg_find_job(81001) == NULL` plus the\nidempotent second sweep (both already present). `expired` is never dereferenced\nafter the sweep; a comment marks the last safe point.\n\nVerification:\n- Test: test_sweep_purges_expired_done_job (same file) — RED before (deterministic\n ASAN abort 4/4), GREEN after (rc=0, 1354/1354 controller unity tests, zero\n ASAN reports).\n- Suites: controller_unit_test 1354 tests 0 failures (ASAN, detect_leaks=0);\n sched_repl_trace_baseline_test 4/4 with regenerated fixtures (separate issue,\n bug-free fixture staleness from fc33a5da's schema change, handled in the same\n working tree).\n- Live: n/a (test-only defect; production sweep unchanged).", + "tags": [ + "test-only", + "uaf", + "gc" + ], + "related_bugs": [], + "fix_commits": [], + "record": ".wolf/bugs/bug-1972.md" } ] } diff --git a/.wolf/bugs/bug-1972.md b/.wolf/bugs/bug-1972.md new file mode 100644 index 00000000..3d47007f --- /dev/null +++ b/.wolf/bugs/bug-1972.md @@ -0,0 +1,86 @@ +--- +id: "bug-1972" +status: "open" +status_updated: "2026-10-26" +status_note: "fix in working tree, commit pending with A-batch" +component: "controller" +severity: "crash" +timestamp: "2026-10-26T09:46:58Z" +last_seen: "2026-10-26T09:46:58Z" +occurrences: 4 +files: ["src/controller/testsuite/test_job_live_gc.c"] +tags: ["test-only", "uaf", "gc"] +related: [] +fix_commits: [] +--- + +# bug-1972: test_job_live_gc asserted destruction by reading freed memory + +## Description +**Observed:** +`controller_unit_test` aborts under ASAN at `test_sweep_purges_expired_done_job` +(test_job_live_gc.c:169): + +``` +==27834==ERROR: AddressSanitizer: heap-use-after-free on address 0x5110000da290 +READ of size 8 at 0x5110000da290 thread T0 + #0 job_get_refcount ../lib/jobj.c:715 + #1 test_sweep_purges_expired_done_job test_job_live_gc.c:169 +freed by: + #3 job_dereference ../lib/jobj.c:705 + #5 job_live_gc_sweep ../controller/job.c:4777 +``` + +**Expected:** a correct sweep dereferences the purged job down to refcount 0 +and destroys it; the test must verify destruction without touching the freed +object. + +**Impact:** controller suite aborts (killed by SIGABRT); the whole +`controller test` meson entry fails. Test-only — the production sweep +(job.c:4765-4777, double dereference: collection ref + table's makeJob ref) +is correct. + +**Found by:** ASAN, first full-suite run after d028c8c3 (the archives' earlier +runs all predate that commit). + +## Environment +- OS: Ubuntu 24.04.2 LTS; kernel Linux 6.8.0-147-generic x86_64 +- libc: glibc 2.39 (host ldd) +- Toolchain: gcc (GCC) 15.2.0, meson 1.9.1 +- Build: asan (xce.build) +- Code: feature/license-resources @ 68eabd8a-dirty (introduced in d028c8c3) +- Deployment: n/a (unit test) + +## Reproduction +1. `cd src/xce.build && ASAN_OPTIONS=detect_leaks=0 ./controller/controller_unit_test` +- Frequency: deterministic (4/4 runs, 2026-10-26) +- Evidence: /home/lchen/code/xce/src/testlogs.archive/xce.build/this-20261026-082318-rc1 + +## Root cause +The assertion `TEST_ASSERT_EQUAL_UINT32(0, job_get_refcount(expired))` runs +AFTER `job_live_gc_sweep` has already dereferenced the job to 0 and freed the +entire RTTI object (header + private + data in one calloc block). Reading the +refcount through the dangling `job_t*` is itself the use-after-free: the +stronger the sweep does its job, the more surely the test crashes. The same +pattern is absent from test_apply_pump.c's refcount test because there the +job is never destroyed. + +## Fix +Move the refcount probe BEFORE the sweep and invert its meaning: assert +refcount == 1 (table's seed reference, nothing else) before `job_live_gc_sweep`, +then verify destruction by absence — `jlg_find_job(81001) == NULL` plus the +idempotent second sweep (both already present). `expired` is never dereferenced +after the sweep; a comment marks the last safe point. + +## Verification +- Test: test_sweep_purges_expired_done_job (same file) — RED before (deterministic + ASAN abort 4/4), GREEN after (rc=0, 1354/1354 controller unity tests, zero + ASAN reports). +- Suites: controller_unit_test 1354 tests 0 failures (ASAN, detect_leaks=0); + sched_repl_trace_baseline_test 4/4 with regenerated fixtures (separate issue, + bug-free fixture staleness from fc33a5da's schema change, handled in the same + working tree). +- Live: n/a (test-only defect; production sweep unchanged). + +## References +- Related: none diff --git a/src/controller/testsuite/test_job_live_gc.c b/src/controller/testsuite/test_job_live_gc.c index 2a820774..03bef978 100644 --- a/src/controller/testsuite/test_job_live_gc.c +++ b/src/controller/testsuite/test_job_live_gc.c @@ -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)); diff --git a/src/docs/xce.md b/src/docs/xce.md index 18b3b5d0..e8f292ee 100644 --- a/src/docs/xce.md +++ b/src/docs/xce.md @@ -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 ""` (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[]`** — 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[=[:duration=[: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 diff --git a/src/frontend/xcecli/cmd/jobs/submit.go b/src/frontend/xcecli/cmd/jobs/submit.go index 1da4a801..86134c46 100755 --- a/src/frontend/xcecli/cmd/jobs/submit.go +++ b/src/frontend/xcecli/cmd/jobs/submit.go @@ -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) diff --git a/src/frontend/xcecli/cmd/jobs/submit_test.go b/src/frontend/xcecli/cmd/jobs/submit_test.go index 3ccf2e74..d2ac9466 100644 --- a/src/frontend/xcecli/cmd/jobs/submit_test.go +++ b/src/frontend/xcecli/cmd/jobs/submit_test.go @@ -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. diff --git a/src/lib/resreq.c b/src/lib/resreq.c index fad1e46a..1ee31b70 100644 --- a/src/lib/resreq.c +++ b/src/lib/resreq.c @@ -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; } diff --git a/src/lib/testsuite/test_select.c b/src/lib/testsuite/test_select.c index 35dbd914..df299f69 100644 --- a/src/lib/testsuite/test_select.c +++ b/src/lib/testsuite/test_select.c @@ -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); diff --git a/src/scheduler/testsuite/fixtures/repl_trace_baseline/contention.replay b/src/scheduler/testsuite/fixtures/repl_trace_baseline/contention.replay index a79b84a128197a8c52e5e07181d6cf5348a13a12..8e04013d81517b9abece6c862131b704cf1a85d3 100644 GIT binary patch delta 250 zcmeCsnxQ=*Q^hRAz|zXt#LC1xih+TFA*nPorzA5kJ}o(|7{mwRo{7D|l5ZG6Oh$%} zj0_Cy3=9Z*;?Gz{md)XeyiDB385kIN7#J8>7+5CfGb<{_GB7Y0F)%QIXf6f@23H1% zUXV#3%s9D_Szb1jfq_AZfq?-;gH+iftD1b4Sy|nefq}t^fq?-;gH)Izs{k1U#!Qnh kGD=RK!OAnahlz_3%wx_?&7J5d4B7?>vKGb;+3GB7Y0F)%QIXf6f@2FA&S z%<{ad3=9lP3=9k)nrre~X61vzoz`zAkGdWUJo;Q?%fkBCZfdNEwO};9s%bU=ARrixjJfu)tPiIs_Y6axbTLsDsGPDy57d|Gl^F^CVsJrjF_CEqZDn2Zb` z85tPZ85j`s#GkQ@EStj_d6~G6GcYjlFfcH%FtAL{XI4~;0Shkx*Fh&; delta 186 zcmX@0yg_L~rixLBp^=q|nU#@Y6axbTLsDsGPDy57d|Gl^F^CVsRTF!IC6_RQn2Zc7 z85tPZ85j`s#GkQ@Oq;_Qd6~GUGcYjlFfcH%FfdKdXI2z6Wnf@1VqjnZ(Oe7+42+Wt zndNy^85kIp7#J8pG}q*}%*qoDSX7)C7#Ktu7#KjxigWTy;FdEmFyyA@g6w9PoWLQm I_y7wp0O0~3E&u=k diff --git a/src/worker/src/xcecd_cgroups.c b/src/worker/src/xcecd_cgroups.c index e3c84e66..78321ca5 100644 --- a/src/worker/src/xcecd_cgroups.c +++ b/src/worker/src/xcecd_cgroups.c @@ -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."); diff --git a/src/worker/src/xcecd_exdata.c b/src/worker/src/xcecd_exdata.c index 78e6163c..f68edf4f 100644 --- a/src/worker/src/xcecd_exdata.c +++ b/src/worker/src/xcecd_exdata.c @@ -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", diff --git a/src/worker/src/xrun.c b/src/worker/src/xrun.c index 6006dbb4..c86baeec 100644 --- a/src/worker/src/xrun.c +++ b/src/worker/src/xrun.c @@ -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) { diff --git a/src/worker/testsuite/test_exdata_jobindex.c b/src/worker/testsuite/test_exdata_jobindex.c index b6f33773..bcff9cb1 100644 --- a/src/worker/testsuite/test_exdata_jobindex.c +++ b/src/worker/testsuite/test_exdata_jobindex.c @@ -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");