Skip to content

feat(proxy): add CN health circuit breaker to skip overloaded CNs - #25073

Merged
XuPeng-SH merged 4 commits into
mainfrom
feat/proxy-cn-health-breaker
Jun 22, 2026
Merged

feat(proxy): add CN health circuit breaker to skip overloaded CNs#25073
XuPeng-SH merged 4 commits into
mainfrom
feat/proxy-cn-health-breaker

Conversation

@XuPeng-SH

@XuPeng-SH XuPeng-SH commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

#24919

What this PR does / why we need it:

Introduce a CN health circuit breaker in Proxy routing to skip temporarily unavailable CN nodes, so fresh logins stop repeatedly paying auth timeout on known-bad CNs.

Highlights:

  • Skip CNs in active cooldown for new sessions; fast-fail with ErrAllCNServersBusy when all candidates are temporarily unhealthy.
  • Allow bounded half-open recovery probes so recovered CNs re-enter rotation automatically.
  • Preserve single-CN semantics by defaulting cn-health-check-fail-threshold to 2, so a single transient blip does not immediately sideline the only CN.
  • Keep transfer/control-plane connects out of breaker accounting.
  • Make cached connection reuse honor the same CN health decision before issuing SET CONNECTION ID.
  • Preserve hard-unavailable semantics: timeout/overload signals trip the breaker, while hard dial/early handshake failures are not reclassified as busy.

Add a per-CN health circuit breaker that tracks connection failures and
temporarily skips unhealthy CNs during route selection, preventing new
connections from paying auth timeout costs on known-bad CNs.

- cn_health: new circuit breaker with configurable fail threshold,
  exponential cooldown (base→max), half-open probe window, and stale
  entry sweeping.
- router: integrate health.pick() into Route(); add ConnectRouteSelected
  to distinguish Route-selected connects (feeds breaker) from
  internal/admin connects (does not).
- client_conn: use RouteForTransfer for session migration to avoid
  consuming breaker-managed probes; Route-selected new-session
  connects feed timeout/overload failures into the breaker.
- config: expose CN-health-check-* configuration knobs.
- metrics: add proxy_cn_health_{unhealthy,trips,success_probe} counters.

Fixes #24919
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I rechecked the latest head (bb1ad656). The code-side cache bypass issue is addressed: cached reuse now checks CN health before SET CONNECTION ID, and plugin routing no longer allows fresh-login cache reuse.

I still cannot approve this exact diff yet because two previously raised cleanup/metadata issues remain in the current head:

  • The PR body still lists #24919 under "Which issue(s) this PR fixes", but that issue is about a cross-CN shuffle INSERT ... SELECT execution deadlock, not proxy CN health routing.
  • pkg/sql/colexec/table_function/filter_helper_gpu_test.go is still in the diff with whitespace-only comment alignment changes, despite the reply saying it was dropped.

Please remove those from the current PR state and I can approve the code changes.

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Rechecked the latest head; the proxy CN health breaker now gates cached connection reuse before SET CONNECTION ID, keeps plugin fresh-login routing from being bypassed by connCache, and fixes the cooldown overflow path.

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

Labels

kind/bug Something isn't working kind/enhancement kind/feature size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants