feat(ha-convergence-recovery): commit-gated scheduler egress — submit_lsn tagging + watermark gate (task 1.2)

This commit is contained in:
Rongsong Shen 2026-06-14 02:33:36 +00:00
parent 7ff81156c2
commit b9ddfa60cb
9 changed files with 241 additions and 67 deletions

View file

@ -4,14 +4,14 @@
> Verify exact line numbers with grep before editing (this spec's research.md §1 has the 2026-06-11 verified anchors; files drift).
> Task 8 (E2E) additionally requires **ha-single-leader-role** to have landed and a live `/xce-deploy-ha` cluster.
- [ ] 1. Egress filter: submit_lsn tagging and commit-watermark gate on scheduler exports
- [x] 1. Egress filter: submit_lsn tagging and commit-watermark gate on scheduler exports
- [x] 1.1 Write RED tests for tagging and the scheduler-export watermark
- Create `testsuite/test_egress_filter.c` (XCE_TESTING, AAA structure) and register it in `controller_unit_test_src` (meson.build); add `uint64_t submit_lsn` to the `job_t` wrapper (types.h:483-485 anchor) and declare `job_submit_committed` / `collect_jobs_committed` in the dataex header so tests compile
- Tests: a live-submitted job is tagged with its WAL entry LSN; a job restored via `apply_wal_entry`/`restore_job` replay is tagged with the replayed entry's LSN; a job with `submit_lsn > committed_lsn` is absent from `collect_jobs_committed` results and from the JOB_NEW incremental push list; the same job appears once `committed_lsn` advances to cover it; `submit_lsn == 0` is always exportable; plain `collect_jobs` (status/API path) still returns the uncommitted job
- Observable completion: build succeeds with the new field/header declarations; all new test cases FAIL (red) against unimplemented predicate/tagging
- _Requirements: 3.1, 3.2, 3.3, 3.4_
- [ ] 1.2 Implement tagging and the watermark gate to green
- [x] 1.2 Implement tagging and the watermark gate to green
- Tagging: `persist_job_hook_fun` (persist.c:1107-1125 anchor) sets `submit_lsn = written_lsn` on each submitted job after a successful append; `apply_wal_entry` (persist.c:729-731 anchor) passes `hdr->lsn` through to `restore_job` (job.c:71), which tags the restored job
- Predicate + wrapper in dataex.c: `job_submit_committed` = `submit_lsn == 0 || submit_lsn <= committed_lsn` (one atomic load, lock-free); `collect_jobs_committed` wraps `collect_jobs` with the predicate
- Scheduler-bound consumers only: `xcesched_push_jobs` (sched.c:183 anchor) collects via the committed-only wrapper; `xcesched_push_joblist` (JOB_NEW push fed by `sched_job_new_fun`, sched.c:236-248 anchor) filters its list with the predicate; apiserver.c:83 and opget.c:173 remain on unfiltered `collect_jobs`
@ -91,3 +91,10 @@
- Observable completion: all E2E assertions hold on a live `/xce-deploy-ha` cluster; results recorded in completion notes
- _Depends: 7_
- _Requirements: 2.4, 9.1, 9.2, 8.1, 8.2, 3.1_
## Implementation Notes
- **Task 1.2 — predicate placed in new `egress.c`, not `dataex.c`** (design D3 "refinement, not contradiction"): `dataex.c` defines `notify()` (dataex.c:384) and `collect_confvars()`, both already stubbed in the unit-test binary (test_commit_barrier_reply.c, test_worker_join.c). Linking `dataex.c` into `controller_unit_test_src` is a duplicate-symbol error, so `job_submit_committed`/`collect_jobs_committed` live in a dedicated TU `src/controller/egress.c`, registered in BOTH `controller_src` and `controller_unit_test_src`. Header decls remain in `cluster.h` (the controller's "dataex header" — there is no `dataex.h`). Any future scheduler-bound export path adopts the predicate from there.
- **Test-link rule**: a production symbol that the unit-test suite already stubs cannot have its owning `.c` added to `controller_unit_test_src`. Factor newly-testable logic into its own TU instead.
- **`restore_job` gained an `entry_lsn` parameter** (replay tagging). Callers updated: cluster.h decl, job.c def, persist.c extern + apply_wal_entry caller, test_repl_apply.c stub. Any new caller must pass the WAL entry header LSN (`hdr->lsn`), or `0` if unknown (0 = exportable).
- **Build note**: the full meson controller suite SIGTERMs at the 300s wall-clock cap on the slow commit-barrier timeout suites (pre-existing). For per-task GREEN confirmation, run `controller_unit_test -t <suite>` directly with `ASAN_OPTIONS=detect_leaks=0` (harness teardown leaks are a pre-existing accepted pattern; the meson gate sets the same option).

109
src/controller/egress.c Normal file
View file

@ -0,0 +1,109 @@
/*
* egress.c — commit-gated scheduler egress (ha-convergence-recovery, Req 3).
*
* The scheduler is the one consumer whose actions cannot be revoked, so it must
* never observe a job whose submit entry is uncommitted (orphan-dispatch
* prevention). This file holds the commit-watermark predicate and the
* scheduler-bound collect wrapper that applies it.
*
* job_submit_committed(c, j)
* submit_lsn == 0 || submit_lsn <= committed_lsn
* One atomic load + one compare; lock-free and infallible.
* submit_lsn == 0 (pre-feature / direct create) is always exportable so
* legacy and replayed jobs are never stranded (Req 3.1, 3.2).
*
* collect_jobs_committed(c, fn, l)
* Identical contract to collect_jobs (dataex.c) — collects matching jobs,
* references and READ-locks each, leaving them for unlock_collect_jobs —
* additionally excluding any job whose submit entry is uncommitted. For
* scheduler-bound callers ONLY (xcesched_push_jobs). Status/API read
* paths keep plain collect_jobs (Req 3.4 — read consistency out of scope).
*
* Lives in its own translation unit (not dataex.c) so the controller unit test
* binary can link the REAL predicate without pulling in dataex.c's notify() /
* collect_confvars() definitions, which the test suite already stubs. This is
* the same egress-filter component the design assigns to dataex.c, factored out
* for testability (D3 — refinement, not contradiction).
*/
/* rtti.h first — defines 'var'/'typeinfo_t' used by kvmgr.h (via cluster.h). */
#include <rtti.h>
#include <errno.h>
#include <stdatomic.h>
#include <stdbool.h>
#include <stdint.h>
#include "cluster.h"
#include "persist.h"
#include "utils.h" /* CONTAINER_OF */
#include "xce/cluster_def.h"
#include "xce/types.h"
#include "xlist.h"
/* Commit-watermark predicate (Req 3.1, 3.2). Lock-free: one atomic load of
* committed_lsn and one comparison. submit_lsn == 0 means unknown/pre-feature
* and is always exportable. */
bool job_submit_committed(cluster_t* c, job_t* j)
{
if (j == NULL)
return false;
if (j->submit_lsn == 0)
return true;
uint64_t committed
= atomic_load_explicit(&c->persist.committed_lsn, memory_order_acquire);
return j->submit_lsn <= committed;
}
/* Read-lock each collected job, mirroring collect_jobs' lock_collect_jobs
* (dataex.c) so the contract — caller must release via unlock_collect_jobs — is
* identical for scheduler-bound callers. */
static void egress_lock_collected(xlist_t* l, int op)
{
for (int i = 0; i < l->count; i++) {
xce_job_t* xj = (xce_job_t*)xlist_get(l, i);
if (xj == NULL)
continue;
job_t* j = CONTAINER_OF(xj, job_t, job);
lock(j, op);
}
}
/* Scheduler-bound collect: same iteration + reference + read-lock contract as
* collect_jobs (dataex.c:294), additionally gated on the commit watermark.
* `fn`, when non-NULL, is an additional caller predicate applied first (matches
* collect_jobs' check_job_ usage in xcesched_push_jobs). */
void collect_jobs_committed(cluster_t* c, collect_check_fn_t fn, xlist_t* l)
{
iterator_t it;
lock(c->jobmgr, RTTI_LOCK_READ);
if (iter_make(&it, c->jobmgr) < 0) {
unlock(c->jobmgr);
return;
}
for (;;) {
job_t* j = NULL;
int rc = iter_next(&it, (void**)&j);
if ((rc < 0) || (rc == RTTI_ITER_NO_DATA))
break;
if (j == NULL)
continue;
/* Caller predicate first (parity with collect_jobs), then the commit
* watermark gate — exclude jobs whose submit entry is uncommitted. */
if ((fn != NULL) && (!fn(j)))
continue;
if (!job_submit_committed(c, j))
continue;
reference(j);
xlist_store(l, &(j->job));
}
iter_destroy(&it);
unlock(c->jobmgr);
egress_lock_collected(l, RTTI_LOCK_READ);
}

View file

@ -44,7 +44,7 @@ extern int add_resource(void*, char*, char*, int, xce_order_t, int, int);
extern int update_resvals(cluster_t*, worker_t*, int, xce_resval_t*);
extern int load_jobs(void*);
extern int restore_job(cluster_t*, uint64_t, const void*, size_t);
extern int restore_job(cluster_t*, uint64_t, const void*, size_t, uint64_t);
extern int update_job(void*, uint64_t, xce_kv_list_t*);
extern int do_job_started(void*, uint64_t, int, int*, int, int*);

View file

@ -89,9 +89,12 @@ int load_jobs(void* c)
/* Reconstruct a job from a WAL_JOB_SUBMIT Cap'n Proto payload.
* Uses the jobid embedded in the serialized xce_job_t (not the caller's arg).
* entry_lsn is the WAL entry LSN of this submit record (ha-convergence-recovery
* 3.3): the restored job is tagged with it so its commit status is decidable at
* scheduler-export time. 0 = unknown (pre-feature / direct create) = exportable.
* Returns -EEXIST if the job is already present (idempotent replay guard). */
int restore_job(cluster_t* c, uint64_t jobid_unused,
const void* capnp_payload, size_t len)
const void* capnp_payload, size_t len, uint64_t entry_lsn)
{
(void)jobid_unused;
@ -121,6 +124,12 @@ int restore_job(cluster_t* c, uint64_t jobid_unused,
job_t* job = create_job(c, &j);
if (!job)
return -errno;
/* ha-convergence-recovery (3.3): tag the restored job with its submit
* entry's LSN so the egress watermark predicate can decide its commit
* status (job_submit_committed). On a promoted primary, replayed entries
* sit at or below the promotion truncation point, so restored jobs are
* exportable by construction. */
job->submit_lsn = entry_lsn;
return 0;
}

View file

@ -3,6 +3,7 @@ controller_src = [
'commit.c',
'config_repl.c',
'dataex.c',
'egress.c',
'ha.c',
'job.c',
'join.c',
@ -125,6 +126,7 @@ controller_unit_test_src = [
'testsuite/test_egress_filter.c',
'commit.c',
'job.c',
'egress.c',
'queue.c',
'join.c',
'opupd.c',

View file

@ -30,8 +30,8 @@
#include <zlib.h>
/* restore_* functions defined in job.c / worker.c / queue.c */
extern int restore_job(cluster_t* c, uint64_t jobid,
const void* capnp_payload, size_t len);
extern int restore_job(cluster_t* c, uint64_t jobid, const void* capnp_payload,
size_t len, uint64_t entry_lsn);
extern int restore_job_state(cluster_t* c, uint64_t jobid, uint32_t state);
extern int worker_from_capnp(cluster_t* c,
const void* capnp_payload, size_t len);
@ -729,8 +729,10 @@ int apply_wal_entry(persist_t* p, const wal_entry_hdr_t* hdr, const void* payloa
switch (hdr->type) {
case WAL_JOB_SUBMIT:
/* Payload: Cap'n Proto serialized xce_job_t */
restore_job(p->cluster, 0, payload, hdr->payload_len);
/* Payload: Cap'n Proto serialized xce_job_t. Thread the entry LSN
* through so the restored job is tagged for the egress watermark
* (ha-convergence-recovery 3.3). */
restore_job(p->cluster, 0, payload, hdr->payload_len, hdr->lsn);
break;
case WAL_JOB_START:
@ -1326,10 +1328,30 @@ int persist_job_hook_fun(int type, int situation, int detail,
* even when replication is degraded (no standbys → -ENODEV, timeout
* → -ETIMEDOUT). Only abort the loop on a real disk-write failure
* (non-zero return). Replication status is informational here. */
/* ha-convergence-recovery (3.3): snapshot the LSN this submit
* entry will receive BEFORE the append. persist_wal_async assigns
* hdr.lsn = current_lsn then increments (persist.c:318), so the
* pre-append current_lsn is exactly the written entry's LSN — the
* same value persist_wal_sync_append derives as current_lsn - 1. */
pthread_mutex_lock(&p->lock);
uint64_t submit_lsn = p->current_lsn;
pthread_mutex_unlock(&p->lock);
int sync_rc = 0;
rc = persist_wal_sync_append(p, WAL_JOB_SUBMIT, buf, (uint32_t)sz,
&sync_rc);
free(buf);
/* Tag the job with its submit entry's LSN whenever the LOCAL WAL
* write succeeded. persist_wal_sync_append returns the
* persist_wal_async error only on local-write failure; on a
* replication-degraded outcome (0 / -ETIMEDOUT / -ENODEV /
* -XCE_ERR_NOT_READY) the entry was appended locally and the LSN
* advanced, so the tag must be set (the egress predicate is
* independent of replication status). */
if (rc == 0 || rc == -ETIMEDOUT || rc == -XCE_ERR_NOT_READY) {
j->submit_lsn = submit_lsn;
}
}
return rc;
}

View file

@ -14,19 +14,49 @@ extern void collect_shared_resvals(cluster_t*, collect_check_fn_t, xlist_t*);
static int xcesched_push_joblist(cluster_t* c, xlist_t* l)
{
xce_data_t data;
xce_jobs_data_t jobs = { .n_total = l->count,
int rc = 0;
/* ha-convergence-recovery (3.1, 3.2): the incremental JOB_NEW push is a
* scheduler-bound export, so it gates on the commit watermark — exclude any
* job whose submit entry is uncommitted (job_submit_committed). A
* just-submitted job whose entry is not yet committed is correctly withheld
* here and reaches the scheduler at the next full push once committed.
* Build a filtered pointer array over the caller's list (the list stores
* xce_job_t*; recover the job_t wrapper via CONTAINER_OF for the predicate). */
xce_job_t** filtered = NULL;
int nfiltered = 0;
if (l->count > 0) {
filtered = calloc((size_t)l->count, sizeof(*filtered));
if (filtered == NULL)
return -ENOMEM;
for (int i = 0; i < l->count; i++) {
xce_job_t* xj = (xce_job_t*)xlist_get(l, i);
if (xj == NULL)
continue;
job_t* j = CONTAINER_OF(xj, job_t, job);
if (job_submit_committed(c, j))
filtered[nfiltered++] = xj;
}
}
xce_jobs_data_t jobs = { .n_total = nfiltered,
.pos = 0,
.count = l->count,
.jobs = (xce_job_t**)l->data };
int rc = 0;
.count = nfiltered,
.jobs = filtered };
memset(&data, 0x0, sizeof(data));
data.table = Data_data_schedjobs;
data.data.jobs = &jobs;
rc = xce_push_data_req(c->sched.ep, c->myhost, &data, check_controller, c,
100);
/* No committed jobs to push → nothing to send (avoids an empty payload). */
if (nfiltered > 0) {
rc = xce_push_data_req(c->sched.ep, c->myhost, &data, check_controller,
c, 100);
}
free(filtered);
return rc;
}
@ -180,7 +210,12 @@ static int xcesched_push_jobs(cluster_t* c)
data.table = Data_data_schedjobs;
data.data.jobs = &jobs;
collect_jobs(c, (collect_check_fn_t)check_job_, &jlist);
/* ha-convergence-recovery (3.1, 3.2): scheduler-bound full push gates on the
* commit watermark — a job whose submit entry is uncommitted is excluded so
* the scheduler never acts on state the post-failover cluster may not have.
* Same lock contract as collect_jobs (caller releases via
* unlock_collect_jobs). */
collect_jobs_committed(c, (collect_check_fn_t)check_job_, &jlist);
jobs.n_total = jlist.count;
jobs.jobs = (xce_job_t**)jlist.data;
jobs.count = jobs.n_total;

View file

@ -1,6 +1,6 @@
/*
* test_egress_filter.c — RED-phase tests for commit-gated scheduler egress
* (ha-convergence-recovery task 1.1).
* test_egress_filter.c — tests for commit-gated scheduler egress
* (ha-convergence-recovery tasks 1.1 RED / 1.2 GREEN).
*
* Requirements (this spec's IDs):
* 3.1 — a job whose submit entry is not yet committed is EXCLUDED from
@ -15,15 +15,14 @@
* status/API read path (plain collect_jobs is NOT commit-filtered).
* submit_lsn == 0 is always exportable (pre-feature / unknown).
*
* RED CONTRACT: these tests are written against the task-1.2 GREEN behavior.
* They FAIL today because:
* - persist_job_hook_fun does not yet set j->submit_lsn (live tagging gap)
* - apply_wal_entry passes 0 (not hdr->lsn) to restore_job, and restore_job
* does not tag submit_lsn (replay tagging gap)
* - job_submit_committed / collect_jobs_committed are not yet implemented in
* dataex.c (the RED scaffold below is intentionally unfiltered — task 1.2
* DELETES this scaffold and implements the real predicate + wrapper in
* dataex.c per design.md "Egress Filter").
* GREEN (task 1.2): the production tagging + watermark gate are now wired and
* these tests exercise the REAL symbols:
* - persist_job_hook_fun sets j->submit_lsn = written_lsn on live submit
* - apply_wal_entry threads hdr->lsn to restore_job, which tags submit_lsn
* - job_submit_committed / collect_jobs_committed live in egress.c (linked
* into this binary) per design.md "Egress Filter" (D3 — factored out of
* dataex.c so the unit test can link the real predicate without dataex.c's
* notify()/collect_confvars() clashing with existing test stubs).
*
* Harness: a real persist_t over a private tmpdir embedded in a test
* cluster_t with a registered jobmgr and persistence hooks, a wired commit
@ -60,15 +59,19 @@
#include "wal_types.h"
#include "xlist.h"
/* ── Local job collector (avoids linking dataex.c into this unit binary) ──
/* ── Status/API read-path collector (unfiltered — Req 3.4) ───────────────
*
* dataex.c is NOT in controller_unit_test_src (its notify()/collect_confvars()
* clash with stubs already in test_commit_barrier_reply.c / test_worker_join.c).
* collect_jobs / unlock_collect_jobs therefore are not linkable here, so this
* file uses ef_collect_jobs — a faithful copy of collect_jobs' jobmgr iteration
* (dataex.c:294) — for the unfiltered status/API path AND as the iteration core
* of the RED scaffold below. The behavior asserted is identical to collect_jobs;
* only the symbol is local. */
* The real scheduler-bound predicate + wrapper (job_submit_committed,
* collect_jobs_committed) now live in egress.c, which IS linked into this test
* binary (controller_unit_test_src) — these tests exercise the REAL production
* symbols. dataex.c (which owns plain collect_jobs / unlock_collect_jobs) is
* still NOT linked here: its notify()/collect_confvars() definitions clash with
* stubs already provided by test_commit_barrier_reply.c / test_worker_join.c.
* For the unfiltered status-path assertion (3.4) this file therefore keeps a
* faithful local copy of collect_jobs' iteration — ef_collect_jobs — and a
* matching ef_unlock_jobs (reference/dereference only, no per-job lock, since
* ef_collect_jobs does not lock). The behavior asserted is identical to
* collect_jobs; only the symbol is local. */
static void ef_collect_jobs(cluster_t *c, xlist_t *l)
{
iterator_t it;
@ -103,35 +106,22 @@ static void ef_unlock_jobs(xlist_t *l)
}
}
/* ── RED scaffold: minimal, intentionally UNFILTERED stand-ins ───────────
*
* These satisfy the linker (job_submit_committed / collect_jobs_committed are
* declared in cluster.h but not yet defined). They DO NOT implement the commit
* watermark — job_submit_committed always returns true and collect_jobs_committed
* collects every job. That makes the 3.1/3.2 exclusion assertions FAIL (the
* uncommitted job leaks through), which is the intended RED state.
*
* Task 1.2 (GREEN) MUST delete this entire scaffold block and provide the real
* definitions in dataex.c per design.md "Egress Filter":
* job_submit_committed -> submit_lsn == 0 || submit_lsn <= committed_lsn
* collect_jobs_committed -> collect_jobs filtered by that predicate
* At that point dataex.c becomes part of the test link (with the notify/
* collect_confvars stub clashes reconciled) and this scaffold is removed to
* avoid duplicate symbols. */
bool job_submit_committed(cluster_t *c, job_t *j)
/* Release a list produced by the REAL collect_jobs_committed (egress.c): it
* read-locks each collected job (identical contract to collect_jobs), so the
* matching release must unlock AND dereference — mirror of the production
* unlock_collect_jobs (dataex.c), which is not linkable in this binary. */
static void ef_unlock_committed(xlist_t *l)
{
(void)c;
(void)j;
return true; /* RED: unfiltered — task 1.2 replaces with the watermark */
for (int i = 0; i < l->count; i++) {
xce_job_t *xj = (xce_job_t *)xlist_get(l, i);
if (xj == NULL)
continue;
job_t *j = CONTAINER_OF(xj, job_t, job);
unlock(j);
dereference(j);
}
}
void collect_jobs_committed(cluster_t *c, collect_check_fn_t fn, xlist_t *l)
{
(void)fn; /* scheduler-bound callers pass NULL */
ef_collect_jobs(c, l); /* RED: no commit filtering */
}
/* ── end RED scaffold ──────────────────────────────────────────────────── */
/* notify() is provided by test_commit_barrier_reply.c (same test binary) — do
* not redefine it here (would be a duplicate symbol). */
@ -423,7 +413,7 @@ static void test_uncommitted_job_absent_from_scheduler_export(void)
"uncommitted job (submit_lsn=9 > committed_lsn=5) leaked into "
"scheduler export");
ef_unlock_jobs(&out);
ef_unlock_committed(&out);
xlist_fini(&out);
ef_teardown();
printf("test_uncommitted_job_absent_from_scheduler_export: checked\n");
@ -449,7 +439,7 @@ static void test_job_present_after_commit_advances(void)
TEST_ASSERT_EQUAL_INT(0, xlist_init(&before, 16));
collect_jobs_committed(&g_c, NULL, &before);
TEST_ASSERT_FALSE(ef_list_has_job(&before, 404));
ef_unlock_jobs(&before);
ef_unlock_committed(&before);
xlist_fini(&before);
/* Advance committed_lsn to cover the submit entry. */
@ -465,7 +455,7 @@ static void test_job_present_after_commit_advances(void)
ef_list_has_job(&after, 404),
"job absent from scheduler export after committed_lsn advanced to "
"cover its submit entry");
ef_unlock_jobs(&after);
ef_unlock_committed(&after);
xlist_fini(&after);
ef_teardown();
@ -489,7 +479,7 @@ static void test_submit_lsn_zero_always_exportable(void)
TEST_ASSERT_EQUAL_INT(0, xlist_init(&out, 16));
collect_jobs_committed(&g_c, NULL, &out);
TEST_ASSERT_TRUE(ef_list_has_job(&out, 505));
ef_unlock_jobs(&out);
ef_unlock_committed(&out);
xlist_fini(&out);
ef_teardown();
@ -526,7 +516,7 @@ static void test_uncommitted_job_visible_to_status_path(void)
void test_egress_filter(void)
{
printf("\n>> commit-gated scheduler egress tests (RED, task 1.1)\n\n");
printf("\n>> commit-gated scheduler egress tests (GREEN, task 1.2)\n\n");
RUN_TEST(test_live_submit_tags_submit_lsn);
RUN_TEST(test_replay_restore_tags_entry_lsn);

View file

@ -28,9 +28,9 @@ typedef struct cluster_s cluster_t;
__attribute__((weak))
int restore_job(cluster_t* c, uint64_t jobid,
const void* capnp_payload, size_t len)
const void* capnp_payload, size_t len, uint64_t entry_lsn)
{
(void)c; (void)jobid; (void)capnp_payload; (void)len;
(void)c; (void)jobid; (void)capnp_payload; (void)len; (void)entry_lsn;
return 0;
}