fix(parca): add connection params as overridable settings and improve… - #3
Merged
javicj merged 1 commit intoSep 4, 2026
Conversation
alexon1234
approved these changes
Sep 4, 2026
alexon1234
reviewed
Sep 4, 2026
|
@javicj LGTM |
alexon1234
reviewed
Sep 4, 2026
alexon1234
reviewed
Sep 4, 2026
alexon1234
reviewed
Sep 4, 2026
| if start.Unix() != 0 && end.Unix() != 0 { | ||
| conditions = append(conditions, "time_nanos > ? AND time_nanos < ?") | ||
| args = append(args, start.UnixNano(), end.UnixNano()) | ||
| conditions = append(conditions, "timestamp BETWEEN ? AND ?") |
Author
There was a problem hiding this comment.
No, it is a Int64.
show create table parca.stacktraces
SHOW CREATE TABLE parca.stacktraces
Query id: 12174b31-2067-4d38-a330-71429647a957
┌─statement──────────────────────────────────────────────────────────────────────────────────┐
1. │ CREATE TABLE parca.stacktraces ↴│
│↳( ↴│
│↳ `name` String, ↴│
│↳ `sample_type` String, ↴│
│↳ `sample_unit` String, ↴│
│↳ `period_type` String, ↴│
│↳ `period_unit` String, ↴│
│↳ `period` Int64, ↴│
│↳ `duration` Int64, ↴│
│↳ `timestamp` Int64, ↴│
│↳ `time_nanos` Int64, ↴│
│↳ `value` Int64, ↴│
│↳ `labels` JSON, ↴│
│↳ `stacktrace.address` Array(UInt64), ↴│
│↳ `stacktrace.mapping_start` Array(UInt64), ↴│
│↳ `stacktrace.mapping_limit` Array(UInt64), ↴│
│↳ `stacktrace.mapping_offset` Array(UInt64), ↴│
│↳ `stacktrace.mapping_file` Array(LowCardinality(String)), ↴│
│↳ `stacktrace.mapping_build_id` Array(LowCardinality(String)), ↴│
│↳ `stacktrace.line_number` Array(Int64), ↴│
│↳ `stacktrace.function_name` Array(LowCardinality(String)), ↴│
│↳ `stacktrace.function_system_name` Array(LowCardinality(String)), ↴│
│↳ `stacktrace.function_filename` Array(LowCardinality(String)), ↴│
│↳ `stacktrace.function_start_line` Array(Int64) ↴│
│↳) ↴│
│↳ENGINE = MergeTree ↴│
│↳PARTITION BY toYYYYMMDD(fromUnixTimestamp64Nano(time_nanos)) ↴│
│↳ORDER BY (name, sample_type, sample_unit, period_type, period_unit, timestamp, time_nanos)↴│
│↳TTL fromUnixTimestamp64Nano(time_nanos) + toIntervalDay(30) ↴│
│↳SETTINGS index_granularity = 8192 │
└────────────────────────────────────────────────────────────────────────────────────────────┘
1 row in set. Elapsed: 0.001 sec.
There was a problem hiding this comment.
BTW why you think using timestamp would be better than using time_nanos?
There was a problem hiding this comment.
Also for this case we are replacing time_nanos > ? for time_nanos >= ? internally, so IDK if it's okay.
There was a problem hiding this comment.
Yes—benchmark confirms a significant improvement.
Tested against a 5M-row ClickHouse MergeTree table with the PR’s ordering key and a 1-hour range:
┌────────────────────────┬───────────┬────────────┬────────┐
│ Predicate │ Rows read │ Read bytes │ Median │
├────────────────────────┼───────────┼────────────┼────────┤
│ timestamp BETWEEN ... │ 212,992 │ 17.5 MB │ 15 ms │
├────────────────────────┼───────────┼────────────┼────────┤
│ time_nanos BETWEEN ... │ 4,629,629 │ 52.8 MB │ 74 ms │
└────────────────────────┴───────────┴────────────┴────────┘
EXPLAIN indexes=1 showed:
- timestamp: 26/613 granules
- time_nanos: 567/613 granules
So timestamp read ~22× fewer rows and was ~5× faster in this test. Best implementation: use timestamp for index pruning plus the exact time_nanos predicate to preserve nanosecond range
semantics.
Author
There was a problem hiding this comment.
Yeah, I tested it and noticed a good improvement.
…table DISTINCT HasProfileData delegated to ProfileTypes with a zero time range, which skips the time filter and ran a DISTINCT over every row in the table, only for the result to be reduced to a boolean. A LIMIT 1 probe returns the same answer after reading a single granule.
javicj
force-pushed
the
jcanadas/pltf-965-optimize-parca-clickhouse-queries-to-prevent-ui-hangs
branch
from
September 4, 2026 12:28
6360a1a to
dfe20f4
Compare
|
LGTM |
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.
Summary
Two ClickHouse query optimizations for the query path, to avoid the Parca UI error"The Parca server hasn't
received any data yet":
HasProfileDatano longer full-scans the table.Change:
HasProfileDataBefore — it delegated to
ProfileTypeswith a zero time range:ProfileTypes skips its time filter when start/end are zero, so every call ran:
The result was immediately reduced to a boolean — the distinct list was never used. DISTINCT cannot terminate early, so ClickHouse read the entire table (426M rows at the time of the incident) on every UI page load, just to answer yes/no. This was the exact RPC failing with 30s clickhouse: acquire conn timeout during the incident.
Identical semantics — both answer "does the table contain at least one row", since any row necessarily has profile-type values (non-nullable columns) — but ClickHouse reads one granule and stops. The check gates the UI's empty-state splash screen only; data browsing uses QueryRange/QueryMerge/ Labels with the user's selected range, so query results are unaffected. ProfileTypes itself is unchanged (its real callers need the list).