From b88e352616ee1c9a2d1de03addc8c13a00342520 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 1 Oct 2026 11:29:11 +0000 Subject: [PATCH] feat(job-array-one-based-index): apiserver backend ID helpers and array ID wrapper (task 5.1) GenerateArrayJobID wraps libxce idgen_array (first element, index 1; one-element arrays included) with a non-cgo stub; pure helpers ParseHumanJobIDStrict (written identity with exact field-width checks, uint64 overflow reported as a width error), IsArrayCapableKey and MakeElementID (bit-identical to libxce jobsel_element_id). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../specs/job-array-one-based-index/tasks.md | 3 +- src/frontend/apiserver/backend/clib.go | 19 +++ src/frontend/apiserver/backend/clib_test.go | 30 ++++ .../apiserver/backend/convert_pure.go | 63 ++++++++- .../apiserver/backend/convert_test.go | 132 ++++++++++++++++++ src/frontend/apiserver/backend/stub.go | 5 + .../apiserver/backend/stub_funcs_test.go | 11 +- 7 files changed, 260 insertions(+), 3 deletions(-) diff --git a/.kiro/specs/job-array-one-based-index/tasks.md b/.kiro/specs/job-array-one-based-index/tasks.md index 37eade56..fdc15338 100644 --- a/.kiro/specs/job-array-one-based-index/tasks.md +++ b/.kiro/specs/job-array-one-based-index/tasks.md @@ -94,7 +94,7 @@ - _Depends: 1.1, 1.2_ - [ ] 5. API server foundation -- [ ] 5.1 Add backend ID helpers and the array ID wrapper +- [x] 5.1 Add backend ID helpers and the array ID wrapper - Wrap the libxce array allocator for the API server, with a non-cgo stub that returns an error - Add strict written-identity parsing with field-width checks, an array-capable key test and element-ID composition as pure helpers - Done when the backend unit tests pass for the strict parser (width rejections), the array-capable test and element composition, and both the cgo and non-cgo builds compile @@ -243,3 +243,4 @@ - 2.2: two extra XCE_TESTING seams exist (`jobsubmit_test_release_hook`, `jobsubmit_inflight_try_add_for_test`); the controller test fixture has no WAL, so async commit-failure runs synchronously inside handle_jobsubmit — reuse it in 2.3. Stale comment at `src/controller/testsuite/test_commit_barrier_reply.c:1458` (claims release via jobsubmit_async_ctx_release) — fix in 2.3. - 4.1: after `jobsub -a N`, `$jobid` is the array identity, so `expect-job $jobid` currently reports not found; 4.3 must make it behave per 4.7/4.9 (not-found hint naming `[k]`, or exact-ID match). Nit for 4.3: the `refused` early return in `cmd_jobsub` sits between the bug-786 ack comment and the code it describes — move it above the comment. - 4.3: the controller acks `setpend` (`do_set_preason`, job.c ~3869-3880) and `jobrun` (`handle_jobdispatch`, job.c ~2246-2258) with status 0 before doing the work, so those two harness commands can never report not-found; 4.7/5.13 for them depend on a controller change outside this spec's boundary. `jobinfo`'s not-found is `rc == 0` with no row (`do_reply_joblist`), not -ENOENT. +- 5.1: Go helpers: `ParseHumanJobIDStrict(s) (uint64, error)` returns sentinel `ErrJobIDSyntax` / `ErrJobIDWidth` (callers trim and split `[...]` first; no whitespace accepted); `MakeElementID(key uint64, k int) uint64`; `IsArrayCapableKey(key)`; `GenerateArrayJobID(c, a, n)` (non-cgo stub returns errNoCGO). diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 95c76da4..cbf902d2 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -346,6 +346,25 @@ func GenerateJobID(clusterID, apiserverID int, arraySize int) (uint64, error) { return uint64(jobID), nil } +// GenerateArrayJobID calls the C idgen_array() to allocate a new array of n +// elements (1 <= n <= 32767, numbered from 1) and returns the first element's +// ID (tag TAG_JOB_ARRAY, index 1); ExtractArrayBaseID of it is the array ID. +// n == 1 is a one-element array, not an ordinary job (job-array-one-based-index +// 1.1, 1.2). Errors on a bad n or when the sequence block would overflow. +func GenerateArrayJobID(clusterID, apiserverID int, n int) (uint64, error) { + if n < 1 { + return 0, fmt.Errorf("idgen_array: invalid array size %d", n) + } + idCtxOnce.Do(initIDGen) + var first C.uint64_t + rc := C.idgen_array(&gIDCtx, C.size_t(clusterID), C.size_t(apiserverID), + C.size_t(n), &first) + if rc < 0 { + return 0, fmt.Errorf("idgen_array failed: %d", int(rc)) + } + return uint64(first), nil +} + // Cleanup releases C resources. Call at process shutdown. func Cleanup() { C.idgen_ctx_fini(&gIDCtx) diff --git a/src/frontend/apiserver/backend/clib_test.go b/src/frontend/apiserver/backend/clib_test.go index 1a5f09e9..963e8819 100644 --- a/src/frontend/apiserver/backend/clib_test.go +++ b/src/frontend/apiserver/backend/clib_test.go @@ -303,3 +303,33 @@ func TestValidateResreqSyntax(t *testing.T) { } } } + +// job-array-one-based-index 1.1/1.2: GenerateArrayJobID wraps libxce +// idgen_array and returns the first element (index 1) of an array whose +// identity is array-capable, for n = 1 (one-element array) and n > 1. +func TestGenerateArrayJobID_FirstElementIsIndexOne(t *testing.T) { + for _, n := range []int{1, 3, 32767} { + first, err := GenerateArrayJobID(1, 2, n) + if err != nil { + t.Fatalf("GenerateArrayJobID(1, 2, %d): err=%v", n, err) + } + if !IsArrayJob(first) || ExtractArrayIdx(first) != 1 { + t.Errorf("n=%d: first=%#x, want tagged element with index 1", n, first) + } + key := ExtractArrayBaseID(first) + if !IsArrayCapableKey(key) || MakeElementID(key, 1) != first { + t.Errorf("n=%d: array identity %#x not array-capable or not the first element's", n, key) + } + if c, a := extractBits(first, clusterIDShift, nClusterIDBits), extractBits(first, apiserverIDShift, nApiserverIDBits); c != 1 || a != 2 { + t.Errorf("n=%d: cluster/apiserver = %d/%d, want 1/2", n, c, a) + } + } +} + +func TestGenerateArrayJobID_RejectsBadSize(t *testing.T) { + for _, n := range []int{0, -1, 32768} { + if id, err := GenerateArrayJobID(1, 2, n); err == nil { + t.Errorf("GenerateArrayJobID(1, 2, %d): got (%#x, nil), want an error", n, id) + } + } +} diff --git a/src/frontend/apiserver/backend/convert_pure.go b/src/frontend/apiserver/backend/convert_pure.go index 2925fba2..163402d4 100644 --- a/src/frontend/apiserver/backend/convert_pure.go +++ b/src/frontend/apiserver/backend/convert_pure.go @@ -3,10 +3,11 @@ package backend import ( "apiserver/models" "encoding/json" + "errors" + "fmt" "regexp" "strconv" "time" - "fmt" ) // JobID bit layout constants — mirrors src/include/config.h. @@ -132,6 +133,66 @@ func ParseHumanJobID(s string) (uint64, bool) { sequence, true } +// Errors returned by ParseHumanJobIDStrict. Callers map them to their own +// messages: ErrJobIDSyntax is the written-form syntax rule, ErrJobIDWidth the +// field-width rule (job-array-one-based-index 5.14). +var ( + ErrJobIDSyntax = errors.New("job ID is not of the form cluster-apiserver-timestamp-sequence") + ErrJobIDWidth = errors.New("job ID field out of range") +) + +// ParseHumanJobIDStrict parses a written job identity "c-a-ts-seq" (decimal +// digits only, leading zeros allowed, no sign, no whitespace, no "[...]") +// into its packed, untagged ID. Unlike ParseHumanJobID it checks every field +// against its bit width -- cluster <= 63, apiserver <= 15, timestamp < 2^32, +// sequence < 2^20 -- so an oversized field is rejected (ErrJobIDWidth) +// instead of spilling into a neighbouring field. A field too large even for +// uint64 is also ErrJobIDWidth. Anything not of that shape is ErrJobIDSyntax. +func ParseHumanJobIDStrict(s string) (uint64, error) { + m := humanJobIDPattern.FindStringSubmatch(s) + if m == nil { + return 0, ErrJobIDSyntax + } + widths := [4]int{nClusterIDBits, nApiserverIDBits, nTimestampBits, nSequenceBits} + var f [4]uint64 + for i, w := range widths { + v, err := strconv.ParseUint(m[i+1], 10, 64) + if err != nil { + if errors.Is(err, strconv.ErrRange) { + return 0, ErrJobIDWidth + } + return 0, ErrJobIDSyntax + } + if v >= uint64(1)<" +// in "[k]"): tag 0 and the low 15 (index) bits clear, i.e. the +// sequence is a multiple of 32768 (job-array-one-based-index 5.8). +func IsArrayCapableKey(key uint64) bool { + return extractIDTag(key) == 0 && key&arrayIdxMask == 0 +} + +// MakeElementID composes the ID of element k of the array identified by key: +// key's cluster, apiserver, timestamp and sequence-high bits, tag +// TAG_JOB_ARRAY and index k (masked to 15 bits). key's own tag and index bits +// are ignored. Mirrors jobsel_element_id() in src/lib/utils.c. +func MakeElementID(key uint64, k int) uint64 { + base := key &^ (uint64(1< uint64: err=%v, want ErrJobIDWidth", err) + } +} + +func TestParseHumanJobIDStrict_RejectsBadSyntax(t *testing.T) { + for _, s := range []string{"", "73933253805867008", "1-2-3", "1-2-3-4-5", + "1-2-3-4[1]", " 1-2-3-4", "1-2-3-4 ", "+1-2-3-4", "1--2-3", "a-b-c-d"} { + if _, err := ParseHumanJobIDStrict(s); !errors.Is(err, ErrJobIDSyntax) { + t.Errorf("ParseHumanJobIDStrict(%q): err=%v, want ErrJobIDSyntax", s, err) + } + } +} + +// 5.8: an array identity has tag 0 and its low 15 bits clear. +func TestIsArrayCapableKey_AlignedUntagged(t *testing.T) { + if !IsArrayCapableKey(makeJobID(1, 0, 100, 0, 5<