Retry the LogCluster readiness probe inside the INSERT retry loop - #113592
Open
alexey-milovidov wants to merge 3 commits into
Open
Retry the LogCluster readiness probe inside the INSERT retry loop#113592alexey-milovidov wants to merge 3 commits into
alexey-milovidov wants to merge 3 commits into
Conversation
`LogCluster.do_query` probed `is_ready` once, before its retry loop. That probe is a `SELECT 1` against the same writer endpoint as the INSERT, so it fails during exactly the memory-pressure spikes (Code 241 for every query) that the retries added in #113409 exist to ride out: a single transient failure of the probe abandoned the upload before any `POST` was attempted, and `build_profile_hook.py` failed the build job closed. Move the probe into the retry loop, on the same growing backoff `select` already uses for it, and report the never-became-ready case explicitly so the fail-close assert is not the only thing in the log. Follow-up to #113409 Related: #113408
Contributor
|
Workflow [PR], commit [a81a86b] Summary: ✅
AI ReviewSummaryThis PR fixes Final Verdict
|
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
When `is_ready` succeeds but every `_session.post` raises (timeout, connection reset, ...), `response` stays `None` and the terminal diagnostic printed `ERROR: LogCluster not ready`, pointing the incident at the wrong subsystem. Track whether a `POST` was ever attempted and print the never-became-ready message only when the loop never got past `is_ready`; otherwise report that every `POST` attempt failed with an exception (the tracebacks are already in the log). Adds regression tests for both terminal paths in `ci/tests/test_build_profile_hook.py`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 6, 2026
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
1 participant
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.
LogCluster.do_query— the build-profile telemetry INSERT transport — probedis_readyonce, before its retry loop. That probe is aSELECT 1against the same writer endpoint as the INSERT itself, so it fails during exactly the minutes-long memory-pressure spikes (Code 241 for every query) that the retries added in #113409 exist to ride out. A single transient failure of the probe abandoned the upload before anyPOSTwas attempted, andbuild_profile_hook.py(correctly) failed the build job closed — defeating the mitigation.selectin the same file already retries its readiness probe inside its own backoff loop, with a comment documenting this exact reason. This movesdo_query's probe into its retry loop on the same growing backoff, and reports the never-became-ready case explicitly so the caller's fail-closeassertis not the only thing in the log.A regression test in
ci/tests/test_build_profile_hook.pydrives the probe as[False, False, True]and asserts the INSERT still runs.Found by
clickhouse-ghreview on #113409 after it merged.Related: #113409
Related: #113408
Changelog category (leave one):