lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(apiserver): fix job control union discriminant, add human-readable job ID support (gitea#45)

Browse source 
ControlJob() set xce_jobctrl_t.action to the job_ctrl_type_t action code
(kill/stop/resume/checkpoint), but that field is actually the capnp
JobControl_u_which union discriminant -- a different enum entirely. This
made kill/resume/checkpoint hit request.c's -EINVAL default case, and made
stop collide with the checkpoint discriminant and read through the wrong
union member. Live-verified against a real deployed cluster: kill/stop/
resume/checkpoint now behave correctly with no apiserver crash.

Also additively accept a human-readable job ID ("{cluster}-{apiserver}-
{timestamp}-{seq}") on the job control endpoint, alongside the existing
numeric form xcecli already translates to -- lets a caller hit the REST
API directly without reimplementing xcecli's ID packing. Live-verified via
a direct curl call with the human-readable ID.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hb6Edtj4KbS8CrgEWYdA2E
This commit is contained in:
Rongsong Shen 2026-09-07 08:31:54 +00:00
parent 5ab344e1ae
commit 099dbdabdc
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
5 changed files with 191 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

53
.wolf/buglog.json
View file

@ -21206,6 +21206,59 @@
"related_bugs": [],
"occurrences": 1,
"last_seen": "2026-09-07T07:55:00Z"
},
{
"id": "bug-1781",
"timestamp": "2026-09-07T08:35:00Z",
"gitea_issue": 45,
"error_message": "Gitea issue #45 REOPENED after bug-1742's fix. User-reproduced: after bug-1742 fixed the earlier panic/empty-error-body symptom, `xcecli jobs kill <jobid>` (and stop/resume/checkpoint) now fails with a real, non-empty error instead: `request to /xce/v1/jobs/<id>/control failed: [failed to write data: xcecli_jobctrl failed: -22]` (-22 = -EINVAL). Reproduced live for kill/stop/resume/checkpoint, all failing.",
"file": "src/frontend/apiserver/backend/clib.go (ControlJob), src/lib/request.c (xce_jobctrl_req), src/lib/controller.capnp.h (JobControl_u_which), src/include/xce/types.h (xce_jobctrl_t), src/controller/job.c (job_ctrl)",
"root_cause": "ControlJob() (clib.go) set xce_jobctrl_t.action to the job_ctrl_type_t action code (models.JOB_STOP=1, JOB_RESUME=2, JOB_KILL=3, JOB_CHKPNT=8, src/include/xce/types.h job_ctrl_type_t) -- but that field is actually the capnp JobControl_u_which UNION DISCRIMINANT (JobControl_u_siginfo=0, JobControl_u_ckptoption=1, src/lib/controller.capnp.h), a completely different enum that says which branch of xce_jobctrl_t.u (siginfo vs ckptoption) is populated. xce_jobctrl_req() (src/lib/request.c:2200) and the controller's job_ctrl() (src/controller/job.c:1972, for a job with no allocs yet) both switch on ctrinfo->action expecting the discriminant values 0/1, not the job_ctrl_type_t codes. Consequences per action: kill(3) and resume(2) never match 0 or 1, so xce_jobctrl_req's `default: return -EINVAL` fires client-side before the request is even sent to the controller -- this is the exact -22 in the bug report. checkpoint(8) hits the same default-EINVAL path, so checkpoint was ALSO silently broken (never reached the controller's real checkpoint-eligibility check). stop(1) is the worst case: it numerically collides with JobControl_u_ckptoption(1), so xce_jobctrl_req took the ckptoption branch and read `ctrinfo->u.ckptoption->method` through a pointer that was actually populated as `xce_signal_t*` (via jobctrl_set_siginfo, clib.go) -- a wrong-typed union-member read, undefined behavior in the apiserver's own address space, not merely a clean rejection. The `signo`-based dispatch already correctly threaded through job_ctrl()'s inner switch (SIGCONT->resume, SIGSTOP/SIGTSTP->stop, SIGKILL/SIGTERM/default->kill on the not-yet-started-job branch) proves the intended design: `action` is the union tag, and `u.siginfo->signo` is what actually distinguishes stop/resume/kill -- ControlJob() conflated the two enums when translating the caller's action into the wire struct.",
"fix": "FIXED and LIVE-VERIFIED (2026-09-07). Added jobCtrlUnionDiscriminant(action int) int to convert_pure.go (pure Go, no CGO types, alongside resolveExecUser from bug-1780) -- returns JobControl_u_ckptoption(1) for JOB_CHKPNT, JobControl_u_siginfo(0) for JOB_STOP/JOB_RESUME/JOB_KILL. ControlJob() (clib.go) now sets `ctrl.action = C.int(jobCtrlUnionDiscriminant(action))` instead of the raw action code; the existing jobctrl_set_siginfo/jobctrl_set_ckptoption C helpers (already correct, only ever touched ctrl.u) are unchanged. TDD: TestJobCtrlUnionDiscriminant_StopResumeKillUseSiginfo and TestJobCtrlUnionDiscriminant_ChkptUsesCkptOption (convert_test.go), passing under CGO_ENABLED=0 go test (RED before the function existed). Verified with a full /xce-build apiserver asan --incremental (exit 0, apiserver/backend real-CGO suite green). LIVE-VERIFIED beyond static analysis (learning from bug-1742's own gap, which was static-analysis-only and left this exact defect in place): deployed a real single-node cluster (controller+scheduler+worker+apiserver) via deploy-ha.sh, submitted a job that stays PEND (unsatisfiable select[] so it never dispatches -- exercises job_ctrl()'s not-yet-started branch, the exact code path this bug hits, without needing job execution), and confirmed via the real built xcecli binary: `jobs kill` succeeds (job removed from listing, previously -22), `jobs stop`/`jobs resume` succeed with no apiserver crash (previously read through the wrong union member -- confirmed apiserver process survived and stayed responsive), and `jobs checkpoint` on an unstarted job now correctly gets a clean business-rule rejection (\"cannot be <checkpoint>\") instead of a spurious client-side -EINVAL.",
"tags": [
"apiserver",
"capnp",
"clib.go",
"job-control",
"kill",
"stop",
"resume",
"checkpoint",
"union-discriminant",
"undefined-behavior",
"gitea-45",
"live-verified",
"fixed"
],
"related_bugs": [
"bug-1742"
],
"occurrences": 1,
"last_seen": "2026-09-07T08:35:00Z"
},
{
"id": "bug-1782",
"timestamp": "2026-09-07T08:40:00Z",
"gitea_issue": 45,
"error_message": "Not a defect -- API ergonomics follow-up discussed while fixing bug-1781/gitea#45. The job control REST endpoint (`/xce/v1/jobs/{jobids}/control`) only accepted the packed numeric job ID; xcecli already translates the human-readable form (e.g. `1-0-1788768547-0`, as printed by `jobs list`) to that numeric form client-side (format.ToAPIJobID) before calling the API, so a caller hitting the endpoint directly (not through xcecli) had to reimplement that bit-packing itself.",
"file": "src/frontend/apiserver/backend/convert_pure.go (ParseHumanJobID, new), src/frontend/apiserver/controllers/jobs.go (ControlJobByID)",
"root_cause": "N/A -- enhancement, not a bug. Existing numeric-only behavior was correct and intentional.",
"fix": "ADDED (2026-09-07), additive/backward-compatible -- the numeric form is untouched and still works exactly as before. Added ParseHumanJobID(s string) (uint64, bool) to convert_pure.go: parses a plain human-readable job ID (\"{cluster}-{apiserver}-{timestamp}-{sequence}\", no array brackets -- controllers/jobs.go's parseArrayName already strips any \"[idx]\" suffix before this runs) back into the packed numeric ID, reusing the bit-layout constants this file already duplicates from xcecli/common/format/jobid.go for the read path (FormatJobID/ExtractArrayBaseID). Works correctly for both a plain job's own ID and an array child's dash-quad prefix (verified equal to ExtractArrayBaseID(original) in the array case -- the two enums are NOT ambiguous here, unlike bug-1781). ControlJobByID (jobs.go) now falls back to ParseHumanJobID when jobid.KeyID isn't numeric, before erroring; numeric IDs skip this path entirely. TDD: TestParseHumanJobID_NonArrayRoundTripsWithFormatJobID, TestParseHumanJobID_ArrayChildPrefixMatchesBaseID, TestParseHumanJobID_RejectsNumeric, TestParseHumanJobID_RejectsGarbage (convert_test.go), passing under CGO_ENABLED=0 go test. Verified with a full /xce-build apiserver asan --incremental (exit 0). LIVE-VERIFIED: against the same deployed cluster used for bug-1781, called POST /xce/v1/jobs/{jobids}/control directly via curl with the URL-encoded human-readable ID (bypassing xcecli's own translation entirely) and confirmed the job was correctly found and killed (HTTP 202, job removed from listing).",
"tags": [
"apiserver",
"api-design",
"job-control",
"human-readable-id",
"gitea-45",
"enhancement",
"live-verified",
"fixed"
],
"related_bugs": [
"bug-1781"
],
"occurrences": 1,
"last_seen": "2026-09-07T08:40:00Z"
}
]
}

