Skip to content

feat(api): add connection-aware readiness probe for SSE - #2787

Merged
t-kikuc merged 5 commits into
mainfrom
feat/sse-connection-aware-readiness
Aug 26, 2026
Merged

feat(api): add connection-aware readiness probe for SSE#2787
t-kikuc merged 5 commits into
mainfrom
feat/sse-connection-aware-readiness

Conversation

@t-kikuc

@t-kikuc t-kikuc commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Part of #2152

What this PR does

Adds a readiness-only health check that returns 503 when SSE connections reach the configured threshold (default 95% of --sse-max-connections). This removes the pod from Kubernetes Endpoints and GCLB NEG, stopping new connections from being routed to saturated pods while keeping existing SSE connections alive.

Background

When a pod hits its SSE connection limit, it returns 503 but the readiness probe continues to return 200. GCLB keeps sending new connections to the saturated pod, causing 503 errors until HPA scale-out catches up. The existing Envoy circuit breaker does not detect SSE connection saturation due to HTTP/2 multiplexing.

This readiness probe reads Dispatcher.ActiveConns() directly to detect saturation.

Verified in dev: 1M SSE load test showed 15x reduction in 5xx rate (867/min vs 13.3K/min baseline).

Points

  • Only affects readiness, not liveness — pod is never restarted due to high connection count
  • New --sse-readiness-threshold flag (default: 0.95) controls the fraction at which readiness starts failing
  • With --sse-max-connections=16000 and threshold 0.95, readiness fails at 15,200 — leaving an 800-connection buffer for probe propagation delay (~3-5s)
  • Helm chart updated: sseReadinessThreshold added to values.yaml and deployment.yaml

When SSE connections reach the configured threshold (default 95% of
--sse-max-connections), the readiness probe returns 503. This removes
the pod from Kubernetes Endpoints and GCLB NEG, preventing new
connections from being routed to saturated pods while keeping existing
SSE connections alive.

The Envoy sidecar circuit breaker is ineffective for SSE over HTTP/2
because multiplexed streams are not counted as active TCP connections
or requests. This readiness check bypasses Envoy and reads the
Dispatcher's connection count directly.

Closes #2786

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 25, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds connection-aware SSE readiness checks so saturated API pods stop receiving new traffic without affecting liveness.

Changes:

  • Adds synchronous readiness-only health checks.
  • Exposes thread-safe SSE connection counts.
  • Configures readiness failure at an SSE capacity threshold.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/health/health.go Adds readiness-specific checks.
pkg/health/health_test.go Tests readiness and liveness behavior.
pkg/api/stream/dispatcher.go Exposes current and maximum connections.
pkg/api/stream/dispatcher_test.go Tests connection status tracking.
pkg/api/cmd/server.go Adds and wires the SSE readiness threshold.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/api/cmd/server.go Outdated
Comment thread pkg/health/health.go
Comment thread pkg/api/cmd/server.go
t-kikuc and others added 4 commits August 26, 2026 04:43
- Use math.Ceil instead of int truncation for threshold calculation
- Validate sse-readiness-threshold is within 0.0-1.0 range
- Add duplicate name guard to WithReadinessCheck (same as WithCheck)
- Add sseReadinessThreshold to Helm chart values and deployment template

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Return only the current connection count since max is unused by callers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread pkg/health/health_test.go
Comment thread pkg/health/health_test.go

@hvn2k1 hvn2k1 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.

Thank you!

@t-kikuc
t-kikuc merged commit 27448d8 into main Aug 26, 2026
14 checks passed
@t-kikuc
t-kikuc deleted the feat/sse-connection-aware-readiness branch August 26, 2026 09:01
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.

3 participants