Repository navigation
fix(clickhouse): enforce time and read limits on every query - #77
Merged
Merged
Conversation
max_execution_time was set as a connection setting, which the HTTP driver sends as a URL parameter, and chproxy strips URL parameters. So no query had a limit: 11-05 saw scans of up to 19 TiB and 745 s. Every query now carries a SETTINGS clause in its text, which reaches ClickHouse through any proxy: max_execution_time (default 20 s), max_rows_to_read (1B) and max_bytes_to_read (64 GiB) with read_overflow_mode 'throw'. A query that hits one of them returns a 400 that tells the caller to narrow its primary-key range, not a 500, also when the limit fires after the first rows have streamed. Also makes TestClient_QueryTimeout outlast its deadline: summing 1B numbers could finish inside 100 ms, and it failed on master. Co-authored-by: Sam Calder-Mason <samcaldermason@gmail.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a1a60e-fffe-740c-b50a-32436a4f6230 Co-authored-by: Sam Calder-Mason <samcaldermason@gmail.com>
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.
Lab data-quality audit (API-2), approved by the Lab coordinator.
Problem.
max_execution_timewas a connection setting. Over HTTP the driver sends it as a URL parameter, and chproxy (between cbt-api and ClickHouse in production) strips URL parameters. So no query ever had a limit, and the readonly server default is 0. On 2026-11-05, scans ran up to 19 TiB and 745 s, long after cbt-api had returned its 30 s error.Fix.
SETTINGSclause in its text, which survives any proxy:max_execution_time(default now 20 s, was 60),max_rows_to_read(default 1,000,000,000) andmax_bytes_to_read(default 64 GiB), withread_overflow_mode = 'throw'. All three are configurable; 0 disables one.rows.Err(), so a limit that fires after rows started streaming returns the 400, not a silently short page.cancel_http_readonly_queries_on_client_closeis not set: it is read from the HTTP request, which chproxy filters, and the server-sidemax_execution_timenow ends the query anyway.Sizing (lab-backend's queries over 7 days, from query_log): p99 901K rows / 109 MiB, max 717M rows / 47.7 GiB (state-expiry
address_like=0x%, custody 19-day probe dialog). The caps sit above the measured max. In production today the heaviest Lab requests take 0.4–4.2 s, well under 20 s.Tests.
TestClient_ReadLimitsagainst a real ClickHouse over native and HTTP: a query undermax_rows_to_readsucceeds, one over it fails and maps to 400.TestClient_QueryTimeoutfailed on master (latest master run): its 1B-number sum could finish inside the 100 ms deadline and then hit a scan error. It now usessleep(2).make testpasses locally: 474 unit tests and the integration suite.Deploy note: platform's xatu-cbt-api values set
max_execution_time: 60explicitly; change it to 20 (or drop it) in the bump.