diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 04840871..54636ea8 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -19162,7 +19162,7 @@ "error_message": "User-reported: `xceadmin scheduler record status` (and by the same mechanism, `record start`/`record stop`) fails with `Error: controller returned status -10014` whenever the `--controller`/default address happens to point at a controller that is currently a STANDBY, not the primary. User's expectation: 'xceadmin/libxce should find the correct primary to talk to it' instead of surfacing the raw rejection.", "file": "src/controller/cluster.c (myreq_handler, ~line 406-414)", "root_cause": "FULLY ROOT-CAUSED, gap is server-side only. `-10014` is `E_XCE_STANDBY` (errnox.h, XCE_ERRNO_BASE+13) -- ha_dispatch_gate() (ha.c:4745-4775) correctly rejects OPCode_schedAdmin on a CTRL_STANDBY node (it is absent from that role's opcode allow-list, by design -- schedAdmin is a primary-only forward-to-scheduler operation). The gate's generic caller in myreq_handler (cluster.c:406-414) sends the rejection reply with `reply.status = rc; reply.data_which = RequestReply_data_nodata;` and NEVER calls fill_redirect_reply() (join.c:27-31, the exact same one-line helper OPCode_join's handler already uses successfully) -- so redirectHost/redirectPort are left at their zero-value defaults on EVERY dispatch-gate rejection, for EVERY opcode, not just schedAdmin (OPCode_upgradeConfig has the identical, already-documented gap -- see test_e2e_live_ha_push.py's module docstring, confirmed 2026-07-30-era). Critically, this is NOT a client-side deficiency: xceadmin's redirect-following client logic is ALREADY fully implemented, generic/opcode-agnostic, and well-tested (`xceadmin.discovery.follow_redirects()`, with a full test suite covering 1-hop/3-hop/too-many-hops/timeout cases; `xceadmin.scheduler._send_admin_request()` already calls it for every record_start/stop/status call). `_redirect_target()` (discovery.py:80-89) correctly requires BOTH redirectHost and redirectPort to be non-empty/non-zero to treat a reply as a redirect -- given the gate-rejection reply supplies neither, the client correctly concludes there is nothing to follow and surfaces the bare rejection status as-is. The client is not 'giving up' -- it is working exactly per its documented contract given the reply it receives; the whole path already works end-to-end against a properly-behaving server (test_cli_scheduler_record_start_follows_redirect_to_primary already exercises the client side against a mock that DOES send a redirect).", - "fix": "PENDING. Minimal, single-call-site fix: in myreq_handler's generic ha_dispatch_gate rejection path (cluster.c:406-414), when the rejection reason is specifically -XCE_ERR_STANDBY (i.e. this node knows or may know who the primary is) call fill_redirect_reply(&reply, &c->ha) before xce_reply(), exactly mirroring OPCode_join's existing handle_worker_join()-adjacent usage (join.c:277). This fixes OPCode_schedAdmin AND the already-documented OPCode_upgradeConfig gap simultaneously, with ZERO xceadmin/client-side changes needed -- the existing follow_redirects() mechanism will start working immediately once the server populates the fields. Must verify: (a) ha_state_t.primary_host/primary_port are populated/valid on a CTRL_STANDBY node that has learned of a live primary (the common case) vs. a node that genuinely does not know any primary yet (mid-election, or has never heard from one) -- fill_redirect_reply() unconditionally aliases these fields with no NULL/empty guard, so confirm what a standby's primary_host looks like before any primary is ever discovered (likely NULL/empty, in which case the reply's redirectHost would encode as empty/absent and the client's existing `if host and port` guard already handles this correctly as 'no redirect' -- but verify, don't assume); (b) add a regression test proving a schedAdmin request to a standby-with-known-primary now receives a populated redirect reply (mirroring test_join_redirect.c's existing pattern, `test_redirect_reply_carries_primary_address`) and a live xceadmin CLI test (extending test_e2e_live_ha_push.py's pattern) proving `xceadmin scheduler record status` against ANY of the 3 nodes now transparently reaches the primary and succeeds.", + "fix": "PARTIALLY FIXED (2026-08-26), live verification FAILS -- see bug-1701 for the blocking root cause, do not close this bug until that is resolved. IMPLEMENTED, unit-tested, and CORRECT exactly as scoped: myreq_handler's generic ha_dispatch_gate rejection path (cluster.c ~410-433) now calls fill_redirect_reply(&reply, &c->ha) when, and only when, rc == -XCE_ERR_STANDBY -- confirmed via ha_dispatch_gate's full body (ha.c:4573-4781) that -XCE_ERR_STANDBY (CTRL_STANDBY's default-case rejection, ha.c:4774) and -XCE_ERR_NOT_READY (CTRL_INIT/CTRL_ELECTING, and CTRL_PRIMARY's config-serve-fence / upgrade-materialization-fence / no-write-quorum branches) are the ONLY two nonzero codes the gate can return -- no third code path exists. -XCE_ERR_NOT_READY is deliberately excluded (no settled primary to redirect to in those states). RED/GREEN proof: added test_schedadmin_standby_with_known_primary_redirects (FAILED pre-fix with 'Expected 10.0.1.7 Was ''', PASSED post-fix) and test_schedadmin_standby_no_known_primary_no_redirect (regression companion, passes in both states) to test_sched_admin_dispatch.c, driving the real myreq_handler via myreq_handler_for_test with OPCode_schedAdmin + CTRL_STANDBY role. Full controller_unit_test: 1200 Tests, 2 Failures both times (pre- and post-fix) -- exactly the pre-existing bug-1561 overlayfs pair (test_conf_trim_keeps_only_n_versions, test_conf_get_saved_tag_no_config), zero new failures. Encode-path NULL-safety CONFIRMED (not assumed): ha_state_t.primary_host is a fixed char[XCE_MAXNAME_LEN] buffer embedded in ha_state_t (ha.h:161), never a pointer, so its address is never NULL; encode_RequestReply (controller.capnp.c:13777-13786) gates strictly on `s->redirect_host != NULL` and falls back to an explicit empty string/0 otherwise; decode_RequestReply (controller.capnp.c:13841) strdup()s the wire text unconditionally (so a client-side redirect_host is always a real, non-NULL, possibly-empty C string, never NULL) -- an unknown primary therefore safely round-trips as redirectHost='' / redirectPort=0, which xceadmin's discovery.py _redirect_target() (host and port both required truthy) already correctly treats as 'no redirect, use this reply as-is'. No NULL-pointer/crash risk anywhere in this path, confirmed by code inspection of every hop, not assumed. LIVE VERIFICATION FAILS, however: deployed a fresh 3-node cluster twice (deploy-ha.sh, controller+worker+scheduler via scripts/start-all.sh) -- once on a binary built from this fix (git stash of cluster.c to produce a clean before-binary, git stash pop to restore it for the after-binary; xce_test ctrlstatus confirmed ctrl-1 elected primary, ctrl-2/ctrl-3 standby, in both deploys). `xceadmin scheduler record status --controller localhost:9992` (a standby) against BOTH the pre-fix and the post-fix binary produced the IDENTICAL result: 'Error: controller returned status -10014', exit 1, no 'redirected: ...' line on stderr either time -- i.e. this fix has ZERO observable effect on the user-reported symptom in a real deployment. Root cause of the no-op: ha_state_t.primary_host/primary_port (ha.h:161-162) are NEVER WRITTEN by any production code path on a standby -- confirmed by grepping every .c file in src/ for an assignment site (none exists outside test fixtures, which set it manually) and by ctrlstatus.c's own pre-existing doc comment (line 38/95: 'the vestigial, never-written ha->primary_host') -- so fill_redirect_reply() always sees an empty string / 0 on a real standby, in every state, permanently, not just during the 'hasn't learned yet' transient window this bug's own original root-cause writeup anticipated and asked to be verified. This is independently corroborated by the ALREADY-CLOSED bug-854 (this bug's own related_bugs entry), whose root_cause section states the identical fact ('primary_host is vestigial/never-written') and whose fix explicitly says 'Deliberately NOT adding server-side primaryName source -- it is only a latency optimization; the client rotation fix alone resolves the hang' for the DIFFERENT ctrlstatus/client-rotation code path. New implication surfaced by this session (see bug-1701): since fill_redirect_reply() is fed by the same permanently-empty field for EVERY caller, OPCode_join's pre-existing, supposedly-successful use of the exact same helper (join.c:277) is, by the same logic, equally non-functional against a live standby in production today -- this was not previously flagged anywhere in the bug log. Per this bug's own instructions, NOT papering over this with a workaround guessed in this session -- stopping for review. The gate-rejection code-path change above is correct, tested, and worth keeping (it is necessary infrastructure and fixes the OPCode_upgradeConfig-adjacent gap's code shape identically to join.c's precedent), but closing the user-visible bug additionally requires a second, larger, currently-unscoped change: something must actually populate primary_host/primary_port on a standby from live heartbeat/AppendEntries/ctrlhello traffic (touches ha.c, which has other work in flight this session was told not to touch), OR xceadmin needs a bug-854-style client-side master-ring rotation fallback for schedAdmin/upgradeConfig (mirroring xcecli_ctrlstatus_primary's fix) so an empty-redirect standby reply drives a retry against the next configured master instead of surfacing the bare rejection. See bug-1701 for the dedicated tracking of the underlying data-gap finding.\n\nOriginal PENDING fix text (superseded by the above, kept for history): PENDING. Minimal, single-call-site fix: in myreq_handler's generic ha_dispatch_gate rejection path (cluster.c:406-414), when the rejection reason is specifically -XCE_ERR_STANDBY (i.e. this node knows or may know who the primary is) call fill_redirect_reply(&reply, &c->ha) before xce_reply(), exactly mirroring OPCode_join's existing handle_worker_join()-adjacent usage (join.c:277). This fixes OPCode_schedAdmin AND the already-documented OPCode_upgradeConfig gap simultaneously, with ZERO xceadmin/client-side changes needed -- the existing follow_redirects() mechanism will start working immediately once the server populates the fields. Must verify: (a) ha_state_t.primary_host/primary_port are populated/valid on a CTRL_STANDBY node that has learned of a live primary (the common case) vs. a node that genuinely does not know any primary yet (mid-election, or has never heard from one) -- fill_redirect_reply() unconditionally aliases these fields with no NULL/empty guard, so confirm what a standby's primary_host looks like before any primary is ever discovered (likely NULL/empty, in which case the reply's redirectHost would encode as empty/absent and the client's existing `if host and port` guard already handles this correctly as 'no redirect' -- but verify, don't assume); (b) add a regression test proving a schedAdmin request to a standby-with-known-primary now receives a populated redirect reply (mirroring test_join_redirect.c's existing pattern, `test_redirect_reply_carries_primary_address`) and a live xceadmin CLI test (extending test_e2e_live_ha_push.py's pattern) proving `xceadmin scheduler record status` against ANY of the 3 nodes now transparently reaches the primary and succeeds.", "tags": [ "controller", "xceadmin", @@ -19175,14 +19175,48 @@ "root-caused", "fix-scoped", "low-risk", - "unfixed" + "partially-fixed", + "code-fix-landed", + "live-verification-failed", + "blocked-on-primary_host-vestigial", + "see-bug-1701" ], "related_bugs": [ "bug-1698", + "bug-854", + "bug-1701" + ], + "occurrences": 2, + "last_seen": "2026-08-26T00:00:00Z" + }, + { + "id": "bug-1701", + "timestamp": "2026-08-26T00:00:00Z", + "error_message": "Discovered while implementing/live-verifying bug-1700's fix (myreq_handler's generic dispatch-gate rejection now calls fill_redirect_reply() on -XCE_ERR_STANDBY, cluster.c ~410-433): the fix has ZERO effect in a real deployment. `xceadmin scheduler record status --controller ` against a live 3-node cluster still fails with 'Error: controller returned status -10014' identically before and after the fix, with no 'redirected: ...' hop ever printed.", + "file": "src/controller/ha.h:161-162 (ha_state_t.primary_host/primary_port), src/controller/join.c:27-31 (fill_redirect_reply), src/controller/ctrlstatus.c:38 (pre-existing doc comment already naming this)", + "root_cause": "ha_state_t.primary_host/primary_port are NEVER WRITTEN by any production code path on a standby -- confirmed by grepping every assignment site across the entire src/ tree (`grep -rn primary_host src/ --include=*.c --include=*.h`): the only writers are test fixtures (test_join_redirect.c, test_p1_integration.c, test_p3_hardening.c, test_ha_redirect.c, test_ha_role_lifecycle.c, this bug's own new test_sched_admin_dispatch.c tests) manually poking the field for isolated unit tests; no heartbeat/AppendEntries/ctrlhello handler in ha.c ever copies the leader's identity into it on the receiving standby. ctrlstatus.c's own field-sourcing doc comment (lines 37-38, 93-101) already names this explicitly ('the vestigial, never-written ha->primary_host') and documents that ctrlstatus_fill() deliberately derives primaryName from role + voted_for_name instead, NEVER from primary_host, for exactly this reason -- so the gap was known and worked around for ctrlstatus, but not for the redirect-reply mechanism (fill_redirect_reply/redirect_host/redirect_port), which has no equivalent workaround and is the ONLY consumer of this permanently-empty field. bug-854 (2026-06-21, CLOSED/FIXED) independently documented the identical fact while root-causing a related but distinct bug (positional client-side primary discovery for ctrlstatus) and explicitly chose NOT to add a server-side primaryName/primary_host source, fixing that bug entirely client-side (xcecli_ctrlstatus_primary master-ring rotation) instead -- a reasonable call for THAT bug, but it leaves fill_redirect_reply() as a piece of server-side machinery that looks functional (it has a real caller, a real unit test proving its own field-copying logic, and a real capnp wire encode/decode path) yet is fed a field that is permanently empty in every real deployment. Direct consequence: OPCode_join's existing use of fill_redirect_reply() (join.c:277, the very precedent bug-1700's fix was told to mirror) is, by the same reasoning, ALSO non-functional against a live standby in production today -- a worker/scheduler/controller join redirect to the primary silently never happens; nothing in the existing test suite (test_join_redirect.c) catches this because those tests call fill_redirect_reply() directly with a manually-populated ha_state_t, never through a live standby that only knows what production heartbeat/election code actually wrote into it.", + "fix": "NOT FIXED -- out of scope for the bug-1700 session that discovered it (bug-1700's task boundary was narrowly cluster.c's myreq_handler; this bug's real fix needs either (a) a new server-side write path in ha.c that captures the leader's host/port into ha->primary_host/primary_port on the standby whenever it processes a heartbeat/AppendEntries/ctrlhello from the current leader -- ha.c has unrelated work in flight this session was explicitly told not to touch, so this needs its own dedicated task/spec -- or (b) extending xceadmin's discovery/follow_redirects (or scheduler.py's _send_admin_request / a generic wrapper) with a bug-854-style client-side master-ring rotation fallback: on an empty-host/zero-port rejection reply (today's actual server behavior, unchanged), rotate to the next configured master instead of surfacing the bare status, mirroring xcecli_ctrlstatus_primary's already-proven fix for the ctrlstatus opcode. Needs its own root-cause-to-design cycle (/kiro-spec-quick or a dedicated debugging session) to pick between (a)/(b) or both; NOT a guess-and-patch fix within this session per the parent task's explicit instruction to stop and report rather than paper over this.", + "tags": [ + "controller", + "ha", + "primary-discovery", + "redirect", + "primary_host", + "vestigial-field", + "join.c", + "fill_redirect_reply", + "found-live", + "root-caused", + "not-fixed", + "needs-spec", + "cross-cutting" + ], + "related_bugs": [ + "bug-1700", "bug-854" ], "occurrences": 1, - "last_seen": "2026-08-26T16:35:00Z" + "last_seen": "2026-08-26T00:00:00Z" } ] -} \ No newline at end of file +}