Successor to scheduler-primary-lifecycle: a new per-node xce-node-manager component (one per host, role-assigned: manager/worker/apiserver/ reverse-proxy) owns starting/stopping XCE components for its node's role -- including starting/stopping the scheduler on a manager node in reaction to that node's controller reporting itself primary vs. standby over a local heartbeat (Unix domain socket, client logic shared via libxce) -- rather than the controller directly spawning/killing its own scheduler process. Carries forward scheduler-primary-lifecycle's underlying requirements (bug-1760's reactive-rejoin deadlock, wedge detection, graceful shutdown, controller-resilience invariants). Design review (kiro-validate-design) fixed 3 issues before GO: a uniform-single-OS-user assumption that was factually wrong given xce-deploy-ha's real worker daemon runs as root (resolved: root only on worker-role nodes); Requirement 4.3's wedge-detection mechanism verified directly against the actual scheduler code (sched_job_picker's cbuf_get() does an unbounded wait when idle, confirming the naive design would false-positive -- fixed by swapping to the existing cbuf_timedget() bounded-deadline variant); and PortCheckMonitor's TCP connect made explicitly non-blocking to match HeartbeatListener's own discipline. Tasks not yet generated. Checkpointing uncommitted spec work found in this shared worktree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
16 KiB
16 KiB
Research & Design Decisions
Summary
- Feature:
xce-node-manager - Discovery Scope: Complex Integration — new top-level daemon, no direct precedent, but heavily grounded in existing patterns across controller/worker/scheduler/apiserver and the AppImage packaging layer.
- Key Findings:
tools/appimage/entry.shis a thin per-role dispatcher (xce <role> args...) that execs exactly one ofxce_controller/xcecd/xce_scheduler/xce_apiserverin the foreground — there is no existing orchestration layer to extend or conflict with, andhaproxy(already bundled in the AppImage perbuild-deployment.md) has no dispatch case at all today, i.e. it is currently unreachable throughentry.sh.src/worker/src/process_worker_manager.c's existingprocess_manager_start_worker()spawns via rawfork()+execl(), notposix_spawn()— an already-shipped instance of the exact mutex-inheritance-deadlock hazard classproduct.md's locked decision ("nofork()for snapshots") and the supersededscheduler-primary-lifecycledesign already ruled out. Not fixed here (out of scope, unrelated component) but explicitly NOT a pattern to imitate.- The controller already has a non-blocking, delaymgr-driven, role-aware periodic task (
ha_start_primary_heartbeat/primary_heartbeat_fn,ha.c:9949) reading the atomicha->roleevery tick — proof that a cheap, always-safe periodic local report is a well-trodden pattern in this codebase, though this specific function is gated on first-promotion (s_armed) and therefore unsuitable to piggyback on directly (see Decision: Heartbeat Call Site). src/configs/hosts.yaml(the cluster's existing per-host config, distributed viaOPCode_replConfigPush) describes scheduler-visible compute-host attributes, not deployment/process-placement roles — confirms no existing config surface for "which node runs which XCE component," and that any such config cannot be cluster-distributed (a node's role must be knowable before any controller is reachable).
Research Log
Existing multi-component launch/packaging surface
- Context: Before designing a new orchestrator, check whether one already exists in disguise (AppImage bundling, systemd units, an existing launcher script).
- Sources Consulted:
tools/appimage/entry.sh,build-deployment.md(AppImage/Production Artifacts sections),xce-deploy-haSKILL.md,xce-test-ha/run-ha-soak.sh. - Findings: The AppImage bundles all daemons plus
haproxybehind onexce <role>entry point, butentry.shonly launches a single named process in the foreground per invocation — no supervision, no multi-process orchestration, no config-driven role resolution. The local dev/test harness (xce-deploy-ha) generates one shell script per component (start.sh,start-worker.sh,start-scheduler.sh), started manually/independently — the same "independently-started component" assumptionscheduler-primary-lifecycle's requirements already flagged, now generalized to worker/apiserver too. - Implications:
xce-node-manageris additive, not competing with an existing mechanism. The natural integration point is one newentry.shcase (manager) that execs the newxce_nodemgrbinary;xce_nodemgritself then resolves and spawns the other bundled binaries directly (samewhich xce_controller-style PATH resolutionentry.shalready uses), including finally wiring up the previously-unreachable bundledhaproxyfor the reverse-proxy role.
Controller-side periodic reporting hook
- Context: Where should the Controller's "I'm alive / I'm primary or standby" report be triggered from, without touching the promotion/demotion callback functions those steering docs flag as the riskiest, least-covered code in the controller?
- Sources Consulted:
ha.c(primary_heartbeat_fn,ha_start_primary_heartbeat,raft_cb_become_leader/raft_cb_become_follower— viacodegraph_explore),ha-async-io.md. - Findings:
primary_heartbeat_fnis armed only once, on a controller instance's first promotion (s_armedguard) — a controller that has never won an election never runs it, which would leavexce-node-managerunable to distinguish "controller alive, still standby" from "controller process is down" on any node that hasn't yet been promoted.raft_cb_become_leader/raft_cb_become_followerthemselves are flagged with no direct test coverage and are the single most call-dense, safety-critical functions in the controller (122+ calls fromraft_cb_become_leaderalone). - Implications: The report must come from a small, independent, unconditionally-armed periodic point tied to the Controller's own process lifetime (not gated on promotion), reading the same atomic
ha->rolefieldprimary_heartbeat_fnalready reads. This adds zero new logic toraft_cb_become_leader/raft_cb_become_follower— a strict reduction in touched surface area compared to the superseded spec, which called full spawn/monitor orchestration from insideraft_cb_become_leaderdirectly.
Scheduler wedge-detection signal shape
- Context: The superseded spec's Requirement 5 (carried forward as 4.3 here) requires detecting a scheduler that is connected but not making real scheduling progress — a bare "I'm alive" ping cannot distinguish a wedged decision-loop from a healthy one if the two run on different threads.
- Sources Consulted: In-conversation review of
scheduler-primary-lifecycle'sdesign.md(SchedulerConnectionBookkeeping's last-progress-timestamp approach, fed from real dispatch/pend-reason traffic, not a synthetic heartbeat); bug-1760's own root-cause text describing the scheduler's outbound wire traffic as reactive/idle-silent ("the scheduler has nothing queued to publish"). - Findings: Reusing the scheduler's reactive wire-publish call as the heartbeat trigger would reintroduce the exact idle-silence failure mode bug-1760 already exposed. The correct trigger is the scheduler's own internal round/picker loop iteration — which must tick on a bounded cadence regardless of whether a round finds any job to act on — not the wire-publish call.
- Implications: One wire-format field (a monotonically increasing sequence number) serves double duty: for the Controller/Worker/Apiserver it is a bare liveness counter; for the Scheduler, since it is incremented once per round-loop iteration (not once per successful publish), its advancement is the real-progress signal Requirement 4.3 needs, with no separate progress field required. See Decision: Unified Heartbeat Wire Format.
Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|---|---|---|---|---|
| Adopt a generic external supervisor (systemd user units, s6, runit, supervisord) for all four roles | Delegate process start/restart entirely to an existing, battle-tested tool | Zero new code for the generic "keep running" case (Requirement 2) | Cannot express Requirement 3's Controller-role-driven conditional scheduler start/stop or Requirement 4.3's progress-aware wedge detection — those are XCE-domain-specific decisions no generic supervisor can make; also conflicts with the AppImage's self-contained, zero-external-dependency portability goal (product.md) across arbitrary air-gapped target hosts that may not run systemd |
Rejected as the primary mechanism; the generic host service manager is still the right tool for the one thing left over — restarting xce-node-manager itself, already scoped out of this feature (Requirement 7 boundary) |
Custom xce-node-manager daemon using only POSIX primitives (posix_spawn, Unix domain datagram sockets), consistent with the already-reviewed scheduler-primary-lifecycle precedent |
Build a small, purpose-specific supervisor | Can express both the generic and Controller-role-driven policies; no new external runtime dependency; reuses the posix_spawn decision already vetted against the mutex-inheritance-deadlock hazard |
New component to build and test | Selected |
Design Decisions
Decision: Heartbeat call site is each component's own natural loop iteration, not a new thread
- Context: Requirements 4.1 and 4.3 require every managed component to report liveness, and require the Scheduler's report specifically to reflect real progress.
- Alternatives Considered:
- A dedicated heartbeat-sender thread per component, independent of its main work loop — simplest to bolt on, but structurally cannot distinguish a wedged work thread from a healthy one (the sender thread keeps reporting regardless).
- Piggyback the Controller's report on the existing
primary_heartbeat_fn— rejected, see Research Log (promotion-gated arming leaves never-yet-primary controllers silent).
- Selected Approach: Each component calls a shared
libxceheartbeat primitive once per iteration of its own existing loop (the Controller's iomgr/delaymgr-driven event loop, the Scheduler's round loop, the Worker'sipc_worker_run/usleeploop); the Apiserver, having no persistent loop of its own (a request-driven Gin server), uses a small dedicated ticker goroutine instead — the one place a new "thread" is unavoidable, since there is no existing loop to hook into. - Rationale: Guarantees the report can only advance when the component's own real work-loop is actually turning, which is what makes the same signal double as both liveness and (for the Scheduler) progress. The Scheduler's live driver (
scheduler.c,sched_job_picker) is the correct call site, not the pure decision engine incore.c/core_events.c/core_session.c/core_clock.c— thescheduler-corespec's own core/driver separation already keeps the core free of controller I/O, and this feature's local heartbeat is new outside-facing I/O that belongs on the same side of that boundary as the existingxcecliconnection. The Scheduler's separate simulation-mode driver (sim.c) gets no heartbeat call site at all and stays entirely outsidexce-node-manager's awareness (see Non-Goals indesign.md) — it is a standalone, manually-invoked tool with no Controller or primary/standby concept. - Trade-offs: Slightly different integration shape per component (loop hook vs. ticker) instead of one uniform mechanism; acceptable since the underlying wire format and
libxceprimitive are identical either way. - Follow-up resolved during design review (not deferred to implementation):
codegraph_exploreconfirmedsched_job_pickercallscbuf_get()(src/lib/mylock.c:144), an unboundedpthread_cond_waitwhen the job queue is empty — a heartbeat placed naively at the top of that loop would never fire during genuine idle periods, causingxce-node-managerto falsely treat a healthy idle Scheduler as wedged and kill it. Fix: swap tocbuf_timedget()(same file, an existing bounded-deadline variant already used elsewhere in the codebase) with a bounded idle-tick deadline, and call the heartbeat primitive on both the real-job path and the timeout path.
Decision: Unified heartbeat wire format, self-identifying payload, no CGO for the Apiserver client
- Context: The heartbeat protocol must be usable by four component types across two languages (C daemons, Go apiserver) over one shared local socket, with the receiving
xce-node-managerable to attribute each datagram without relying on socket-level peer identity (aSOCK_DGRAMUnix socket exposes no reliable peer address unless the sender also binds, which adds needless coupling). - Alternatives Considered:
- Route the Apiserver's heartbeat through
libxcevia the existing CGO bridge, matching howapiserver's C-tier calls already work. - A small, independently-implemented Go client speaking the identical wire format natively, with no CGO involved.
- Route the Apiserver's heartbeat through
- Selected Approach: One canonical fixed-layout binary wire format (magic/version, role, status, pid, sequence number — no Cap'n Proto), implemented once in
libxcefor the three C daemons (which already link it natively, zero extra cost), and once as a small native Go implementation for the Apiserver. - Rationale: The payload is a fixed, trivial, few-field structure — reimplementing it natively in Go is a handful of lines with negligible drift risk, and avoids pulling a CGO dependency into a code path (a periodic status ticker) that has no other reason to touch the C tier;
tech.mdalready documents that Apiserver's own unit tests deliberately runCGO_ENABLED=0, and only the existing backend bridge work requires the CGO build. The payload self-identifies its sender (role + pid fields), so the receiver never depends on socket-level peer identity. This is a deliberate, narrow exception tostructure.md's "Cap'n Proto for wire formats" convention: that convention targets the real cross-host, cross-version network protocol (controller.capnp/c.capnp), where schema evolution and cross-language generated bindings earn their cost; this protocol is same-host-only, always emitted and consumed by binaries from the same build (Node Manager and the components it manages ship together), and confirmed (go.mod/go.suminspection) that the Go apiserver carries no existing capnp dependency at all — adopting Cap'n Proto here would mean a new Go dependency purely for a fixed 4-field local datagram, which the Simplification lens rejects as disproportionate. - Trade-offs: Two implementations of the same format instead of one; mitigated by the format's small size and by both being validated against the same contract in this design.
- Follow-up: none — the format is fixed at design time (see Data Models in
design.md).
Decision: Node Manager privilege is role-conditional (root only on worker-role nodes)
- Context:
xce-deploy-ha's own tooling documents that the real (non-virtual) Worker daemonxcecdruns as root ("plain kill can't touch it," needssudo) — discovered during design review, not anticipated during requirements. A uniform single-OS-user assumption forxce-node-manager(as first drafted) would leave it unable to start/stop/signalxcecdon a real worker-role node. - Alternatives Considered:
xce-node-manageralways runs as root everywhere, regardless of role.xce-node-managerstays unprivileged everywhere; grant it a narrow Linux capability or scopedsudorule for the worker case specifically.- Treat real (root-requiring) worker management as out of scope for this spec; only support
xcecd_virtual.
- Selected Approach (user decision):
xce-node-managerruns as root only on a node whose assigned role includesworker; on manager/apiserver/reverse-proxy-only nodes it runs as an ordinary service account. This is fixed by the Node Role Manifest at Node Manager startup, not a runtime toggle. - Rationale: Matches
xcecd's own pre-existing requirement exactly, with no new privilege-escalation surface beyond what that daemon already needs; simpler than a capability-based scheme, and avoids narrowing this spec's worker-role support to the virtual test double only. - Trade-offs: The heartbeat socket's permission model is now per-node-role rather than one uniform story (see
design.mdSecurity Considerations) — an implementer must not assume a single OS user across all node roles. - Follow-up: none — fixed at design time.
Risks & Mitigations
- A node-manager crash on a manager node strands its Controller's Scheduler with nothing left to stop it on a subsequent demotion — mitigated by retaining a scheduler-side independent controller-liveness check as a backstop (Requirement 6), so the Scheduler never depends solely on
xce-node-managerstaying alive. xce-node-managerrestarting could double-spawn a component that is already running — mitigated by a small persisted per-role record (role, pid, start time, binary path) it consults on startup before spawning (seedesign.mdData Models); a stale/mismatched record falls back to a fresh start rather than blindly trusting the file, and each component's own listen-port bind-failure is a natural backstop against an accidental true duplicate.- Sizing the heartbeat interval/timeout thresholds is an implementation-time tuning decision (same posture the superseded spec already took for its own liveness thresholds), not a design decision.
References
.kiro/specs/scheduler-primary-lifecycle/design.md— superseded design; source of theposix_spawnmutex-inheritance-deadlock finding and the wedge-detection real-progress principle, both carried forward here..wolf/buglog.jsonbug-1760 — motivating incident for the scheduler-lifecycle requirements this spec restates.