fix(weave): classify ClickHouse TOO_SLOW as QueryEstimatedTimeoutExceededError - #7607
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ededError ClickHouse rejects an over-broad query up front with error code 160 (TOO_SLOW) when its *estimated* execution time exceeds max_estimated_execution_time -- the planner declines to run it rather than timing out mid-flight. handle_clickhouse_query_error() had no branch for this, so it fell through to the generic clickhouse_connect DatabaseError and was surfaced as a 502 "Temporary backend error". That is a benign, self-inflicted user-query rejection (the same class of query-too-heavy guard as TIMEOUT_EXCEEDED), but the misclassification meant it read as an unexpected server fault: it returned an opaque 5xx to the customer instead of actionable "limit the scope" guidance, and it tripped the paging monitor for unexpected weave-trace 5xx (DD 19065158 / WB-33068). Add a dedicated QueryEstimatedTimeoutExceededError (504) and map TOO_SLOW to it. It is kept distinct from QueryTimeoutExceededError -- a query that actually ran and timed out mid-flight -- so the pre-execution rejection is identifiable on its own in tracing and alerting. NOTE: because this is a new error type, monitor 19065158 (which excludes error.type:QueryTimeoutExceededError) must also exclude error.type:QueryEstimatedTimeoutExceededError to stay quiet on these customer-driven query rejections. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mscavezze-cw
force-pushed
the
mike/trace-server-map-clickhouse-too-slow
branch
from
July 22, 2026 21:52
b66cbfa to
6bb633f
Compare
mscavezze-cw
changed the title
mscavezze-cw
marked this pull request as ready for review
July 22, 2026 22:58
gtarpenning
approved these changes
Jul 22, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
JIRA Issue(s)
WB-37836
Description
TOO_SLOWcurrently shows up as(Unexpected error - investigate!)in our alerts (recent example). This raises unnecessary concern for a known-issue and creates alert fatigue. I'm adding a new error type that we can filter on, so we can have cleaner alerts. I'm keeping this distinct from QueryTimeoutExceededError, so that we can differentiate between timeouts and predicted timeouts.Testing
How did you test your PR? What is the blast radius of your change? Provide a test plan.