docs: save scheduler-config-version-tag spec (requirements/design/tasks)

Requirements, design, research, and tasks all generated and approved
(ready_for_implementation: true). Implementation not yet started (0/8
tasks) -- saving the completed spec authoring; the 8-task
implementation is separate future work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
This commit is contained in:
Rongsong Shen 2026-08-29 11:47:28 +00:00
parent 347e996e76
commit 42fdeb6669
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
5 changed files with 483 additions and 0 deletions

View file

@ -0,0 +1,283 @@
# Design Document
## Overview
**Purpose**: This feature lets the scheduler recognize when a controller's configuration push carries content it already has, so it can skip redundant resource/shared-value/queue re-registration — eliminating the burst of false error-level log entries that currently fires on every HA-failover reconnect (bug-1727/1728/1729).
**Users**: Operators monitoring scheduler logs during HA failover, who currently see 20-33 misleading error entries per failover cycle with no way to tell them apart from a genuine resource-registration failure.
**Impact**: Extends the controller's existing config-push payload (`Conf4Sched`) with one new field, and adds one comparison + one stored value to the scheduler's core state. No change to what configuration data is sent, how HA election/failover works, or how explicit administrator resource/queue update operations behave.
### Goals
- A scheduler reconnect that delivers an unchanged configuration produces zero error-level log entries and does not re-run resource/shared-value/queue registration.
- A scheduler reconnect (or live config push) that delivers a genuinely changed configuration is still fully applied, exactly as today.
- The skip decision reuses the controller's existing, already-replicated config-identity value — no new versioning or consensus mechanism.
- Both existing config-application code paths (join/reconnect and live config upgrade) share one consistent skip decision.
### Non-Goals
- Changing how the controller computes, stores, or replicates its config-identity value.
- Changing the add-or-update behavior of explicit administrator resource/queue update operations (`XCE_SCHED_OP_UPDATE_RESOURCES` and equivalents) — unaffected, per Requirement 4.
- Changing what configuration data is sent in a push, or the wire shape of anything other than the one new field.
- Any change to HA election, promotion, or failover mechanics.
## Boundary Commitments
### This Spec Owns
- The `configVersionTag` field on the `Conf4Sched` wire message and its population by the controller.
- The scheduler's stored "last-applied configuration identity" state and the comparison that decides skip vs. full apply.
- The skip/apply branching in both existing config-application code paths (join/reconnect via `sched_init_config()`, and live config upgrade via `core_events.c`'s `XCE_CORE_EVENT_CONFIG_PUSH` handling), and the diagnostic-level log entry recorded on skip.
- The memory-ownership responsibility for the incoming config payload when it is not adopted (skip branch).
### Out of Boundary
- Computation or replication of the controller's underlying config-identity value (`config_version_tag` in `controller/config_repl.c`) — an existing, unmodified mechanism this feature only reads.
- The add-or-update logic for `XCE_SCHED_OP_UPDATE_RESOURCES` and any other explicit administrator update operation — untouched, verified against Requirement 4.
- HA election, promotion, heartbeat, or failover logic in `controller/ha.c`.
- Any change to the scheduler's `.replay` snapshot save/load format (`snapshot.c`/`snapshot_load.c`) — see Data Models' explicit scope decision below.
### Allowed Dependencies
- The controller's existing `myconf->config_version_tag` (`controller/config_repl.c`), read-only.
- The existing `Conf4Sched`/`xce_conf4sched_t` capnp-generated type and its existing `free_Conf4Sched_ptr()` deallocator.
- The existing `xce_sched_core_t` struct (`scheduler/include/core.h`) as the storage location for the new comparison state.
- The existing two config-application call sites (`scheduler/src/scheduler.c`'s `sched_init_config()`, `scheduler/src/core_events.c`'s `XCE_CORE_EVENT_CONFIG_PUSH` case) as the two integration points — not new call sites.
### Revalidation Triggers
- `config_version_tag`'s computation in `controller/config_repl.c` becomes dependent on which node is primary/leadership state (would break the cross-node stability this feature's safety rests on).
- `Conf4Sched`'s field list or generated-binding mechanism changes shape.
- A third live config-application code path is introduced (would need the same skip-decision wiring to stay consistent with Path A/Path B).
- `xce_sched_core_t`'s zero-initialization contract changes (this feature's "no prior config" sentinel relies on `config_version_tag` starting at `0`).
### Project Invariant Cross-Check
- **Async I/O (delaymgr trigger-only rule)**: not applicable — this feature adds one `uint64_t` comparison to code that already runs synchronously on the existing request-handling/event-translation path (`handler.c`'s `scheduler_req_handler`, `driver.c`'s event-translation loop), neither of which is delaymgr. No new blocking or I/O is introduced; the skip path does strictly less work than today.
- **Controller self-termination rule**: not applicable — no controller-side control-flow or exit path is touched; the controller-side change is a single field population in an existing send path.
- **Save/restore field-completeness**: not applicable to the `.replay` snapshot format — see Data Models section for the explicit scope decision.
## Architecture
### Existing Architecture Analysis
- The controller already computes a stable, content-derived config identity (`config_version_tag`) and keeps it consistent across HA nodes via the existing config-replication mechanism (`controller/config_repl.c`) — this feature is a pure consumer, not a new producer.
- Two independent code paths apply a pushed config to the scheduler's core state, both unconditionally re-registering every resource/shared-value/queue on every invocation (see `research.md`'s "Which code path(s)" finding): Path A (join/reconnect, `scheduler.c`'s `sched_init_config()`) and Path B (live config upgrade, `core_events.c`'s `XCE_CORE_EVENT_CONFIG_PUSH` case). Both read/write the same underlying `xce_sched_core_t`.
- `xce_sched_core_t` already carries a self-incrementing local counter, `configver`, bumped by both paths on every apply — this feature adds a second, independent field next to it (the controller's own content identity, not a local counter).
### Architecture Pattern & Boundary Map
One new wire field, one new stored value + one shared comparison function on the core, consumed identically by both existing application paths.
```mermaid
graph TB
ConfigRepl[controller config_repl.c config_version_tag]
PushConfigs[controller sched.c xcesched_push_configs]
Wire[Conf4Sched configVersionTag]
PathA[scheduler.c sched_init_config Path A reconnect]
PathB[core_events.c CONFIG_PUSH handling Path B live upgrade]
Compare[core.c config_changed comparison]
CoreState[xce_sched_core_t config_version_tag]
ConfigRepl --> PushConfigs
PushConfigs --> Wire
Wire --> PathA
Wire --> PathB
PathA --> Compare
PathB --> Compare
Compare --> CoreState
CoreState --> Compare
```
**Architecture Integration**:
- Selected pattern: shared core-level comparison function consumed by both existing application paths (see `research.md`'s Architecture Pattern Evaluation for the rejected alternatives).
- Domain/feature boundaries: the controller owns computing/replicating the identity value (unchanged); the scheduler core owns storing the last-applied value and deciding skip vs. apply; each application path (A/B) owns only invoking that decision and branching its own registration calls accordingly.
- Existing patterns preserved: capnp schema → generated C struct → existing per-path apply logic, unchanged in shape; only one new field and one new conditional branch per path.
- New components rationale: one new pure function (`xce_sched_core_config_changed`) is the only new component — everything else extends an existing struct field list or existing conditional logic.
- Steering compliance: TDD required for every new behavior (`testing.md`); no new blocking work on any async-I/O-constrained thread (see Project Invariant Cross-Check above).
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|-----------------|-------|
| Wire schema | Cap'n Proto (`schema/controller.capnp`, existing toolchain) | Carries `configVersionTag` from controller to scheduler | One new `UInt64` field on the existing `Conf4Sched` struct; generated bindings regenerate automatically via the existing meson `CUSTOM_COMMAND` |
| Controller (C) | `controller/sched.c` | Populates the new field from the existing `myconf->config_version_tag` | No new dependency |
| Scheduler (C) | `scheduler/src/core.c`/`core.h`, `scheduler/src/scheduler.c`, `scheduler/src/core_events.c` | Stores the last-applied identity, compares, branches skip/apply, frees the redundant payload on skip | No new dependency |
## File Structure Plan
### Modified Files
- `schema/controller.capnp` — add `configVersionTag @9: UInt64;` to the `Conf4Sched` struct (next available field number after `limits @8`).
- `controller/sched.c` — in `xcesched_push_configs()`, populate `configdata.data.conf4sched->configVersionTag = c->myconf->config_version_tag;`.
- `scheduler/include/core.h` — add `uint64_t config_version_tag;` to `xce_sched_core_t` (alongside the existing `configver` field); declare `bool xce_sched_core_config_changed(xce_sched_core_t* core, const xce_conf4sched_t* config);`.
- `scheduler/src/core.c` — define `xce_sched_core_config_changed()`: returns `true` when `config == NULL`, or `config->configVersionTag == 0`, or `config->configVersionTag != core->config_version_tag`; returns `false` (unchanged) only when `config->configVersionTag != 0 && config->configVersionTag == core->config_version_tag`.
- `scheduler/src/scheduler.c` — `sched_init_config()` (Path A): call `xce_sched_core_config_changed()` before the existing `sched_init_resources()`/`sched_init_sharedInst()`/`sched_init_queues()` calls; on unchanged, skip those three calls, free the incoming `config` via `free_Conf4Sched_ptr()`, log at `XCE_LOG_DEBUG`, and return without adopting it as `sched->core.config`; on changed, keep today's behavior and additionally set `sched->core.config_version_tag = config->configVersionTag;`.
- `scheduler/src/core_events.c` — the `XCE_CORE_EVENT_CONFIG_PUSH` case (Path B): same skip/apply branching as Path A, applied to `core_apply_config_resources()`/`core_apply_config_resvals()`/`core_apply_config_queues()`; the existing `picker->picker_iface->on_config()` hook still fires unconditionally in both branches (unaffected by this feature — it is not a resource/shared-value/queue registration call).
### New Test Files
- `scheduler/testsuite/test_core_config_version.c` — unit tests for `xce_sched_core_config_changed()` in isolation (pure function, no I/O).
- Extends existing `scheduler/testsuite/test_handlers.c` (or the file covering `sched_init_config()`/Path A today) with skip/apply/first-push/zero-tag cases.
- Extends existing `scheduler/testsuite/test_core_events.c` (Path B's `CONFIG_PUSH` coverage) with the equivalent skip/apply cases.
- `controller/testsuite/` — extends the existing test covering `xcesched_push_configs()` (or equivalent) to assert `configVersionTag` is populated from `myconf->config_version_tag`.
> No changes to `scheduler/src/snapshot.c`/`snapshot_load.c` or their test files — see Data Models' explicit scope decision.
## System Flows
```mermaid
sequenceDiagram
participant Ctrl as Controller
participant Sched as Scheduler Path A/B
participant Core as xce_sched_core_t
Ctrl->>Sched: Config push (configVersionTag = T)
Sched->>Core: xce_sched_core_config_changed(core, config)
alt T != 0 and T == core.config_version_tag
Core-->>Sched: unchanged
Sched->>Sched: free incoming config payload
Sched->>Sched: log DEBUG config unchanged
else T == 0 or T != core.config_version_tag
Core-->>Sched: changed
Sched->>Core: register resources, shared values, queues
Sched->>Core: core.config_version_tag = T
Sched->>Core: core.config = incoming config payload
end
```
**Key decisions visible in the diagram**: the comparison happens once, before any registration work; the incoming payload is either adopted (changed branch) or freed (unchanged branch) — it is never both, and never leaked.
## Requirements Traceability
| Requirement | Summary | Components | Interfaces | Flows |
|-------------|---------|------------|------------|-------|
| 1.1 | Config push carries an identity indicator | `Conf4Sched` schema, `xcesched_push_configs()` | `configVersionTag` field | Sequence diagram, "Config push" step |
| 1.2 | Same content → same indicator across nodes | `config_version_tag` (existing, controller) | — | research.md "Is config_version_tag safe to trust" |
| 1.3 | Different content → different indicator | `config_version_tag` (existing, controller) | — | research.md, same |
| 1.4 | Indicator derived from existing replicated identity | `xcesched_push_configs()` | — | Architecture diagram |
| 2.1 | Skip re-registration on match | `xce_sched_core_config_changed()`, Path A, Path B | `xce_sched_core_config_changed()` | Sequence diagram, "unchanged" branch |
| 2.2 | No error-level log on skip | Path A, Path B | — | Sequence diagram, "unchanged" branch |
| 2.3 | Existing state retained on skip | Path A, Path B (no-op on skip) | — | Sequence diagram, "unchanged" branch |
| 2.4 | Diagnostic-level log on skip | Path A, Path B | — | Sequence diagram, "unchanged" branch |
| 3.1 | Full apply on mismatch | Path A, Path B | Existing `sched_init_resources()`/etc., `core_apply_config_resources()`/etc. | Sequence diagram, "changed" branch |
| 3.2 | Adopt new indicator after apply | Path A, Path B | `core.config_version_tag` assignment | Sequence diagram, "changed" branch |
| 3.3 | First-ever push always applied | `xce_sched_core_config_changed()` (zero-initialized `config_version_tag`) | — | Design Decisions: "Zero as sentinel" |
| 3.4 | Controller "no config" indicator always applied | `xce_sched_core_config_changed()` (`configVersionTag == 0` check) | — | Design Decisions: "Zero as sentinel" |
| 4.1 | Admin update operations unaffected | Out of Boundary (untouched) | — | — |
| 4.2 | Admin update on a skipped-push resource still works | Out of Boundary (untouched) | — | — |
## Components and Interfaces
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|-----------|--------------|--------|--------------|--------------------------|-----------|
| `Conf4Sched.configVersionTag` | Wire schema | Carries config identity to the scheduler | 1.1, 1.4 | `config_repl.c`'s `config_version_tag` (P0) | State |
| `xcesched_push_configs()` (modified) | Controller | Populates the new field | 1.1, 1.4 | `myconf->config_version_tag` (P0) | State |
| `xce_sched_core_config_changed()` (new) | Scheduler core | Pure skip/apply decision | 2.1, 3.1, 3.3, 3.4 | `xce_sched_core_t.config_version_tag` (P0) | Service |
| `sched_init_config()` (modified, Path A) | Scheduler | Join/reconnect config application, branches on the decision | 2.1-2.4, 3.1-3.2 | `xce_sched_core_config_changed()` (P0), `free_Conf4Sched_ptr()` (P0) | Service |
| `core_events.c` `CONFIG_PUSH` case (modified, Path B) | Scheduler core | Live config-upgrade application, branches on the decision | 2.1-2.4, 3.1-3.2 | `xce_sched_core_config_changed()` (P0), `free_Conf4Sched_ptr()` (P0) | Event |
### Scheduler Core
#### `xce_sched_core_config_changed`
| Field | Detail |
|-------|--------|
| Intent | Decide whether a pushed configuration differs from the one currently held, using the controller's content-identity indicator |
| Requirements | 2.1, 3.1, 3.3, 3.4 |
**Responsibilities & Constraints**
- Pure function: no side effects, no I/O, no locking (reads two `uint64_t` values already available to the caller).
- Treats `configVersionTag == 0` (controller reports no established config) as always "changed," per 3.4.
- Treats `core->config_version_tag == 0` (no configuration applied since process start, or never yet applied) as always "changed," per 3.3 — this falls out of the same comparison without a separate flag, since `xce_sched_core_t` is zero-initialized and a real content-derived tag is practically never exactly `0`.
**Dependencies**
- Inbound: `sched_init_config()` (Path A, P0), `core_events.c`'s `CONFIG_PUSH` case (Path B, P0)
- Outbound: none
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
##### Service Interface
```c
/* Returns true when `config` should be fully applied (registered), false
* when it is identical to what `core` already holds and may be skipped. */
bool xce_sched_core_config_changed(xce_sched_core_t* core,
const xce_conf4sched_t* config);
```
- Preconditions: `core != NULL`.
- Postconditions: no mutation of `core` or `config`; return value is deterministic given the two inputs.
- Invariants: `xce_sched_core_config_changed(core, config) == false` implies `config->configVersionTag == core->config_version_tag` and both are nonzero.
**Implementation Notes**
- Integration: called once, before any resource/shared-value/queue registration, in both Path A and Path B.
- Validation: unit-tested in isolation (`test_core_config_version.c`) — no fixture beyond two `uint64_t` values and a minimal `xce_conf4sched_t`.
- Risks: none — pure function, fully covered by unit tests.
### Scheduler Application Paths (Path A / Path B)
Both paths get the identical shape of change; documented once, applied at two call sites (`scheduler.c`'s `sched_init_config()` and `core_events.c`'s `CONFIG_PUSH` case).
| Field | Detail |
|-------|--------|
| Intent | Apply or skip a pushed configuration based on the shared decision, owning the incoming payload's lifetime either way |
| Requirements | 2.1, 2.2, 2.3, 2.4, 3.1, 3.2 |
**Responsibilities & Constraints**
- On "changed": unchanged from today's behavior (full registration), plus one new line adopting `configVersionTag` into `core->config_version_tag`.
- On "unchanged": skip the resource/shared-value/queue registration calls entirely; do not touch existing `core` state; free the incoming `config` payload via `free_Conf4Sched_ptr()` (it is not adopted as `core->config`, so it has no other owner); log one `XCE_LOG_DEBUG` entry identifying the skip.
- Neither branch changes the existing unconditional `picker->picker_iface->on_config()` call (Path B) or `plugin_config_update()` call (Path A) — out of this feature's boundary (Non-Goals).
**Dependencies**
- Inbound: `handler.c`'s `scheduler_req_handler()` (Path A, P0), `driver.c`'s event-translation loop via `xce_core_push_events()` (Path B, P0)
- Outbound: `xce_sched_core_config_changed()` (P0), `free_Conf4Sched_ptr()` (P0)
**Contracts**: Service [ ] / API [ ] / Event [x] / Batch [ ] / State [x]
##### Event Contract
- Consumed events: Path A's `Request_body_configpushmsg`; Path B's `XCE_CORE_EVENT_CONFIG_PUSH`.
- Ordering / delivery guarantees: unchanged from today — each path already processes pushes strictly in arrival order; this feature adds no new ordering requirement.
##### State Management
- State model: `xce_sched_core_t.config_version_tag`, a single `uint64_t`, updated only on the "changed" branch.
- Persistence & consistency: in-memory only, process-lifetime; resets to `0` on scheduler restart, which correctly forces a full apply on the next push (Requirement 3.3) — see Data Models.
- Concurrency strategy: unchanged from today's existing single-threaded-per-path access pattern for `core->config`/`core->configver` (both paths already mutate these without new locking; this feature's field follows the same existing access discipline).
**Implementation Notes**
- Integration: two call sites, one shared decision function — see File Structure Plan.
- Validation: TDD — extend each path's existing test file with skip/apply/first-push/zero-tag cases before changing the production code (`testing.md`).
- Risks: memory leak if the free-on-skip step is omitted at either call site — mitigated by an explicit test asserting no leak under ASAN (the project's default debug build) for the skip path at both call sites.
## Data Models
### Domain Model
- No new domain entity. `config_version_tag` is a scalar attribute added to two existing structures: `Conf4Sched` (wire) and `xce_sched_core_t` (in-memory scheduler state).
### Logical Data Model
- `Conf4Sched.configVersionTag: UInt64` — new wire field, always present (defaults to `0` if a sender predates this feature, which this feature's own comparison already treats as "always changed," so an old controller talking to a new scheduler degrades safely to today's unconditional-apply behavior).
- `xce_sched_core_t.config_version_tag: uint64_t` — new in-memory field, zero-initialized, updated only on a full config apply.
### Explicit Scope Decision — Not Part of Snapshot Save/Restore
`config_version_tag` is **not** added to `sched_snapshot_data_t` or the `.replay` snapshot format (`snapshot.c`/`snapshot_load.c`), and is **not** restored on snapshot load. Rationale:
- It is live-session reconnect-optimization state, not part of the scheduling-decision-relevant entity the snapshot format exists to reconstruct (jobs/workers/queues/resources/allocations) — its absence does not change any scheduling decision or resource accounting.
- The simulation/replay driver (`sim.c`) never calls `sched_init_config()` or pushes an `XCE_CORE_EVENT_CONFIG_PUSH` event at all (`sim.c`'s own documented scope boundary) — Path A and Path B, and therefore this feature, are entirely inapplicable to snapshot/replay loading.
- On a real scheduler process restart, `config_version_tag` naturally zero-initializes, which this feature's own comparison already treats as "no prior config" — forcing exactly one full re-apply on the next live push after restart. This is the same safe, existing behavior a restart already produces today (no regression, no special-cased recovery logic needed).
### Data Contracts & Integration
**Wire field**: `Conf4Sched.configVersionTag: UInt64` (Cap'n Proto, `schema/controller.capnp`), populated by `xcesched_push_configs()` from `myconf->config_version_tag`, consumed identically by Path A and Path B on the scheduler side. No versioning strategy needed beyond the existing capnp schema-evolution convention already used for every other field in this struct (new fields get the next unused field number; older readers/writers default absent fields to `0`, which this feature's comparison already treats as "always apply").
## Error Handling
### Error Strategy
This feature introduces no new error path. It removes a class of misleading error-level log entries (Requirement 2.2) by recognizing them as a no-op case, not by suppressing or downgrading a real failure — a genuine resource-registration failure (e.g., a malformed resource definition) still occurs, and still logs, on the "changed" branch exactly as today, since that branch is otherwise unmodified.
### Error Categories and Responses
- **Redundant, unchanged push** (not an error): recognized and skipped per Requirement 2; logged at `XCE_LOG_DEBUG` only.
- **Genuinely changed push, registration failure** (existing behavior, unmodified): still logs at whatever level the existing `sched_init_resources()`/`core_apply_config_resources()` etc. already use today — out of this feature's boundary.
- **Malformed/absent config payload** (`config == NULL`): `xce_sched_core_config_changed()` returns `true` (changed) for a `NULL` config, matching each path's existing `NULL`-tolerant behavior (Path B already null-checks before calling the apply trio; Path A's caller already guarantees non-`NULL` per the existing capnp decode contract) — no new failure mode introduced.
### Monitoring
No new metrics. The new `XCE_LOG_DEBUG` entry (Requirement 2.4) is the only new observability surface, deliberately below the operating log level so it does not itself become new noise.
## Testing Strategy
### Unit Tests
- `xce_sched_core_config_changed()`: matching nonzero tags → `false`; differing nonzero tags → `true`; incoming tag `0` → `true`; stored tag `0` (fresh core) → `true`; `NULL` config → `true`.
- Path A (`sched_init_config()`): unchanged push does not call `makeResource()`/queue-add functions (assert via existing call-count/spy conventions this test suite already uses) and does not emit an error-level log line; changed push behaves exactly as today plus adopts the new tag.
- Path B (`core_events.c`'s `CONFIG_PUSH` case): same two cases as Path A, exercised via `xce_core_push_events()`.
### Integration Tests
- Controller `xcesched_push_configs()` populates `configVersionTag` from `myconf->config_version_tag` (assert the wire struct field, not just that the function returns success).
- End-to-end: two consecutive config pushes with identical controller-side config content produce two different observable outcomes (first: full registration; second: skip, zero error-level log lines) — this is the direct regression test for bug-1727/1728/1729's originally-reported symptom.
- Leak check (ASAN): the skip branch, exercised at both Path A and Path B, produces no leaked `xce_conf4sched_t`.
### E2E / Live HA Tests
- Live 3-node HA soak (`xce-test-ha` conventions): trigger a failover and confirm the reconnecting scheduler's log contains zero error-level `sched_init_resources`/`core_apply_config_resources` entries for the reconnect, while a genuinely changed config pushed mid-soak is still fully applied and observable (e.g., via `info resource`/`info worker` in the scheduler REPL, matching this session's earlier live-verification technique).

View file

@ -0,0 +1,55 @@
# Requirements Document
## Project Description (Input)
Thread the controller's existing config_version_tag (SHA-256-derived content fingerprint, controller/config_repl.c) through the scheduler CONFIG_PUSH wire message (Conf4Sched, schema/controller.capnp) so the scheduler can detect an unchanged config on HA-failover reconnect and skip redundant re-initialization. Currently sched_init_resources() (scheduler.c:828) and its mirror core_apply_config_resources() (core_events.c:363) unconditionally call makeResource() for every configured resource on every CONFIG_PUSH -- which fires on every scheduler reconnect/re-registration during HA failover, not just on genuine config changes -- and log ERROR for every already-registered resource (makeResource returns NULL/EEXIST), producing a burst of 20-33 false ERROR log lines per failover cycle even though nothing is actually broken (this is bug-1727/1728/1729's session-sibling finding, logged in .wolf/buglog.json).
Design direction already agreed with the user:
1. Schema: add a configVersionTag (UInt64) field to Conf4Sched in schema/controller.capnp.
2. Controller: in xcesched_push_configs() (controller/sched.c:138-199), populate the new field from c->myconf->config_version_tag (already computed, already stable across HA failover/leadership changes -- confirmed by reading config_repl.c:686 and config_repl.c:906, both config-load/config-replication events only, never touched by ha.c's election code).
3. Scheduler: store the last-applied tag (new field on xce_sched_core_t, alongside the existing self-incrementing core->configver in scheduler.c/core.h); in sched_init_config() (scheduler.c:934) and/or its core_events.c CONFIG_PUSH-handling mirror, compare the incoming tag against the stored one -- if equal, skip sched_init_resources()/sched_init_sharedInst()/sched_init_queues() entirely (log at DEBUG that the config was unchanged); if different, run the full init and update the stored tag.
This must follow this project's TDD discipline (test in testsuite/ before the code change) and needs to preserve today's existing behavior for a genuinely new/changed config (full re-init still happens, no regression). Needs to be verified against a live 3-node HA soak per this project's HA testing conventions (xce-test-ha and/or the existing scheduler reconnect scenario) since it touches the controller<->scheduler wire protocol and HA reconnect path.
## Introduction
On every scheduler reconnect to a controller — including the routine re-registration that happens on every HA failover, not just when an operator actually changes the cluster configuration — the controller re-sends the full configuration to the scheduler, and the scheduler unconditionally re-runs its resource/queue/shared-value initialization. Because that initialization currently cannot tell "this is the exact same configuration I already have" apart from "this is a genuinely new configuration," every reconnect with an unchanged configuration produces a burst of false error log entries (one per already-registered resource) even though nothing is actually broken. This feature gives the scheduler a reliable way to recognize an unchanged configuration on reconnect and skip the redundant re-initialization, while still fully re-initializing whenever the configuration has genuinely changed.
## Boundary Context (Optional)
- **In scope**: carrying a configuration-identity indicator from the controller to the scheduler on every configuration push; the scheduler comparing that indicator against the configuration it already holds; skipping resource/queue/shared-value re-initialization only when the indicator matches; always falling back to full re-initialization when it does not match or has never been seen before.
- **Out of scope**: changing how the controller computes or replicates its own configuration-identity indicator (an existing mechanism this feature reuses, not redefines); changing the add-or-update behavior of explicit administrator-initiated resource/queue update operations (a separate, already-existing code path); changing what configuration data is sent on a push (only that an identity indicator accompanies it); any change to how HA election, failover, or primary promotion itself works.
- **Adjacent expectations**: this feature depends on the controller's existing configuration-identity indicator being identical across controller nodes for the same configuration content, and unaffected by which node is currently primary — that guarantee is provided by the existing configuration replication mechanism, not by this feature. If a newly-promoted primary ever reports a different indicator for what is actually the same configuration, that is a defect in the underlying replication guarantee, not something this feature is responsible for detecting or correcting.
## Requirements
### Requirement 1: Configuration Push Carries a Configuration-Identity Indicator
**Objective:** As a scheduler, I want every configuration push from the controller to include an indicator of the configuration's identity, so that I can tell an unchanged configuration apart from a genuinely new one without re-inspecting every individual setting.
#### Acceptance Criteria
1. When the Controller sends a configuration push to the Scheduler, the Controller shall include a configuration-identity indicator that represents the exact configuration content being sent.
2. While two configuration pushes carry byte-for-byte identical configuration content, the Controller shall present the same configuration-identity indicator for both pushes, regardless of which controller node sent them.
3. While two configuration pushes carry different configuration content, the Controller shall present different configuration-identity indicators for the two pushes.
4. The Controller shall derive the configuration-identity indicator from its own existing, already-replicated notion of configuration identity, not from a separately maintained value.
### Requirement 2: Skip Redundant Re-Initialization for an Unchanged Configuration
**Objective:** As an operator monitoring scheduler logs, I want the scheduler to recognize when a reconnect delivers a configuration it already has, so that routine HA failover reconnects do not produce misleading error log entries or repeat unnecessary work.
#### Acceptance Criteria
1. When the Scheduler receives a configuration push whose configuration-identity indicator matches the indicator of the configuration it currently holds, the Scheduler shall skip re-registering its resources, shared resource values, and queues from that push.
2. When the Scheduler skips re-registration under this condition, the Scheduler shall not emit an error-level log entry for any resource, shared resource value, or queue from that push.
3. While the Scheduler skips re-registration under this condition, the Scheduler shall retain the resource, shared resource value, and queue state it already holds unchanged.
4. When the Scheduler skips re-registration under this condition, the Scheduler shall record a diagnostic-level log entry noting that the pushed configuration was recognized as unchanged, so the behavior remains observable to someone investigating scheduler startup or reconnect behavior without being visible at normal operating log levels.
### Requirement 3: Fully Apply a Genuinely Changed Configuration
**Objective:** As an operator who updates the cluster configuration, I want the scheduler to always fully apply a configuration that has actually changed, so that skipping redundant work on reconnect never causes a real configuration change to be silently ignored.
#### Acceptance Criteria
1. When the Scheduler receives a configuration push whose configuration-identity indicator differs from the indicator of the configuration it currently holds, the Scheduler shall fully register the resources, shared resource values, and queues from that push, the same as it does today.
2. When the Scheduler completes registering a configuration push under this condition, the Scheduler shall adopt the newly pushed configuration's identity indicator as the configuration it currently holds.
3. When the Scheduler receives its first configuration push since starting, the Scheduler shall treat that push as a changed configuration and fully register it, regardless of any indicator value it carries.
4. If a configuration push carries a configuration-identity indicator that signals the controller's configuration is not yet established, the Scheduler shall treat that push as a changed configuration and fully register it rather than skipping registration.
### Requirement 4: No Regression to Explicit Configuration Update Operations
**Objective:** As an operator, I want administrator-initiated resource and queue update operations to keep working exactly as they do today, so that this feature's reconnect-skip behavior does not interfere with intentional, explicit configuration changes.
#### Acceptance Criteria
1. The Scheduler shall apply an administrator-initiated resource or queue update operation the same way it does today, independent of any configuration-identity indicator comparison introduced by this feature.
2. While an administrator-initiated resource or queue update changes a resource or queue that was previously registered via a configuration push recognized as unchanged, the Scheduler shall apply that update without requiring a subsequent configuration push to take effect.

View file

@ -0,0 +1,73 @@
# Research & Design Decisions
## Summary
- **Feature**: `scheduler-config-version-tag`
- **Discovery Scope**: Extension (existing controller<->scheduler wire protocol and existing HA config-replication mechanism)
- **Key Findings**:
- The controller already computes and replicates a content-derived config identity (`config_version_tag`, first 8 bytes of the config file's SHA-256) that is stable across HA failover/leadership changes — this feature reuses it rather than inventing a new versioning scheme.
- There are **two independent, structurally-mirrored code paths** that apply a pushed config to the live scheduler core, both exhibiting the same unconditional-re-registration behavior and needing the same fix.
- `xce_conf4sched_t`'s generated C fields keep their capnp camelCase names as-is (no snake_case mapname), confirmed against `CtrlStatus.configVersion` (`controller/ctrlstatus.c:119`, referenced directly as `st->configVersion` in C).
- A memory-ownership responsibility is newly introduced by the "skip" branch: the incoming, now-redundant `xce_conf4sched_t` payload must be explicitly freed (`free_Conf4Sched_ptr()`) rather than silently dropped, because both existing call sites currently treat "config push handled" as unconditional ownership transfer.
## Research Log
### Which code path(s) actually apply a config push to the live scheduler
- **Context**: bug-1727/1728/1729 (session-sibling findings, `.wolf/buglog.json`) diagnosed `sched_init_resources()` (`scheduler/src/scheduler.c:828`) as the ERROR-spam source, reached via the scheduler's join/reconnect handshake. Before designing a fix, confirmed whether this is the only live application path.
- **Sources Consulted**: `scheduler/src/handler.c`, `scheduler/src/scheduler.c`, `scheduler/src/core_events.c`, `scheduler/src/driver.c`, `scheduler/src/main.c`-adjacent comments, `scheduler/CLAUDE.md`'s architecture description.
- **Findings**:
- **Path A (join/reconnect — the reported bug's trigger)**: `handler.c`'s `scheduler_req_handler()`, on `Request_body_configpushmsg`, calls `sched_init_config()` (`scheduler.c:934`) synchronously, which unconditionally calls `sched_init_resources()`/`sched_init_sharedInst()`/`sched_init_queues()`. The controller sends `ConfigPushMsg` on every scheduler (re)join, including every HA-failover reconnect — matching the observed ERROR bursts correlated with `xcecli failover: attempt N` log lines.
- **Path B (live config upgrade while already connected)**: an admin-initiated `XCE_SCHED_OP_UPDATE_CONFIG` request is queued as a pend_update, later drained and translated by the normal-mode driver (`driver.c:202`, `driver.c:555`) into an `XCE_CORE_EVENT_CONFIG_PUSH` event applied via `xce_core_push_events()` → `core_events.c`'s `case XCE_CORE_EVENT_CONFIG_PUSH` (`core_events.c:702-717`), which unconditionally calls `core_apply_config_resources()`/`core_apply_config_resvals()`/`core_apply_config_queues()` — a structural mirror of Path A's trio, confirmed by `core_events.c`'s own header comment stating it mirrors `scheduler.c`'s functions "exactly."
- Both trios ultimately read/write the same `xce_sched_core_t` (`sched->core` in Path A is the same struct type `core` in Path B) — `resmgr`/`sharedInst`/`queues` are core-level state, not duplicated per path.
- **Implications**: the fix must live at a shared, core-level seam so both paths stay consistent (Path B fixed for free, not left to silently diverge from Path A) rather than duplicating comparison logic once per path. A single new field on `xce_sched_core_t` plus one shared comparison function, called from both `sched_init_config()` and `core_events.c`'s CONFIG_PUSH case, satisfies this.
### Is `config_version_tag` safe to trust as a reconnect-stability signal
- **Context**: Requirement 1.2 depends on the controller's config-identity indicator being identical across nodes for identical content and unaffected by leadership changes.
- **Sources Consulted**: `controller/config_repl.c` (both `config_version_tag` assignment sites), `controller/ha.c` (grepped for any assignment — none found).
- **Findings**: `config_version_tag = be64toh(first 8 bytes of SHA256(config file bytes))`, set only in `config_repl.c:686` (primary config load) and `config_repl.c:906` (post-replication commit of a new config version) — both are config-load/config-replication events. No assignment exists anywhere in `ha.c` (election/promotion/heartbeat code), confirming it does not change on failover/leadership transitions absent an actual config change. `ha.c:8733`'s own comment independently confirms `config_version_tag == 0` is already an established, meaningful sentinel ("a config-less node ... must NEVER start ...") elsewhere in this codebase — reused here for Requirement 3.4 rather than inventing a new sentinel convention.
- **Implications**: safe to treat as an equality oracle for "did the config actually change." No new coordination/consistency mechanism needed — this feature is purely a consumer of an existing, already-trusted value.
### Ownership of the incoming config payload on a skipped push
- **Context**: today, both Path A (`handler.c`, unconditional `request->body.configpushmsg->data.conf4sched = NULL;` after any `rc>=0`) and Path B assume the callee took ownership of the pushed `xce_conf4sched_t*` once "handled," regardless of whether it was actually adopted as the live config.
- **Sources Consulted**: `scheduler/src/handler.c:687-692`, `lib/controller.capnp.h`/`.c` (`free_Conf4Sched_ptr` existence, already used at `driver.c:114` and `scheduler.c:406` for an unrelated discard path).
- **Findings**: `free_Conf4Sched_ptr()` is a real, already-used deallocator for this exact type. When this feature's skip branch chooses not to adopt the incoming payload as `core->config`, that payload has no other owner and must be freed there, or every skipped reconnect leaks one `xce_conf4sched_t` (plus its nested resource/queue/resval/group arrays).
- **Implications**: the skip branch's owning component (see design's Components section) is explicitly responsible for calling `free_Conf4Sched_ptr()` on the incoming payload — captured as a Data Ownership decision in `design.md`, not left implicit.
## Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| Shared core-level comparison function | One `xce_sched_core_t`-level field + one pure comparison function, called from both Path A and Path B | Single source of truth; both paths stay consistent by construction; minimal surface area | None significant | **Selected** |
| Per-path duplicated comparison | Inline the same check independently in `scheduler.c` and `core_events.c` | Slightly less coupling between files | Duplicates logic the codebase already flags as "mirrors ... exactly"; two places to keep in sync | Rejected — repeats the exact maintenance hazard the existing mirrored trios already represent |
| Field-by-field resource/queue diffing (no version tag) | Compare each resource/queue's individual fields on `EEXIST` instead of a whole-config identity check | Doesn't require a schema change | Only fixes resources, not queues/resvals/groups; re-does the same comparison work on every single reconnect instead of one O(1) check; doesn't reuse an already-trusted value | Rejected in favor of the user-directed version-tag approach (see conversation record) — kept here for traceability |
## Design Decisions
### Decision: Store the comparison state on `xce_sched_core_t`, not `xce_scheduler_t`
- **Context**: Path A's caller has an `xce_scheduler_t* sched` (which embeds `xce_sched_core_t core`); Path B's caller has a bare `xce_sched_core_t* core`. The new stored tag must be reachable from both.
- **Alternatives Considered**:
1. Store on `xce_scheduler_t` — unreachable from Path B without threading an extra pointer through `core_events.c`.
2. Store on `xce_sched_core_t` — reachable from both as `sched->core.config_version_tag` and `core->config_version_tag` (same field).
- **Selected Approach**: Option 2.
- **Rationale**: `xce_sched_core_t` is already the shared state both paths mutate (`resmgr`, `sharedInst`, `queues`, the existing self-incrementing `configver`); this field belongs alongside `configver` for the same reason.
- **Trade-offs**: None material.
- **Follow-up**: none.
### Decision: Zero as "no config applied yet" sentinel, no separate boolean flag
- **Context**: Requirement 3.3 (first push after start) and 3.4 (controller reports no established config) both must force a full apply.
- **Alternatives Considered**:
1. Add a separate `bool config_version_tag_valid` flag alongside the stored tag.
2. Reuse `0` as "unset"/"no config" on both sides of the comparison (matching `ha.c`'s existing `config_version_tag == 0` convention).
- **Selected Approach**: Option 2 — `xce_sched_core_t.config_version_tag` starts at `0` (already the case via existing zero-initialization of `xce_sched_core_t`); the comparison treats a match as "skip" only when the incoming tag is nonzero AND equal to the stored tag.
- **Rationale**: reuses an already-established codebase convention instead of introducing a second, redundant state variable; a SHA-256-derived tag colliding with `0` is not a practical concern (same assumption `ha.c` already relies on elsewhere).
- **Trade-offs**: none material.
- **Follow-up**: none.
## Risks & Mitigations
- **Risk**: skip branch leaks the incoming `xce_conf4sched_t` payload if the free responsibility is missed. — **Mitigation**: explicit Data Ownership decision in `design.md`; ASAN is already the project's default debug build (per `build-deployment.md`), so a leak here is caught by the existing test/build pipeline if a leak-detecting test exercises the skip path.
- **Risk**: a future change to `config_version_tag`'s computation (e.g., made leadership-dependent) would silently break this feature's core safety assumption. — **Mitigation**: captured as a Revalidation Trigger in `design.md`.
- **Risk**: Path B (live config upgrade) skipping an "unchanged" push could surprise an admin who explicitly re-ran an upgrade expecting a visible re-apply. — **Mitigation**: the resulting state is identical whether skipped or reapplied (same content); Requirement 2.4's diagnostic-level log entry keeps the behavior inspectable without being a behavior change an admin would notice functionally.
## References
- `.wolf/buglog.json` bug-1727, bug-1728, bug-1729 — session findings that motivated this feature.
- `controller/config_repl.c` — `config_version_tag` computation and replication (existing mechanism this feature reuses).
- `scheduler/CLAUDE.md` — Core/Drivers architecture description confirming Path A vs. Path B are genuinely separate, both-live code paths.

View file

@ -0,0 +1,22 @@
{
"feature_name": "scheduler-config-version-tag",
"created_at": "2026-08-28T10:41:46Z",
"updated_at": "2026-08-28T11:20:00Z",
"language": "en",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": true,
"approved": true
},
"design": {
"generated": true,
"approved": true
},
"tasks": {
"generated": true,
"approved": true
}
},
"ready_for_implementation": true
}

View file

@ -0,0 +1,50 @@
# Implementation Plan
- [ ] 1. Add the configuration-identity wire field and the scheduler-core skip/apply decision
- Write failing unit tests first (TDD red) in a new `test_core_config_version.c`: matching nonzero identity values on both sides yields "unchanged"; differing nonzero values yields "changed"; an incoming value of zero yields "changed" regardless of what is stored; a freshly-initialized core (stored value still zero) yields "changed" regardless of the incoming value; a `NULL` incoming configuration yields "changed".
- Add the new configuration-identity field to the config-push wire message (`schema/controller.capnp`'s `Conf4Sched`) and let the existing build regenerate its bindings — no other field on that message changes shape.
- Add a zero-initialized configuration-identity field to the scheduler's core state (`xce_sched_core_t`), alongside its existing self-incrementing local config-version counter.
- Implement the pure comparison function against the five cases above; it must not mutate either input.
- Observable completion: the new test suite goes from failing to passing, and the comparison function returns "changed" for every input combination except two identical nonzero values.
- _Requirements: 1.1, 1.4, 3.3, 3.4_
- [ ] 2. Core: populate the identity on push, and skip redundant re-registration on both existing application paths
- [ ] 2.1 (P) Populate the configuration-identity field on every controller-side configuration push
- Extend the existing controller-side config-push test coverage (`controller/testsuite/test_job_array_sched_push.c`) with a failing case first (TDD red) asserting the pushed configuration carries the controller's own current configuration-identity value.
- Populate the field from the controller's existing, already-replicated configuration-identity value — do not compute or derive a new one.
- Observable completion: the new test goes from failing to passing, and every configuration push's wire payload carries a value equal to the controller's current configuration identity at push time.
- _Requirements: 1.1, 1.4_
- _Depends: 1_
- _Boundary: Controller configuration push (xcesched_push_configs)_
- [ ] 2.2 (P) Skip redundant re-registration on an unchanged join/reconnect configuration push
- Extend the existing join/reconnect configuration-application test coverage (`scheduler/testsuite/test_handlers.c`) with failing cases first (TDD red): a push whose identity matches the currently-held configuration registers no resources, shared resource values, or queues, and emits no error-level log entry; a push whose identity differs is fully registered exactly as today and the scheduler adopts the new identity afterward; the scheduler's first-ever push is always fully registered regardless of the identity value it carries.
- Implement the skip/apply branching at the join/reconnect configuration-application call site using the comparison function from 1: on "unchanged," skip resource/shared-value/queue registration entirely, free the now-redundant incoming configuration payload (it is not adopted), and record one diagnostic-level log entry noting the skip; on "changed," keep today's full registration behavior and additionally adopt the pushed configuration's identity value as currently held.
- Observable completion: the extended test suite goes from failing to passing, and a two-push sequence with identical content produces zero error-level log entries and zero resource/queue registration calls on the second push, with no leaked configuration payload under the project's memory-safety build.
- _Requirements: 2.1, 2.2, 2.3, 2.4, 3.1, 3.2, 3.3, 3.4_
- _Depends: 1_
- _Boundary: Scheduler join/reconnect configuration application (Path A)_
- [ ] 2.3 (P) Skip redundant re-registration on an unchanged live configuration-upgrade push
- Extend the existing live configuration-upgrade test coverage (`scheduler/testsuite/test_core_events.c`) with the same failing cases as 2.2 (unchanged skips with no error log, changed fully applies and adopts the new identity, first-ever push always applies), exercised through this path's own event-application entry point instead of the join/reconnect one.
- Implement the identical skip/apply branching at this second configuration-application call site, using the same comparison function from 1 and the same core-level stored identity field as 2.2 — both paths must observe and update the same value.
- Observable completion: the extended test suite goes from failing to passing, and a two-push sequence with identical content produces zero error-level log entries and zero resource/queue registration calls on the second push, with no leaked configuration payload under the project's memory-safety build.
- _Requirements: 2.1, 2.2, 2.3, 2.4, 3.1, 3.2, 3.3, 3.4_
- _Depends: 1_
- _Boundary: Scheduler live configuration-upgrade application (Path B)_
- [ ] 3. Validation: prove the end-to-end fix and confirm no regressions
- [ ] 3.1 Verify the full skip/apply lifecycle end to end and confirm administrator update operations are unaffected
- Write an integration test driving a scheduler instance through a first configuration push (full registration observed), a second, identical push (skip observed: no registration calls, no error-level log entries), and a third push with genuinely different content (full registration observed again, including the previously-skipped resources).
- Run the existing explicit administrator resource/queue update operation tests unmodified and confirm they still pass exactly as today, independent of any configuration-identity comparison.
- Observable completion: the integration test passes end to end without any error-level log entry appearing for the identical second push, and the full existing administrator-update test suite remains green with no changes to its own assertions.
- _Requirements: 2.1, 2.2, 2.3, 2.4, 3.1, 3.2, 4.1, 4.2_
- _Depends: 2.1, 2.2, 2.3_
- [ ] 3.2 Close the originally-reported symptom on a live 3-node HA cluster
- Run a live HA soak that triggers at least one failover and confirms the reconnecting scheduler's log contains zero error-level resource/queue-registration entries for that reconnect.
- During the same soak, push a genuinely changed configuration and confirm it is still fully and correctly observed by the scheduler (for example, via its existing resource/worker inspection interface), proving the skip behavior never suppresses a real change.
- Confirm the configuration-identity value observed after a failover to a different, newly-primary controller node matches what the scheduler already held when the underlying configuration content is unchanged — the direct live proof that the identity value is stable across which node is primary.
- Observable completion: the soak log shows zero false error-level entries across every reconnect, and the mid-soak configuration change is confirmed applied.
- _Requirements: 1.2, 1.3, 2.1, 2.2, 2.4, 3.1_
- _Depends: 3.1_