fix(apiserver): auto-recover after full controller death+restart (task #12)

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 <noreply@anthropic.com>

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.

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)) {

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)
 

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*);

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;