From e505066162a40a29970281632ba38fc35c6c29d5 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Fri, 14 Aug 2026 08:01:35 +0000 Subject: [PATCH] fix(apiserver): auto-recover after full controller death+restart (task #12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The apiserver stayed circuit-breaker-degraded after a full controller restart until manually restarted. Two root causes: 1. Breaker recovery was decoupled from the breaker itself. The breaker can be tripped by a backend write failure (e.g. E_XCE_STANDBY -10014 while the controller is electing) with no heartbeat ever failing, but StartHeartbeat only recovered on a fail→success heartbeat transition (prevFailed), so the breaker stayed degraded forever. → extract needsRejoinAfterHeartbeat(hbOK, prevFailed): recover when prevFailed OR the breaker is currently tripped. 5 new unit tests. 2. The 30s heartbeat's fire-and-forget reconnect only fires when the lib's internal ctx state is XCE_CTX_ERR. After a controller restart whose election outlasts the FAILOVER_CALL retry budget, the retry loop's xcecli_ctx_recover leaves the ctx RUN on a half-open connection, so the FF path never reconnects and the client stays disconnected forever. → lib: new xcecli_force_reconnect() forces a rotate+recover regardless of state; apiserver: 2s recoveryLoop force-reconnects + rejoins while degraded. Verified on cluster: after controller kill+restart, the apiserver logs "backend re-joined controller after reconnect" and job submissions resume without a manual restart; a fresh job runs to DONE. Controller, scheduler (rejoin ok, bug-1513) and workers all auto-recover. Co-Authored-By: Claude --- docs/left_scheduler_work.md | 4 +- src/frontend/apiserver/backend/clib.go | 49 +++++++++-- src/frontend/apiserver/backend/heartbeat.go | 17 ++++ .../apiserver/backend/heartbeat_test.go | 81 +++++++++++++++++++ src/include/client.h | 1 + src/lib/client.c | 26 ++++++ 6 files changed, 171 insertions(+), 7 deletions(-) diff --git a/docs/left_scheduler_work.md b/docs/left_scheduler_work.md index 8c791d0b..d02143aa 100644 --- a/docs/left_scheduler_work.md +++ b/docs/left_scheduler_work.md @@ -93,7 +93,9 @@ spec 声称 36/36 任务完成，但 re-audit（research.md）发现缺陷。来 - **worker 无限重试**（controller_connector.c `checking_connection`）：去掉 5 次上限，无限重试直到重连（连接器线程专用，不阻塞硬件/进程监控线程）。实测：杀 controller 后 3 worker 全部存活并持续重试，controller 回来后自动 rejoin。**永久故障**：持续重试不放弃（正确）；**HA**：`xcecd_join_cluster` 已按 --peers 候选列表迭代（best-effort）。 - **apiserver rejoin**（clib.go `StartHeartbeat` + `xce_backend_rejoin`）：心跳从失败转成功时显式 rejoin（HostRole_apiserver），恢复 push 通知。实测：controller 重启后日志 `backend re-joined controller after reconnect`。 - **apiserver fail-fast 电路熔断（Req 4，commit 2bbe894）**：health.go 原子熔断 + `callBackend` 包 4 个写调用。degraded 时立即返回 `backend controller unavailable`，不调 lib；传输错误或 XCE NOMASTER/REDIRECT/NOT_READY/STANDBY 触发熔断；语义错误不触发。实测：杀 controller 后第 2 次提交 ~11ms 返回 fail-fast。7 单测。 - - **⚠️ 已知缺口**：**apiserver 心跳恢复在 controller 完整死亡+重启后不总是可靠**——lib transport reconnect 未稳定重建到新 controller（与 scheduler reconnect 同类，task #13）。当前需手动重启 apiserver 恢复（clean rejoin 正常）。 + - **✅ apiserver 自动恢复已修复（2026-08-14）**——两个根因： + 1. **熔断恢复与心跳解耦**：breaker 可被 backend 写失败（如选举期 E_XCE_STANDBY -10014）触发，但 `StartHeartbeat` 只按「心跳失败→成功」转移恢复，`prevFailed` 为 false 时永不恢复 → 一直 degraded 直到手动重启。修复：`needsRejoinAfterHeartbeat` 在 `prevFailed.Swap(false) || IsDegraded()` 时恢复（heartbeat.go + 5 个单测）。 + 2. **transport reconnect 靠 lib 状态启发式不可靠**：30s 心跳的 fire-and-forget 只在 `ctx->state==ERR` 时重连；controller 重启的选举期超出 FAILOVER_CALL 重试预算后，ctx 停在 RUN 的半开连接，永不重连。修复：lib 新增 `xcecli_force_reconnect`（无视状态强制 rotate+recover），apiserver 加 2s `recoveryLoop`（degraded 时强制重连+rejoin）。实测：controller 完整死亡+重启后 apiserver 自动 `backend re-joined controller after reconnect`，无需手动重启，提交恢复。 - **✅ scheduler reconnect 已修复（2026-08-14 根因定位 + 集群验证）**： - **关键结论**：所谓 "libxce reconnect heap corruption" **不是 lib 竞态**——是 scheduler rejoin 状态重同步的 3 个真实 bug（均在 config/dataset 重推路径）。 - **根因 1（group.c:234/247）**：`makeGroup` 刷新已有 group 的成员时 `FREEUP(m->u.name)` **raw-free 了共享 interned xstring**（`make_g_member` 用 `makeString` 建的是全局引用计数共享串，多个 group 共享同一指针）。正确释放是 `dereference()`（`group_killer` 就这么做）。ASAN 报 `bad-free`/`free(): invalid pointer` 即源于此。 diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 1156e8ff..4ec8ddd5 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -45,6 +45,16 @@ static int xce_backend_rejoin(backend_ctx_t *ctx) { return xcecli_join(ctx->ctx, HostRole_apiserver, NULL); } +// xce_backend_force_reconnect force-reconnects the transport regardless of the +// lib's internal state (see xcecli_force_reconnect). Used by the Go recovery +// loop when the circuit breaker is degraded: the 30s heartbeat's fire-and-forget +// reconnect only fires when ctx->state == XCE_CTX_ERR, and after a controller +// restart whose election outlasts the FAILOVER_CALL retry budget the ctx can +// stay RUN on a half-open connection, never reconnecting on its own. +static int xce_backend_force_reconnect(backend_ctx_t *ctx) { + return xcecli_force_reconnect(ctx->ctx); +} + // Helpers to extract typed row pointers from a dataset_t. static xce_job_t **dataset_jobs(dataset_t *d) { return (xce_job_t **)d->rows; } static xce_worker_t **dataset_workers(dataset_t *d) { return (xce_worker_t **)d->rows; } @@ -70,6 +80,7 @@ import ( "fmt" "sync" "sync/atomic" + "time" "unsafe" "go.uber.org/zap" @@ -222,16 +233,42 @@ func StartHeartbeat(ctx context.Context) { var prevFailed atomic.Bool go runHeartbeat(ctx, func(secs int) { rc := C.xcecli_heartbeat(gCtx.ctx, C.int(secs)) - if rc < 0 { - prevFailed.Store(true) - MarkDegraded() - return - } - if prevFailed.Swap(false) { + if needsRejoinAfterHeartbeat(rc >= 0, &prevFailed) { rejoinBackend() MarkHealthy() } }) + go recoveryLoop(ctx) +} + +// recoveryLoop retries a forced transport reconnect + rejoin on a short +// interval while the circuit breaker is degraded (bug-1513 follow-up). The +// 30s heartbeat's fire-and-forget reconnect only fires when the lib's internal +// state is ERR; after a controller restart whose election outlasts the +// FAILOVER_CALL retry budget the ctx can stay RUN on a half-open connection and +// the heartbeat never reconnects — the backend stays degraded until a manual +// restart. This loop force-reconnects deterministically and re-joins, so a +// full controller death+restart recovers on its own. +func recoveryLoop(ctx context.Context) { + t := time.NewTicker(2 * time.Second) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + if !IsDegraded() { + continue + } + rc := C.xce_backend_force_reconnect(&gCtx) + if rc < 0 { + // Still unreachable (controller electing/booting) — retry next tick. + continue + } + rejoinBackend() + MarkHealthy() + } + } } // go_push_handler is called by the C library on every incoming request. diff --git a/src/frontend/apiserver/backend/heartbeat.go b/src/frontend/apiserver/backend/heartbeat.go index fff98c78..d1409364 100644 --- a/src/frontend/apiserver/backend/heartbeat.go +++ b/src/frontend/apiserver/backend/heartbeat.go @@ -18,6 +18,23 @@ func SetHeartbeatInterval(secs int) { } } +// needsRejoinAfterHeartbeat reports whether one heartbeat result should +// trigger a backend rejoin + breaker clear. A failed heartbeat marks the +// backend degraded and returns false; a successful heartbeat returns true +// when EITHER a prior heartbeat failed (a transport reconnect just completed) +// OR the circuit breaker is currently tripped. The breaker can be tripped by a +// backend write failure (e.g. E_XCE_STANDBY while the controller is electing) +// without any heartbeat ever failing — tracking only the heartbeat transition +// would leave the breaker stuck degraded forever (bug-1513 follow-up). +func needsRejoinAfterHeartbeat(hbOK bool, prevFailed *atomic.Bool) bool { + if !hbOK { + prevFailed.Store(true) + MarkDegraded() + return false + } + return prevFailed.Swap(false) || IsDegraded() +} + // runHeartbeat loops until ctx is cancelled, calling fn(intervalSecs) on each tick. // Extracted from clib.go for testability without CGO. func runHeartbeat(ctx context.Context, fn func(int)) { diff --git a/src/frontend/apiserver/backend/heartbeat_test.go b/src/frontend/apiserver/backend/heartbeat_test.go index 0924d3fd..7a28b1cb 100644 --- a/src/frontend/apiserver/backend/heartbeat_test.go +++ b/src/frontend/apiserver/backend/heartbeat_test.go @@ -31,6 +31,87 @@ func TestSetHeartbeatInterval_NegativeIgnored(t *testing.T) { } } +func TestNeedsRejoinAfterHeartbeat_FailureMarksDegraded(t *testing.T) { + var prev atomic.Bool + resetBreaker() + + got := needsRejoinAfterHeartbeat(false, &prev) + if got { + t.Error("failed heartbeat should not request rejoin") + } + if !IsDegraded() { + t.Error("failed heartbeat should trip the circuit breaker") + } + if !prev.Load() { + t.Error("failed heartbeat should record the failure for the next transition") + } +} + +func TestNeedsRejoinAfterHeartbeat_HealthySuccessNoOp(t *testing.T) { + var prev atomic.Bool + resetBreaker() + + got := needsRejoinAfterHeartbeat(true, &prev) + if got { + t.Error("successful heartbeat with healthy breaker and no prior failure should not rejoin") + } +} + +func TestNeedsRejoinAfterHeartbeat_TransportRecoveryRejoins(t *testing.T) { + var prev atomic.Bool + resetBreaker() + + needsRejoinAfterHeartbeat(false, &prev) // transport failed + if !IsDegraded() { + t.Fatal("precondition: breaker should be tripped") + } + + got := needsRejoinAfterHeartbeat(true, &prev) // transport recovered + if !got { + t.Error("successful heartbeat after a failure should request rejoin") + } + // The caller (StartHeartbeat) clears the breaker when the helper says rejoin. + MarkHealthy() + if IsDegraded() { + t.Error("caller must clear the breaker after the recovery transition") + } +} + +// bug-1513 follow-up: the breaker can be tripped by a backend write failure +// (e.g. E_XCE_STANDBY while the controller is electing) with no heartbeat ever +// failing. A successful heartbeat must still clear it, or the backend stays +// degraded forever until a manual apiserver restart. +func TestNeedsRejoinAfterHeartbeat_BreakerTrippedNoHeartbeatFailure(t *testing.T) { + var prev atomic.Bool + resetBreaker() + + MarkDegraded() // tripped by a backend write failure, not by a heartbeat + + got := needsRejoinAfterHeartbeat(true, &prev) + if !got { + t.Error("successful heartbeat must clear a breaker tripped by a backend write failure") + } +} + +func TestNeedsRejoinAfterHeartbeat_RecoveryResetsPrevFailed(t *testing.T) { + var prev atomic.Bool + resetBreaker() + + if needsRejoinAfterHeartbeat(true, &prev) { + MarkHealthy() // simulate the caller clearing the breaker + } + + if prev.Load() { + t.Error("prevFailed should be reset after the success transition") + } + + // Next success with a healthy breaker must be a no-op (no repeated rejoin). + got := needsRejoinAfterHeartbeat(true, &prev) + if got { + t.Error("second consecutive successful heartbeat should not rejoin again") + } +} + func TestRunHeartbeat_CallsFunction(t *testing.T) { atomic.StoreInt32(&heartbeatIntervalSecs, 1) diff --git a/src/include/client.h b/src/include/client.h index 1de3ac56..d2e5bc8d 100644 --- a/src/include/client.h +++ b/src/include/client.h @@ -48,6 +48,7 @@ extern int xcecli_wakeup(xcecli_ctx_t, int); extern int xcecli_hello(xcecli_ctx_t, char*); extern int xcecli_nop(xcecli_ctx_t); extern int xcecli_heartbeat(xcecli_ctx_t, int); +extern int xcecli_force_reconnect(xcecli_ctx_t); extern int xcecli_shutdown(xcecli_ctx_t); extern int xcecli_reslist(xcecli_ctx_t, xce_resource_t***); extern int xcecli_join(xcecli_ctx_t, int, hostattrs_t*); diff --git a/src/lib/client.c b/src/lib/client.c index c0b82236..1eb6205b 100644 --- a/src/lib/client.c +++ b/src/lib/client.c @@ -264,6 +264,32 @@ int xcecli_heartbeat(xcecli_ctx_t c, int hb_interval) return XCECLI_FF_CALL(ctx, xce_heartbeat(&(ctx->ctx), hb_interval)); } +int xcecli_force_reconnect(xcecli_ctx_t c) +{ + api_ctx_t* ac = (api_ctx_t*)c; + + if (ac == NULL) { + return -EINVAL; + } + + /* Force a transport reconnect + state reset REGARDLESS of the lib's + * internal state, so a degraded client can recover even when the + * fire-and-forget heartbeat's state==ERR heuristic never fires. That + * happens after a controller restart whose election outlasts a FAILOVER_CALL + * retry budget: the retry loop's xcecli_ctx_recover leaves the ctx RUN on a + * half-open connection, so the FF path never reconnects and the client + * stays disconnected forever. Used by the apiserver's recovery loop + * (bug-1513 follow-up). */ + mylock_lock(&ac->failover.lock); + int rc = xce_failover_rotate(ac); + if (rc >= 0) { + xcecli_ctx_recover(ac); + } + mylock_unlock(&ac->failover.lock); + + return rc; +} + int xcecli_shutdown(xcecli_ctx_t c) { api_ctx_t* ctx = (api_ctx_t*)c;