Skip to content

feat(clm): add active-standby HA with leader election and reconciler (#1211) - #1363

Open
shsaihdsaiudh wants to merge 3 commits into
TencentCloud:masterfrom
shsaihdsaiudh:feat/clm-active-standby-1211
Open

feat(clm): add active-standby HA with leader election and reconciler (#1211)#1363
shsaihdsaiudh wants to merge 3 commits into
TencentCloud:masterfrom
shsaihdsaiudh:feat/clm-active-standby-1211

Conversation

@shsaihdsaiudh

Copy link
Copy Markdown
Contributor

Fixes #1211

What

Adds active-standby HA to cube-lifecycle-manager so a crashed replica no longer blocks the auto-pause/auto-resume workflow:

  • Leader election via Redis lease (cube:v1:shared:lock:lifecycle_manager:leader, SETNX + token-matched renew/release Lua, default TTL 15s). Only the leader runs the stateful loops: stream consumer, stale-pending claim, last-active poller, idle sweeper, and the periodic reconciler.
  • Standby behavior: keeps the HTTP server up and reports 503 on /readyz so the Service routes /internal/resume only to the leader; a standby can still serve resume via the meta-hash fallback on a registry miss.
  • Failover (bounded by one leader TTL): the new leader bootstraps from the meta hash, replays the snapshot to every CubeProxy, claims the dead consumer's pending stream entries via XAUTOCLAIM (min-idle = reconcile interval), and the reconciler converges remaining drift between the meta hash, the in-memory registry, and proxy meta dicts.
  • Hot-loop guard: after 3 consecutive fast-failing leader stints (each shorter than the leader TTL) the process exits so the pod supervisor restarts it; clean stints and stints surviving at least one TTL reset the counter.
  • Config safety: in HA mode CUBE_LCM_RECONCILE_INTERVAL must be >= CUBE_LCM_LEADER_TTL (validated at startup) — the interval doubles as the XAUTOCLAIM min-idle, and a smaller value could steal pending stream entries from a merely partitioned — still alive — old leader.

HA is opt-in via CUBE_LCM_HA_ENABLED (default off keeps single-replica behavior); the Helm chart enables it by default with replicas: 2. One-click and terraform deployments remain single-replica.

Upgradability note

HA deployments that set reconcileInterval < leaderTTL will now fail fast at startup — raise reconcileInterval before upgrading. Helm defaults (60s/15s) satisfy the constraint.

Testing

  • Unit tests: cube-lifecycle-manager go test ./... all pass (leaderelect, reconciler, resumer, httpapi, config, redisstream, and the new leader supervisor).
  • HA failover exercised end-to-end against a live Redis + 2-replica deployment (leader kill / partition / restart scenarios).

🤖 Generated with Claude Code

silasyyyang and others added 3 commits August 17, 2026 16:21
…encentCloud#1211)

Introduce active-standby support for cube-lifecycle-manager so a crashed
replica no longer blocks auto-pause/auto-resume. Replicas elect a leader
through a Redis lease (SETNX + compare-and-expire/release Lua scripts);
only the leader runs the stream consumer, stale-pending claim, last-active
poller, idle sweeper, and periodic reconciler. Standbys keep the HTTP
server up, report 503 on /readyz so the Service routes resume traffic to
the leader, and can still serve /internal/resume via a meta-hash fallback
when the registry misses.

On failover the new leader bootstraps from the meta hash, replays the
snapshot to every CubeProxy, claims the dead consumer's pending entries
via XAUTOCLAIM, and the reconciler converges remaining drift between the
meta hash, the in-memory registry, and proxy meta dicts. HA is opt-in via
CUBE_LCM_HA_ENABLED (default off keeps single-replica behavior); the Helm
chart enables it by default with replicas: 2.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: silasyyyang <silasyyyang@tencent.com>
Assisted-by: Kimi Work:Kimi
…>= leaderTTL (TencentCloud#1211)

- add leaderSupervisor: after 3 consecutive fast-failing leader stints (each shorter than the leader TTL) the process exits so the pod supervisor restarts it, instead of hot-looping elect -> fail -> step down forever; clean stints and stints surviving at least one TTL reset the counter

- config: in HA mode validate ReconcileInterval >= LeaderTTL, since the reconcile interval doubles as the XAUTOCLAIM min-idle for taking over a dead leader's pending stream entries; a smaller value could steal entries from a merely partitioned old leader

- docs: note both behaviors in README, values.yaml and stream.go

Assisted-by: Kimi Work:Kimi
Signed-off-by: silasyyyang <silasyyyang@tencent.com>
…1211)

da7f2eb1 only updated docs/zh/. Mirror the leader-lock registration
(redis-key-spec.md §5/§6) and the CLM high-availability section
(lifecycle.md Operational Notes) into docs/, matching the i18n sync
check in 73ea1f3.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Assisted-by: Kimi Work:Kimi
Signed-off-by: silasyyyang <silasyyyang@tencent.com>
r.o.Registry.Upsert(meta)
r.pushUpsert(ctx, sid, meta)
adopted++
case !reflect.DeepEqual(cur.Meta, meta):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral divergence from the stream OpUpdate path — stale-meta refresh never resets LastActiveMs.

handleEvent (main.go, OpUpdate case) does reg.Upsert(*ev.Meta) and reg.ResetLastActive(ev.SandboxID) after an update. This reconciler branch refreshes a stale meta via Registry.Upsert + pushUpsert but never calls ResetLastActive.

Consequence: when an update (e.g. an operator extending TimeoutSeconds) is applied only through this reconciler path — because the corresponding stream event was lost during a failover, which is exactly the case this loop exists for — the registry keeps the old LastActiveMs baseline, so the sweeper can consider the sandbox idle and pause it on the next sweep, defeating the timeout extension. The same update arriving via the stream would have reset last-active and protected it. Recommend mirroring the stream path with reg.ResetLastActive(sid).

// and Ack the returned events just like ReadGroup output. Requires Redis
// 6.2+ (XAUTOCLAIM).
func (c *Client) ClaimPending(ctx context.Context, group, consumer string, minIdle time.Duration, count int64) ([]Event, error) {
msgs, _, err := c.rdb.XAutoClaim(ctx, &redis.XAutoClaimArgs{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues in the claim loop:

  1. Cursor ignored. XAutoClaim returns a cursor as its second result, and it's discarded here while every pass restarts at "0-0". For a failover backlog larger than count (100), the takeover drains at only 100 entries per reconcile interval (60s) — a several-thousand-entry backlog takes many minutes to drain through this path. The reconciler eventually converges from the hash, so this is bounded, but it's slower than the "takes over pending entries" claim implies. Consider threading the cursor across passes.

  2. Trimmed entries are skipped but never acked on Redis 6.2. The comment says "nothing to ack (XAUTOCLAIM already dropped them from the PEL on Redis ≥ 7)" — true on 7.x, but the function explicitly documents "Requires Redis 6.2+". On 6.2, a value-less (trimmed) entry stays in the PEL, is returned again on every subsequent claim pass, and is never acked here — a slow, unbounded PEL leak. Acking the value-less entries would be harmless on 7.x (no-op) and correct on 6.2.

}
if s.fleet != nil {
resp["fleet_size"] = s.fleet.Snapshot()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standby resume fallback is unreachable via the Service during failover.

With HA enabled, standbys report 503 here and are therefore removed from the Kubernetes Service endpoints. During the failover window (leader death → new leader ready), the Service has no ready endpoints, so /internal/resume requests fail at the Service level and never reach a standby's meta-hash fallback. The fallback's real value is the freshly-promoted leader during its bootstrap (leader is ready per this gate but its registry is empty) — which is legitimate and worth keeping.

The README's claim that "a standby can still answer a resume that reaches it" is only true for direct pod-IP access, not through the Service. If resume-during-switch is a goal, standbys need to stay in the endpoint set (e.g. publishNotReadyAddresses, or a readyz criterion that doesn't gate the Service); otherwise the docs should state that resume requests fail for the switch window.

if v := os.Getenv("CUBE_LCM_HA_ENABLED"); v != "" {
// Same truthy set as CUBE_LCM_USE_STATIC_FLEET.
switch v {
case "1", "true", "TRUE", "yes":

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Truthy set is narrower than it looks. CUBE_LCM_HA_ENABLED=True (mixed case), YES, or on are all treated as false, silently disabling HA — a footgun for operators writing env files by hand (the Helm chart renders "true", so chart users are fine). HA-off degrades to the legacy single-replica behavior, so this fails safe, but a value that looks enabled while actually disabling the feature is surprising. Consider accepting any case / ParseBool-style values, or at least warning on unrecognized values instead of silently defaulting.

@cubesandboxbot

Copy link
Copy Markdown

Review: feat(clm): add active-standby HA with leader election and reconciler (#1363)

AI-generated review — reviewed the full pr.diff against the base tree; the workspace was checked out at the base branch.

Overall assessment

The core design is sound and the failure-boundedness arguments hold up:

  • Leader election uses the standard SETNX + token + Lua idiom (compare-and-expire renew, compare-and-delete release), so a stale holder can neither extend nor delete a lease it has lost.
  • Pending-entry takeover is correctly gated: XAUTOCLAIM min-idle = ReconcileIntervalLeaderTTL (enforced at startup), so entries belonging to a merely-partitioned — still alive — old leader are not stolen.
  • Cross-replica coordination is anchored on the Redis SETNX state keys, so the dual-leader window inherent to any lease-based election cannot double-pause or double-resume a sandbox.
  • The hot-loop guard, config validation, OnLost registry reset, and re-anchored startupTs per election are all thoughtful.

No blocking defect found. The issues below are behavioral divergences and robustness gaps rather than crashes.

Findings

1. [Medium] Reconciler update path omits ResetLastActive — diverges from the stream OpUpdate pathcube-lifecycle-manager/internal/reconciler/reconciler.go:110
When the reconciler refreshes a stale meta (e.g. a timeout extension that arrived via a lost update event — the exact case this loop exists for), it calls Registry.Upsert + pushUpsert but not Registry.ResetLastActive, whereas the stream consumer's OpUpdate path always resets LastActiveMs. A sandbox whose update was applied only through the reconciler keeps its old idle baseline, so the sweeper can pause it on the next sweep, defeating the operator's intent; which path applies the update changes behavior. Mirror the stream path (reg.ResetLastActive(sid)).

2. [Medium] XAUTOCLAIM cursor discarded; trimmed entries never acked on Redis 6.2cube-lifecycle-manager/internal/redisstream/stream.go:163
The cursor returned by XAutoClaim is ignored and every pass restarts at "0-0", so a large failover backlog drains at only 100 entries per reconcile interval (60s). Separately, value-less (trimmed) entries are skipped without acking — correct on Redis ≥ 7 (where XAUTOCLAIM auto-drops them from the PEL) but on Redis 6.2 — which the comment explicitly claims to support — they stay in the PEL forever, are returned on every pass, and are never acked: a slow PEL leak. Acking them is a no-op on 7.x and correct on 6.2.

3. [Medium] Standby resume fallback is unreachable via the Service during failovercube-lifecycle-manager/internal/httpapi/server.go:179
Standbys report 503 on /readyz, so Kubernetes removes them from Service endpoints. During the failover window (leader death → new leader ready) the Service has no ready endpoints and /internal/resume fails at the Service level — a standby's meta-hash fallback is never exercised. The fallback's real value is the freshly-promoted leader during its bootstrap (ready, empty registry), which is legitimate. The README's "a standby can still answer a resume that reaches it" only holds for direct pod-IP access. If resume-during-switch is the goal, standbys need to stay in the endpoint set (e.g. publishNotReadyAddresses); otherwise the docs should state resume requests fail for the switch window.

4. [Low] CUBE_LCM_HA_ENABLED truthy set is narrowcube-lifecycle-manager/internal/config/config.go:262
Accepts 1/true/TRUE/yes but not True, YES, or on. A common True in an operator env file silently disables HA (fails safe to legacy behavior, but surprising). Consider case-insensitive / ParseBool-style parsing or warning on unrecognized values.

Lower-severity notes

  • Lease-expiry dual-leader window (inherent, bounded). When renewals fail with transport errors, the lease can expire in Redis while this instance still believes it is leader until the next renew attempt (≤ renew interval later); a peer can acquire during that window, giving two concurrent leaders briefly. Safe here because (a) XAUTOCLAIM min-idle (60s) ≫ the window so pending entries aren't stolen, and (b) pause/resume is serialized by the SETNX state keys. Worth a code comment, since the fence is implicit.
  • claimStalePending can re-claim the live leader's own in-flight batch. consumeStream processes a batch of up to 100 events sequentially; if a batch stalls on slow proxy pushes past min-idle (60s), the front of the batch gets claimed and re-processed concurrently. Handling is idempotent (upserts/deletes/state sets), so this is wasted work, not corruption — but under sustained proxy slowness it can amplify load. Consider XCLAIM targeting only other consumers, or a larger min-idle.
  • Elector can start a new stint before the previous one fully drains. loseLeadership cancels the leader context but nothing waits for the previous OnElected goroutine (and its five loop goroutines) before a re-acquisition can spawn a new one. In practice the renew interval (5s) is far longer than the drain, so this is largely theoretical, but a WaitGroup in the elector (or a short min-delay before re-running OnElected) would make it a guarantee.
  • Reconciler cost at scale. Every interval it does a full HGETALL of the meta hash plus a DeepEqual over every entry, and re-pushes fleet-wide on any drift. Fine at the Helm defaults, worth watching on large meta hashes.
  • claimStalePending runs its body immediately on election (before the first tick), unlike the reconciler which waits for the first tick. Not a bug, just an asymmetry worth a comment.

Test quality

The new unit tests are strong: supervisor fast-fail/exit/reset matrix, elector acquire/renew/stolen-lease/step-down/shutdown, reconciler adopt/refresh/evict/grace-window/bootstrap-error, resumer meta-fallback (including the deliberate no-cache guarantee), and the readyz leader gate. Config tests cover the HA validation constraints including the reconcile >= leaderTTL rule. The XAUTOCLAIM/failover path and the standby-reachable-resume question are the two areas that would benefit from an integration-level test against a live Redis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add Active-Standby support for CLM

3 participants