insights clickhouse query improvements based on quantilesTDigest - #8401
insights clickhouse query improvements based on quantilesTDigest#8401n1ru4l wants to merge 9 commits into
quantilesTDigest#8401Conversation
| ADD COLUMN IF NOT EXISTS graph_name LowCardinality(String) DEFAULT '' CODEC(ZSTD(1)) AFTER target, | ||
| ADD COLUMN IF NOT EXISTS graph_version String DEFAULT '' CODEC(ZSTD(1)) AFTER graph_name |
There was a problem hiding this comment.
Since we want to introduce soon multiple graphs within a single target and showing usage only for a specific graph_version (schema version), it makes sense to introduce these columns as part of this pull request to avoid another migration in the near future.
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/apollo |
0.48.7-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/cli |
0.63.2-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/core |
0.22.5-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/envelop |
0.40.12-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-plugin-console-sdk |
0.1.6-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/yoga |
0.49.6-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
hive |
11.13.0-alpha-20260909144711-dab4fb297371f72ff392d03e4bde03cdcb9ee6fa |
npm ↗︎ unpkg ↗︎ |
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tags: |
quantilesTDigestquantilesTDigest
e1f2999 to
d02a1f0
Compare
jdolle
left a comment
There was a problem hiding this comment.
If you have time, and since youve already managed to create a great set of test data, I think this is a good opportunity to explore moving the timestamp up in the order. We've managed to address one-off cases (e.g. metric alerts) where the index was causing slow requests by creating a separate specialized table, but I believe moving the timestamp will better support a more diverse set of query conditions and remove some of the latency peaks.
| ) | ||
| ENGINE = SummingMergeTree | ||
| PARTITION BY tuple() | ||
| PRIMARY KEY (target, graph_id, hash) |
There was a problem hiding this comment.
| PRIMARY KEY (target, graph_id, hash) | |
| PRIMARY KEY (target, graph_id, timestamp, hash) |
| ENGINE = SummingMergeTree | ||
| PARTITION BY tuple() | ||
| PRIMARY KEY (target, graph_id, hash) | ||
| ORDER BY (target, graph_id, hash, client_name, client_version, timestamp, graph_version_id) |
There was a problem hiding this comment.
| ORDER BY (target, graph_id, hash, client_name, client_version, timestamp, graph_version_id) | |
| ORDER BY (target, graph_id, timestamp, hash, client_name, client_version, graph_version_id) |
I agree with the graph_id being always part of our lookup, since the use-cases for graph_id includes things like feature branches. It makes sense to always separate that traffic. So I agree it should come after the target.
However, all the data we show and use is also time-bound. I think we should move the timestamp to after the target, graph_id in order to ensure our operations can efficiently use the sort key(s). This may slightly slow down the per-hash operation, (Insights -> Operation) since it means first filtering the larger column (timestamp) and then filtering the hash, but it should make the unfiltered results and the table showing a the list of operations significantly faster.
This ordering is likely the reason why there's less of an improvement to the hourly query with the current table design. It can't efficiently use the sort key when applying the aggregate function.
| PROJECTION by_graph_version_id | ||
| ( | ||
| SELECT * | ||
| ORDER BY (target, graph_id, graph_version_id, hash, client_name, client_version, timestamp) |
There was a problem hiding this comment.
note that unlike the main table, the projection uses the order by as the primary key and doesn't allow an override. So if you want to have the primary key as target, graph_id, timestamp, graphql_version_id, hash and keep client_name and client_version as sorted columns, then it may make sense to create yet another table.
Another option is to select everything (as you have) and remove client_name, client_version from the ORDER BY to have them as unordered columns. Considering how everything is time-boxed, this may be reasonable.
…est_minutely, operations_tdigest_hourly and operations_tdigest_daily, clients_tdigest_minutely, clients_tdigest_hourly and clients_tdigest_daily with their corresponding ingestion materialized views
4e84615 to
7059cbe
Compare
Add new ClickHouse operation and client rollup tables using
quantilesTDigestfor faster Insights queries.This also introduces the
graph_idandgraph_version_iddimensions to the sort key, to allow us filtering by those later on when we introduce these features.The implementation allows gradually switch reads to the new tables based on a
CLICKHOUSE_TDIGEST_ROLLUPS_STARTISO timestamp, while retaining legacy rollups for older data ranges.A summary of the performance improvements:
operation daily aggregations: 2.8–4.5x faster across 3–12 months
operation hourly aggregations: Improvements decline with range, from 3.2–3.9x at 1 day to 1.2–1.4x at 14 days. At 30 days, only per-operation percentiles improved meaningfully
The bottleneck for the quantile aggregations is CPU.
Querying a dataset of 14 days based on hourly aggregation table benchmarked with different CPU limits:
Aside from these improvements, we should consider enabling time based scaling for traffic peaktimes.