5
src/frontend/apiserver/backend/clib.go
View file

@ -664,7 +664,10 @@ func SubmitJob(job *models.JobInfo, arraySize int) error {
func ControlJob(jobid uint64, action int, content string) error {
var ctrl C.xce_jobctrl_t
ctrl.jobid = C.uint64_t(jobid)
ctrl.action = C.int(action)
// bug-1781/gitea#45: xce_jobctrl_t.action holds the JobControl_u_which
// capnp union discriminant, not the job_ctrl_type_t action code -- the
// two enums must not be conflated.
ctrl.action = C.int(jobCtrlUnionDiscriminant(action))
// For signal-based actions set signo; for checkpoint set ckpt options.
// xce_jobctrl_t.u is a C union — we use a static C helper to set the

50
src/frontend/apiserver/backend/convert_pure.go
View file

@ -3,6 +3,8 @@ package backend
import (
"apiserver/models"
"encoding/json"
"regexp"
"strconv"
"time"
"fmt"
)
@ -98,12 +100,60 @@ func FormatJobID(uniqueJobID int64) string {
clusterID, apiserverID, timestamp, sequence)
}
// humanJobIDPattern matches a plain human-readable job ID, "{cluster}-
// {apiserver}-{timestamp}-{sequence}" (no array-index brackets).
var humanJobIDPattern = regexp.MustCompile(`^(\d+)-(\d+)-(\d+)-(\d+)$`)
// ParseHumanJobID parses a plain human-readable job ID (as produced by
// FormatJobID, brackets already stripped by the caller) back into the
// packed numeric job ID the job_id/unique_job_id DB columns store. Returns
// ok=false if s doesn't match that shape (e.g. it's already numeric).
//
// Deliberately does not itself handle an "[idx]" array-child suffix:
// controllers/jobs.go's parseArrayName already splits that off (for the
// API's own array-index filter syntax) before this ever runs, leaving a
// plain quad whose 4th number is the packed ID's low sequence bits with
// tag=0 -- true for a real non-array job's own ID, and also true for an
// array child's dash-quad prefix (FormatJobID's arrayBase is those same
// bits already shifted into position with tag/idx zeroed, i.e. exactly
// ExtractArrayBaseID(original)).
func ParseHumanJobID(s string) (uint64, bool) {
m := humanJobIDPattern.FindStringSubmatch(s)
if m == nil {
return 0, false
}
clusterID, _ := strconv.ParseUint(m[1], 10, 64)
apiserverID, _ := strconv.ParseUint(m[2], 10, 64)
timestamp, _ := strconv.ParseUint(m[3], 10, 64)
sequence, _ := strconv.ParseUint(m[4], 10, 64)
return (clusterID << clusterIDShift) |
(apiserverID << apiserverIDShift) |
(timestamp << timestampShift) |
sequence, true
}
// WorkerDefaultOutputPath returns the output file path the worker uses by default
// for the given job ID and task index, matching uv_proc.c make_job_output_path().
func WorkerDefaultOutputPath(uniqueJobID int64, taskIdx int) string {
return fmt.Sprintf("/tmp/job_%s_task%d.out", FormatJobID(uniqueJobID), taskIdx)
}
// jobCtrlUnionDiscriminant returns the value xce_jobctrl_t.action must carry
// for the given job_ctrl_type_t action (models.JOB_STOP/RESUME/KILL/CHKPNT):
// the capnp JobControl_u_which union discriminant (JobControl_u_siginfo=0,
// JobControl_u_ckptoption=1 -- src/lib/controller.capnp.h), NOT the action
// code itself. xce_jobctrl_req() (src/lib/request.c) switches on this field
// and rejects anything else with -EINVAL; ControlJob (clib.go) previously
// stored the raw action code there instead, so kill(3)/resume(2) always hit
// that -EINVAL default case (gitea issue #45) while stop(1) happened to
// collide with ckptoption(1) and read through the wrong union member.
func jobCtrlUnionDiscriminant(action int) int {
if action == models.JOB_CHKPNT {
return 1 // JobControl_u_ckptoption
}
return 0 // JobControl_u_siginfo (stop/resume/kill all signal the job)
}
// resolveExecUser returns the effective exec user for a job submission: the
// caller-specified ExecUser if present, otherwise the submitting user
// (bug-1780: an unset execuser in the startjob request reaches xcecd as

71
src/frontend/apiserver/backend/convert_test.go
View file

@ -3,6 +3,7 @@ package backend
import (
"apiserver/models"
"encoding/json"
"fmt"
"testing"
)
@ -28,6 +29,76 @@ func TestResolveExecUser_EmptyWhenBothEmpty(t *testing.T) {
}
}
// gitea#45/bug-1783: xce_jobctrl_t.action must carry the capnp
// JobControl_u_which union discriminant (siginfo=0, ckptoption=1), not the
// job_ctrl_type_t action code -- conflating the two made every kill/resume
// call hit request.c's -EINVAL default case, and every stop call
// (action code 1) collide with ckptoption's discriminant (also 1) and read
// through the wrong union member.
func TestJobCtrlUnionDiscriminant_StopResumeKillUseSiginfo(t *testing.T) {
for _, action := range []int{models.JOB_STOP, models.JOB_RESUME, models.JOB_KILL} {
if got, want := jobCtrlUnionDiscriminant(action), 0; got != want {
t.Errorf("jobCtrlUnionDiscriminant(%d): got %d, want %d (JobControl_u_siginfo)", action, got, want)
}
}
}
func TestJobCtrlUnionDiscriminant_ChkptUsesCkptOption(t *testing.T) {
if got, want := jobCtrlUnionDiscriminant(models.JOB_CHKPNT), 1; got != want {
t.Errorf("jobCtrlUnionDiscriminant(JOB_CHKPNT): got %d, want %d (JobControl_u_ckptoption)", got, want)
}
}
// gitea#45 follow-up: the job control endpoint additionally accepts a
// human-readable job ID (as printed by FormatJobID/`jobs list`) alongside
// the numeric one, so a caller doesn't have to reimplement xcecli's ID
// packing just to hit the REST API directly.
func TestParseHumanJobID_NonArrayRoundTripsWithFormatJobID(t *testing.T) {
const packed = uint64(1)<<clusterIDShift | uint64(0)<<apiserverIDShift | uint64(1788768547)<<timestampShift | 42
s := FormatJobID(int64(packed))
got, ok := ParseHumanJobID(s)
if !ok {
t.Fatalf("ParseHumanJobID(%q): ok=false, want true", s)
}
if got != packed {
t.Errorf("ParseHumanJobID(%q): got %d, want %d", s, got, packed)
}
}
// The array-child dash-quad prefix left over after the caller strips the
// "[idx]" suffix (parseArrayName in controllers/jobs.go) must parse back to
// exactly the job's base ID (ExtractArrayBaseID(original)) -- not the
// original tagged/array-idx-bearing packed value.
func TestParseHumanJobID_ArrayChildPrefixMatchesBaseID(t *testing.T) {
const arrayIdx = 3
packed := uint64(1)<<clusterIDShift | uint64(0)<<apiserverIDShift | uint64(1788768547)<<timestampShift |
uint64(1)<<tagShift | uint64(arrayIdx)
full := FormatJobID(int64(packed)) // "{cluster}-{apiserver}-{timestamp}-{base}[{idx}]"
prefix := full[:len(full)-len(fmt.Sprintf("[%d]", arrayIdx))]
got, ok := ParseHumanJobID(prefix)
if !ok {
t.Fatalf("ParseHumanJobID(%q): ok=false, want true", prefix)
}
if want := ExtractArrayBaseID(packed); got != want {
t.Errorf("ParseHumanJobID(%q): got %d, want %d (ExtractArrayBaseID)", prefix, got, want)
}
}
func TestParseHumanJobID_RejectsNumeric(t *testing.T) {
if _, ok := ParseHumanJobID("73933253805867008"); ok {
t.Error("ParseHumanJobID(plain numeric): ok=true, want false")
}
}
func TestParseHumanJobID_RejectsGarbage(t *testing.T) {
for _, s := range []string{"", "not-a-jobid", "1-2-3", "1-2-3-4-5"} {
if _, ok := ParseHumanJobID(s); ok {
t.Errorf("ParseHumanJobID(%q): ok=true, want false", s)
}
}
}
func TestRawJobToModel_BasicFields(t *testing.T) {
r := rawJobData{
jobID: 123,

17
src/frontend/apiserver/controllers/jobs.go
View file

@ -2352,11 +2352,20 @@ func (j JobController) ControlJobByID(ctx *gin.Context) {
}
logger.Debug("GetJobByIDs after parseArrayName", zap.Any("jobid", jobid))
// gitea#45 follow-up: accept the human-readable job ID form
// ("{cluster}-{apiserver}-{timestamp}-{seq}", e.g. what `jobs list`
// prints) in addition to the numeric one xcecli already translates
// to -- lets a caller hit this endpoint directly without
// reimplementing xcecli's ID packing. Numeric IDs are unaffected.
if !utils.IsNumeric(jobid.KeyID) {
err := errors.New("bad jobid: " + jobid.KeyID)
logger.Error(err.Error())
ReturnError(ctx, common.INVALID_PARAMS_CODE, common.INVALID_PARAMS_MSG+": "+err.Error(), nil)
return
if packed, ok := backend.ParseHumanJobID(jobid.KeyID); ok {
jobid.KeyID = strconv.FormatUint(packed, 10)
} else {
err := errors.New("bad jobid: " + jobid.KeyID)
logger.Error(err.Error())
ReturnError(ctx, common.INVALID_PARAMS_CODE, common.INVALID_PARAMS_MSG+": "+err.Error(), nil)
return
}
}
mainFiltCond := utils.FilterCondition{