fix: default probe ports and probe-failure rollout gating#262
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the default ClickHouse container liveness/readiness probes to better reflect ClickHouse startup phases, aiming to prevent liveness-driven restarts during slow synchronous table loads and to make readiness imply “serving queries” (so endpoints/rollout gating are more accurate).
Changes:
- Switch liveness probing to
/pingon the interserver port (intended to open before table loading finishes). - Switch readiness probing to
/pingon a newly introduced management HTTP port (9002). - Add
PortManagementHTTP = 9002and register a newmanagement-httpcomposable protocol/port.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/controller/clickhouse/templates.go |
Adjusts default liveness/readiness probe handlers and adds a new management-http composable protocol definition. |
internal/controller/clickhouse/constants.go |
Introduces PortManagementHTTP constant used by the new readiness probe and protocol. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
458
to
462
| readinessProbe.ProbeHandler = corev1.ProbeHandler{ | ||
| HTTPGet: &corev1.HTTPGetAction{ | ||
| Path: "/ping", | ||
| Port: intstr.FromInt32(PortInterserver), | ||
| Port: intstr.FromInt32(PortManagementHTTP), | ||
| Scheme: corev1.URISchemeHTTP, |
Comment on lines
+446
to
448
| // Interserver listener opens before table loading, client listeners after: | ||
| // liveness survives slow synchronous loads, readiness implies serving queries. | ||
| livenessProbe := controller.DefaultLivenessProbeSettings |
Liveness now probes /ping on the interserver port, which opens before table loading: a slow synchronous load (async_load_databases=false, large table count) no longer gets the container killed after ~110s. Readiness moves to /ping on a new always-on management-http composable protocol (9002), which opens together with client listeners after table loading: Ready now implies the server accepts queries, keeping Service endpoints and the rolling-update gate truthful during slow startups. A probe or reload failure against a reachable replica no longer escalates to StageError: the error branch updates replicas without rollout serialization, so a transient dial failure against the healthy peer mid-rollout could take down the last serving replica. Reload is attempted only after a successful probe, and ReloadError now maps to StageNotReadyUpToDate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GrigoryPervakov
force-pushed
the
fix-default-probes
branch
from
450d82c to
4dd46ae
Compare
GrigoryPervakov
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A probe or reload failure against a reachable replica no longer
escalates to StageError: the error branch updates replicas without
rollout serialization, so a transient dial failure against the healthy
peer mid-rollout could take down the last serving replica. Reload is
attempted only after a successful probe, and ReloadError now maps to
StageNotReadyUpToDate.
What
Liveness now probes /ping on the interserver port, which opens before
table loading: a slow synchronous load (async_load_databases=false,
large table count) no longer gets the container killed after ~110s.
Readiness moves to /ping on a new always-on management-http composable
protocol (9002), which opens together with client listeners after table
loading: Ready now implies the server accepts queries, keeping Service
endpoints and the rolling-update gate truthful during slow startups.
Related Issues
Related to #261