Make ClickhouseRepo query timeout and max_execution_time configurable via env vars - #6691
Open
sp-kjablonski wants to merge 1 commit into
Open
sp-kjablonski wants to merge 1 commit into
sp-kjablonski wants to merge 1 commit into
Conversation
Since plausible#6018 stats queries against ClickHouse have been limited by a hardcoded 15s client-side timeout and a 20s server-side max_execution_time, with no way to change either on Community Edition short of building a custom image. Read both from CLICKHOUSE_QUERY_TIMEOUT_MS and CLICKHOUSE_MAX_EXECUTION_TIME_SEC. The current values stay as defaults, and max_execution_time defaults to the client timeout in seconds + 5 so the server-side limit keeps sitting slightly above the client one. Ref plausible/community-edition#277
|
|
This branch has not been deployed
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
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.
Changes
Addresses plausible/community-edition#277 (see also plausible/community-edition#75 and #4359).
Since #6018,
Plausible.ClickhouseReporuns stats queries with two hardcoded limits inconfig/runtime.exs: a client-sidetimeout: 15_000and a server-side ClickHouse settingmax_execution_time: 20. There was no way to change either on Community Edition short of building a custom image, so on a high-traffic site any dashboard query taking longer than 15s (e.g. month-long period +with_imported=true+event:props:*filters) fails withDBConnection.ConnectionError ... timed out because it queued and checked out the connection for longer than 15000ms.This PR makes both values configurable via environment variables while keeping the current values as defaults:
CLICKHOUSE_QUERY_TIMEOUT_MSClickhouseRepoconnection:timeoutfor stats queries15000(unchanged)CLICKHOUSE_MAX_EXECUTION_TIME_SECmax_execution_timesettingCLICKHOUSE_QUERY_TIMEOUT_MS / 1000 + 5, i.e.20(unchanged)Deriving the server-side limit from the client timeout keeps the existing invariant (server limit slightly above client timeout) without requiring self-hosters to tune two knobs; the second variable is there for those who want to override it explicitly. Nothing changes for deployments that don't set these variables.
Tests
Added a
describe "clickhouse"block totest/plausible/config_test.exscovering the defaults,CLICKHOUSE_QUERY_TIMEOUT_MSalone (with derivedmax_execution_time), both variables together, and a non-integer value raising.Changelog
Documentation
The relevant place is the CE wiki Configuration page (Database section), which I can't open a PR against. Suggested text:
Dark mode
🤖 Generated with Claude Code