feat: add HOOK_DETAIL_JOB_STOP/RESUME; fix xce_reply ordering after AFTER_EVENT hooks (P0-4a)

- cluster_def.h: add struct tag `cluster_s` to cluster_t to resolve
  conflicting typedef with persist.h forward declaration
- cluster_def.h: add HOOK_DETAIL_JOB_STOP and HOOK_DETAIL_JOB_RESUME enum
  values after HOOK_DETAIL_JOB_RUSAGE_UPDATE
- job.c on_job_stop(): replace mark_job_update() call with direct
  xce_run_hooks(BEFORE/AFTER, HOOK_DETAIL_JOB_STOP) so WAL writer
  records stop events with the correct detail value
- job.c on_job_resume(): same pattern with HOOK_DETAIL_JOB_RESUME
- job.c do_clr_preason(): fix typo — second hook call was HOOK_BEFORE_EVENT
  instead of HOOK_AFTER_EVENT (double-before, zero-after bug)
- join.c handle_worker_join(): move xce_reply() to after HOOK_AFTER_EVENT
  so client ACK is not sent before WAL write completes (durability fix)
- testsuite/test_hooks.c: new TDD tests for JOB_STOP/RESUME enum values
- testsuite/controller_test.c + meson.build: register test_hooks suite

Reply ordering audit results:
  job.c handle_jobsubmit: reply before AFTER_EVENT — intentional (jobid
    returned to client before scheduler notification; WAL hook belongs in
    BEFORE_EVENT for this path, handled in Task 13)
  job.c do_set_preason: reply before set_preason() loop — intentional
    fire-and-forget (no AFTER_EVENT hook at top level)
  job.c do_clr_preason: reply before loop — same fire-and-forget pattern
  job.c do_jobcontrol_op: no AFTER_EVENT hook, reply only — OK
  join.c handle_worker_join: FIXED (reply moved after AFTER_EVENT)
  worker.c on_worker_update/dead: reply after all hooks — OK
  cluster.c: AFTER_EVENT hooks are internal (no reply) — OK
This commit is contained in:
Rongsong Shen 2026-05-07 12:08:52 +00:00
parent f905f891a8
commit 045d9e3287
6 changed files with 59 additions and 6 deletions

View file

@ -715,8 +715,12 @@ int on_job_stop(cluster_t* c, uint64_t jobid)
lock(job, RTTI_LOCK_WRITE);
state = job->job.state;
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_BEFORE_EVENT,
HOOK_DETAIL_JOB_STOP, c, job);
state |= JOB_STAT_USUSP;
mark_job_update(c, job, state);
job->job.state = state;
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_AFTER_EVENT,
HOOK_DETAIL_JOB_STOP, c, job);
unlock(job);
dereference(job);
@ -745,8 +749,12 @@ int on_job_resume(cluster_t* c, uint64_t jobid)
lock(job, RTTI_LOCK_WRITE);
state = job->job.state;
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_BEFORE_EVENT,
HOOK_DETAIL_JOB_RESUME, c, job);
state &= ~JOB_STAT_USUSP;
mark_job_update(c, job, state);
job->job.state = state;
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_AFTER_EVENT,
HOOK_DETAIL_JOB_RESUME, c, job);
unlock(job);
dereference(job);
@ -1023,7 +1031,7 @@ int do_clr_preason(cluster_t* c, endpoint_t* ep, xce_request_t* request)
FREEUP(job->pend_reasons);
}
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_BEFORE_EVENT,
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_AFTER_EVENT,
HOOK_DETAIL_CLRPREASON, c, j);
unlock(j);

View file

@ -62,10 +62,10 @@ static int handle_worker_join(cluster_t* c, endpoint_t* ep,
reply.status = idx;
reply.data_which = RequestReply_data_nodata;
xce_reply(ep, c->myhost, &reply);
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_WORKER, HOOK_AFTER_EVENT,
HOOK_DETAIL_WORKER_JOIN, c, from->name, idx);
xce_reply(ep, c->myhost, &reply);
unlock(c->workermgr);
if (idx < 0) {
ERROR("worker %s join failed, rc %d", from->name, idx);

View file

@ -73,6 +73,7 @@ controller_unit_test_src = [
'testsuite/test_persist_schema.c',
'testsuite/test_persist_checkpoint.c',
'testsuite/test_persist_recovery.c',
'testsuite/test_hooks.c',
'persist.c',
]

View file

@ -10,6 +10,7 @@ extern void test_persist_wal(void);
extern void test_persist_schema(void);
extern void test_persist_checkpoint(void);
extern void test_persist_recovery(void);
extern void test_hooks(void);
void setUp(void) { return; }
@ -89,6 +90,7 @@ int main(int argc, char* argv[])
TEST(test_persist_schema);
TEST(test_persist_checkpoint);
TEST(test_persist_recovery);
TEST(test_hooks);
return UNITY_END();
}

View file

@ -0,0 +1,40 @@
#include <stdio.h>
#include <unity/unity.h>
#include "xce/cluster_def.h"
static void test_hook_detail_job_stop_exists(void)
{
/* HOOK_DETAIL_JOB_STOP must be defined and distinct from JOB_UPDATE */
int stop_val = (int)HOOK_DETAIL_JOB_STOP;
int update_val = (int)HOOK_DETAIL_JOB_UPDATE;
TEST_ASSERT_TRUE(stop_val != 0);
TEST_ASSERT_TRUE(stop_val != update_val);
printf("test_hook_detail_job_stop_exists: PASS\n");
}
static void test_hook_detail_job_resume_exists(void)
{
int resume_val = (int)HOOK_DETAIL_JOB_RESUME;
int update_val = (int)HOOK_DETAIL_JOB_UPDATE;
TEST_ASSERT_TRUE(resume_val != 0);
TEST_ASSERT_TRUE(resume_val != update_val);
TEST_ASSERT_TRUE(resume_val != (int)HOOK_DETAIL_JOB_STOP);
printf("test_hook_detail_job_resume_exists: PASS\n");
}
static void test_hook_detail_job_stop_and_resume_are_distinct(void)
{
TEST_ASSERT_NOT_EQUAL((int)HOOK_DETAIL_JOB_STOP, (int)HOOK_DETAIL_JOB_RESUME);
printf("test_hook_detail_job_stop_and_resume_are_distinct: PASS\n");
}
void test_hooks(void)
{
printf("\n>> hook detail enum tests\n\n");
RUN_TEST(test_hook_detail_job_stop_exists);
RUN_TEST(test_hook_detail_job_resume_exists);
RUN_TEST(test_hook_detail_job_stop_and_resume_are_distinct);
}