feat(job-array-one-based-index): collision checks after registration (task 2.3)

handle_jobsubmit now registers the submission first and checks the job
table second; a hit is answered -EEXIST and released through the atomic
reply-and-release step. For array submissions, array_identity_exists()
does one read-locked scan of the 32768 tagged element keys of the
array's block (element 0 included for pre-upgrade arrays), covering
both the element range and the array identity; unmarked submissions
keep jobid_range_exists. An existing X|3 now blocks a new X|5.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-01 10:14:47 +00:00
parent 81bfaf1f50
commit fe5ddc9854
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
5 changed files with 278 additions and 27 deletions

View file

@ -101,8 +101,8 @@ static job_t* create_job(cluster_t* c, xce_job_t* j)
* process-local registry, and treat a hit there identically to a
* c->jobmgr hit — both mean "this exact write already happened or is
* already in flight," which is precisely what a retried write must see as
* a duplicate. Populated in handle_jobsubmit() right after this check
* passes (before any WAL work).
* a duplicate. Populated in handle_jobsubmit() before the job-table
* collision check (and before any WAL work).
*
* job-array-one-based-index (Req 7.7, 7.10, 7.11): an entry also carries
* whether it is an array and its array identity (tag cleared, low 15 bits
@ -283,9 +283,13 @@ int jobsubmit_inflight_try_add_for_test(uint64_t base, uint64_t size)
* happened".
*
* bug-1419: this check alone is no longer sufficient (see
* inflight_range_try_add's doc comment) — handle_jobsubmit additionally
* calls inflight_range_try_add() after this check passes, which atomically
* closes the remaining not-yet-applied window this function cannot see. */
* inflight_range_try_add's doc comment) — handle_jobsubmit first calls
* inflight_range_try_add(), which atomically closes the not-yet-applied
* window this function cannot see, and runs this check only after that
* registration succeeds (job-array-one-based-index, Req 7.10, 7.11);
* a hit is answered and released through inflight_reply_and_remove().
* Used for unmarked (tag-0) submissions only; an array submission is
* checked by array_identity_exists() instead. */
static bool jobid_range_exists(cluster_t* c, uint64_t base_jobid,
uint64_t array_size)
{
@ -304,6 +308,34 @@ static bool jobid_range_exists(cluster_t* c, uint64_t base_jobid,
return false;
}
/* job-array-one-based-index (Req 7.10): true if any element key of
* array_id's block -- MAKE_JOBID(.., TAG_JOB_ARRAY, A|k) for k = 0..
* MAX_ARRAY_IDX, element 0 included for pre-upgrade arrays -- is in
* c->jobmgr. One scan under a single read-lock hold (key_find takes no lock
* of its own) answers both the element-range check for a classified array
* submission (its range lies inside the block) and the array-identity
* check, so it replaces jobid_range_exists() for tag-1 submissions. Bounded
* CPU work (32768 hash lookups), no I/O, no blocking (ha-async-io.md). */
int array_identity_exists(cluster_t* c, uint64_t array_id)
{
const uint64_t first = MAKE_JOBID(
EXTRACT_ID_CLUSTER(array_id), EXTRACT_ID_APISERVER(array_id),
EXTRACT_ID_TIMESTAMP(array_id), TAG_JOB_ARRAY,
EXTRACT_ARRAY_JOB(array_id));
int found = 0;
lock(c->jobmgr, RTTI_LOCK_READ);
for (uint64_t k = 0; k <= MAX_ARRAY_IDX && !found; k++) {
job_t* existing = NULL;
if (key_find(c->jobmgr, (void*)(first | k), (void**)&existing) > 0
&& existing != NULL)
found = 1;
}
unlock(c->jobmgr);
return found;
}
/* Build one xce_job_t submission payload per array element, WITHOUT
* constructing or registering any job object (raft-apply-decoupling task
* 3.1 / Req 2.1, 2.3) — these are plain heap structs carrying only the data
@ -2452,7 +2484,7 @@ static void jobsubmit_commit_continuation(void* ctx, uint64_t lsn, int rc)
* the array-shaped sibling of handle_jobsubmit()'s single-job append +
* continuation-registration block below, taken for submit->array_size > 1.
* Called strictly AFTER handle_jobsubmit() has already run the SAME
* pre-commit range validation (jobid_range_exists + inflight_range_try_add)
* pre-commit validation (inflight_range_try_add, then array_identity_exists)
* the single-job path uses — this function never repeats or reorders that
* validation (Req 9.1, 9.2); it only takes over at the point the two paths
* diverge.
@ -2599,7 +2631,19 @@ int handle_jobsubmit(cluster_t* c, endpoint_t* ep, xce_request_t* request)
jobid = submit->jobid;
if (jobid_range_exists(c, jobid, submit->array_size)) {
/* bug-1419: atomically claim this jobid range (and, for an array, its
* array identity) as in-flight, closing the window the c->jobmgr check
* alone cannot see (see jobid_range_exists' doc comment). A false
* return means another submission (a genuine concurrent collision, or
* -- the actual motivating case -- this exact request's own retried
* resend racing its still-in-flight original attempt) already holds an
* overlapping range or the same array identity; reject it
* duplicate-submission. From here on every answer goes through
* inflight_reply_and_remove(), which sends the reply and releases this
* registration as one step (job-array-one-based-index, Req 7.7, 7.10,
* 7.11) -- the early-outs below directly, the commit outcome via
* jobsubmit_reply_*_once. */
if (!inflight_range_try_add(jobid, submit->array_size)) {
errno = EEXIST;
reply.status = -errno;
reply.data_which = RequestReply_data_nodata;
@ -2608,24 +2652,22 @@ int handle_jobsubmit(cluster_t* c, endpoint_t* ep, xce_request_t* request)
return -errno;
}
/* bug-1419: atomically claim this jobid range as in-flight, closing the
* window jobid_range_exists' c->jobmgr check alone cannot see (see its
* doc comment). A false return means another submission (a genuine
* concurrent collision, or -- the actual motivating case -- this exact
* request's own retried resend racing its still-in-flight original
* attempt) already holds this range; reject exactly as the
* jobid_range_exists branch above does. From here on every answer
* goes through inflight_reply_and_remove(), which sends the reply and
* releases this registration as one step (job-array-one-based-index,
* Req 7.7, 7.10, 7.11) -- the early-outs below directly, the commit
* outcome via jobsubmit_reply_*_once. */
if (!inflight_range_try_add(jobid, submit->array_size)) {
errno = EEXIST;
reply.status = -errno;
/* job-array-one-based-index (Req 7.10, 7.11): the job-table checks run
* AFTER registration, so a same-identity submission applied and
* answered between the two steps cannot be missed by both (an applied
* submission is in c->jobmgr before it leaves the registry). An array
* is checked by one scan of its whole block (element range and array
* identity); an ordinary job keeps the element-range check. */
if (is_array_tag(jobid)
? array_identity_exists(c, array_jobid(jobid))
: jobid_range_exists(c, jobid, submit->array_size)) {
reply.status = -EEXIST;
reply.data_which = RequestReply_data_nodata;
xce_reply(ep, c->myhost, &reply);
return -errno;
inflight_reply_and_remove(ep, c->myhost, &reply, jobid,
submit->array_size);
errno = EEXIST;
return -EEXIST;
}
/* job-array-single-wal-record task 2.2 (Req 1.1-1.3, 8.1, 9.1, 9.2):