Use a continuous primary-key range for whole-metric PromQL selectors of TimeSeries tables

[nikitamikhaylov]

A PromQL selector over a TimeSeries table filters the samples table with id IN (SELECT id FROM tags WHERE <matchers>). For a metric with tens of thousands of series, KeyCondition runs its single-threaded generic exclusion search with the whole set: 284 ms per selector on a 62-billion-row part with 1.9M marks (503 ms at 8.1M marks), and rule-style queries evaluate up to 5 selectors.

With the two-component id layout Tuple(UInt64, UUID) the canonical id generator derives the first component from the metric name alone, so all series of one metric form one continuous primary-key range. When a selector matches a whole metric — verified by metadata checks plus one LIMIT 1 probe on the tags table, which also detects out-of-range ids left by an earlier ALTER ... MODIFY SETTING id_generator — the generated WHERE additionally carries id >= tuple(hash(name), min) AND id <= tuple(hash(name), max) and the inner query sets use_index_for_in_with_subqueries_max_values = 1. Index analysis uses the range; the IN stays for exact row filtering, so both emissions return identical rows on any data. Any failed check emits today's SQL unchanged. The range can select a few extra boundary granules (+5 of 135,005 marks on a 30-day scan).

Measured effect

tsbench PromQL suite: 62.455B samples / 361,432 series, 1.9M-mark part; Ryzen 9950X (16C/32T); baseline = clean master 9b6a2d7. Cold medians of 3 interleaved rounds:

query	master	this PR	delta
s07 (30m range)	2.22 s	1.23 s	−44.8%
r03 (rule, 3 selectors)	5.02 s	3.07 s	−38.8%
s11 (24h range)	4.30 s	2.98 s	−30.8%
r02 (25.6k-series instant)	3.04 s	2.17 s	−28.7%
s06 (24h instant)	5.04 s	3.61 s	−28.4%
full 24-query suite, cold geomean	950 ms	847 ms	−10.8%

Selectors that do not match a whole metric fall back and are unaffected (r05, s05: ±0.3%). Probe cost on non-firing selectors: ~2–4 ms each (r07: 66 → 73 ms); single-component id layouts never reach the probe. The removed cost grows with mark count, so the effect is larger at index_granularity_bytes = 262144.

Tests

04836_time_series_selector_whole_metric_pk_range: fires for whole-metric selectors (plan carries the range, IN retained), falls back byte-identically for label-filtered, regex, custom-generator, and ALTERed-id_generator history cases; both tuple layouts; full prometheusQuery/prometheusQueryRange results compared. tests/performance/promql_selector_pk_range.xml: 20,000-series metric, range path plus fallback control.

Related: #113768 (open) — removes no-op casts in the same generated SELECT; complementary, each stands alone.

---

Changelog category (leave one):

• Performance Improvement

Changelog entry:

PromQL selectors that match all series of one metric now filter the samples table of a TimeSeries table with a continuous primary-key range on id during index analysis instead of a large id IN <set> condition, when the id layout is a two-component tuple with the canonical id generator. Removes the dominant single-threaded index-analysis cost of selector-heavy PromQL queries: up to −45% cold latency on dashboard and rule query shapes, −11% cold geomean over the full suite on a 62-billion-sample table.

🤖 Generated with Claude Code

[clickhouse-gh]

Workflow [PR], commit [06852ed]

---

AI Review

Summary

This PR adds a whole-metric fast path for timeSeriesSelector / PromQL selectors on TimeSeries tables by adding a continuous id range for canonical two-component ids and excluding the large id IN set from key analysis. The default (id, timestamp) layout looks consistent with the intended optimization, but the current head still leaves two previously reported blockers unresolved: it enables the path on supported samples layouts where that range cannot prune marks, and it relies on mutable id_generator metadata after the probe result has already been computed.

Findings

❌ Blockers

• [src/Storages/StorageTimeSeriesSelector.cpp:643-645, 826-837] whole_metric_id_range_conditions only checks that the samples target physically has an id column of the expected type before forcing use_index_for_in_with_subqueries_max_values = 1. TimeSeries still supports custom and external samples targets with arbitrary engines and key layouts, and normalizeTimeSeriesDefinition only validates their column types, not that the primary key starts with raw id ([src/Storages/TimeSeries/normalizeTimeSeriesDefinition.cpp:904-909]). On a supported layout like ORDER BY (timestamp, id), the new range cannot prune marks, so whole-metric selectors lose the existing id IN (...) primary-key pruning and regress to timestamp-only or full scans. Suggested fix: guard the optimization on a MergeTree-family samples target whose primary key / sorting key begins with raw id, or leave use_index_for_in_with_subqueries_max_values unchanged otherwise; add a regression test with a custom or external samples table.
• [src/Storages/StorageTimeSeriesSelector.cpp:653-659, 677-680, 809-837] Once the computed metric range is appended to WHERE, it stops being a pure hint and must stay a superset until the main query takes its read snapshot. TimeSeries still allows ALTER TABLE ... MODIFY SETTING id_generator after CREATE ([src/Storages/StorageTimeSeries.cpp:63-68, 1053-1063]), and external tags tables can change the id DEFAULT independently. If that generator source changes after the probe but before the main tags set / samples read are planned, newly inserted series of the same metric can land outside the precomputed range; the id IN (...) subquery will include them, but the extra id >= ... AND id <= ... predicates will silently filter their samples out. Suggested fix: hold a metadata snapshot / lock that keeps the generator source immutable for the lifetime of the outer query, or disable the optimization whenever the generator source can change concurrently.

Final Verdict

Status: ❌ Block

1. Guard the optimization by samples-table key layout, not just id type.
2. Keep the id_generator proof valid until the main read snapshot is taken, or fall back to the old SQL when that metadata can change concurrently.

[clickhouse-gh]

This branch only checks that the selector's id set is a row-level subset of one continuous id range. It does not check that the samples target can actually use that range for primary-key pruning.

TimeSeries still supports custom and external samples tables with arbitrary engines / key layouts; normalizeTimeSeriesDefinition only validates the column types of external targets, and inline SAMPLES ENGINE = MergeTree ORDER BY ... is user-configurable. On a supported layout like ORDER BY (timestamp, id) (or any non-id primary-key prefix), this branch still sets use_index_for_in_with_subqueries_max_values = 1, so the existing id IN (...) pruning is disabled even though the new id range cannot constrain the key. Whole-metric selectors would then regress to timestamp-only or full-part scans.

This needs one more guard before enabling the optimization: require the samples target to have id as the leading primary-key / sorting-key column (or keep the IN eligible for index analysis otherwise). A regression test with a custom or external samples target would pin this down.

[clickhouse-gh]

📊 Cloud Performance Report

✅ AI verdict: no_change — no significant changes across 37 queries analysed

no significant changes detected. K_source=6 K_base=30 flagged=0/65

clickbench

🟢 No significant changes

tpch_adapted_1_official

🟢 No significant changes

Debug info

• StressHouse run: 053a0418-6678-4054-91c9-870eb4a26068
• MIRAI run: 4ccb2c3b-14b8-492f-a996-3d3a3e9d1533
• PR check IDs:
  • clickbench_961842_1786568820
  • clickbench_961848_1786568820
  • clickbench_961865_1786568820
  • tpch_adapted_1_official_961873_1786568820
  • tpch_adapted_1_official_961899_1786568820
  • tpch_adapted_1_official_961943_1786568821

[nikitamikhaylov]

@groeneai I'm not sure how to read the performance test results. Where are the results of promql_selector_pk_range? How does it compare to the master?

[groeneai]

There are no results for it: promql_selector_pk_range never ran. It crashes at parse time in both shards it was assigned to.

Where to look: the test landed on Performance Comparison (amd_release, master_head, 1/6) and (arm_release, master_head, 3/6), the two shards reporting FAIL. Each shard's report.html has a Run Errors table just above "Changes in Performance":

• amd 1/6
• arm 3/6

Both show the same thing:

promql_selector_pk_range | KeyError: 'dc=~".*"'
  perf.py line 342, in substitute_parameters
    values = [available_parameters[k] for k in keys]

perf.py runs every <query> through string.Formatter().parse() to expand <substitutions>, so the PromQL label matchers {dc=~".*"} (line 40 of the test) and {dc="0"} (line 44) are read as substitution placeholders and the lookup raises. That happens before connect, fill and run, so the whole file aborts and all four queries produce zero rows on both architectures. all-query-metrics.tsv of those two shards confirms it: no rows for the test.

That is also why nothing surfaced anywhere else. The CI summary table lists the two shards as FAIL but names only unrelated unstable queries, and the cloud-benchmark comment reports no_change because ClickBench and TPC-H never enter this code path.

The fix is to double the braces, which is what the 12 existing perf tests with literal braces do (concat_hits.xml, alter_select.xml and others):

'sum(pkr_metric{{dc=~".*"}})'
'sum(pkr_metric{{dc="0"}})'

With no substitutions declared, str.format turns {{ back into {, so the emitted SQL is byte-identical to what the file says today. I checked that both queries then parse to an empty key set and round-trip exactly.

Until that lands, the only measurements for this change are the tsbench numbers in the description, taken off-CI.

[nikitamikhaylov]

@groeneai suggest a diff for the fix.

[groeneai]

You already pushed exactly it in 96e5612 — the two <query> lines in tests/performance/promql_selector_pk_range.xml with doubled braces, nothing else. Verified at that ref: both queries now parse to an empty substitution key set, and str.format() returns the single-brace SQL byte-identical, so the queries sent to the server are unchanged. Removing one pair of braces reproduces the KeyError (dc="0").

Performance Comparison is still PENDING at that head, so the numbers should appear in shards amd_release 1/6 and arm_release 3/6 once it finishes.

[nikitamikhaylov]

Good improvement:

[clickhouse-gh]

Build profile diff (arm_release)

No arm_release build profile data for commit 097574b - the build was skipped, reused from cache, or predates profile upload.

[clickhouse-gh]

whole_metric_id_range_conditions stops being a pure index hint once it is added to the WHERE: it becomes part of the row-level filter, so the proof that "future inserts stay inside the range" has to remain true until the main read snapshot is taken.

TimeSeries still allows ALTER TABLE ... MODIFY SETTING id_generator after CREATE, and external tags targets can also change the id DEFAULT. If that metadata changes after this probe but before the main tags/samples reads are planned, newly inserted series for the same metric can get ids outside the precomputed range. The id IN (...) subquery will still return those ids, but the extra id >= ... AND id <= ... predicates added below will silently drop their samples.

This path needs a snapshot/lock that keeps the generator immutable for the lifetime of the query, or it needs to stay disabled whenever the generator source can change concurrently.

[nikitamikhaylov]

@groeneai investigate the test failures.

[groeneai]

None of them are caused by this PR. Current head 988ab5e2 has one red: BuzzHouse (amd_tsan), and it is a pre-existing trunk bug that reproduces on plain master in four lines.

BuzzHouse (amd_tsan) - Bad cast from type DB::ColumnVector<unsigned short> to DB::ColumnVector<int> (report)

The crashing query, from fatal.log plus server.log for query id a5718b05-2e80-449d-9ca7-3cb8e3e4f533, is INSERT INTO d4.t203 (c0, c2) FORMAT Values - a plain MergeTree insert. d4.t203 is PRIMARY KEY (modulo(c0, c0), c1, toMonday(c1)) with c1 DateTime64, and the fuzzer ran ALTER TABLE d4.t203 MODIFY SETTING ... with enable_extended_results_for_datetime_functions = 1 in the session. No TimeSeries table, no PromQL, no timeSeriesSelector anywhere in the fuzzer session (0 occurrences of any of those tokens in fuzzerout.sql or server.log).

This is issue #109181, which is still open. toMonday(DateTime64) returns Date with enable_extended_results_for_datetime_functions = 0 and Date32 with it on. KeyDescription::data_types is recomputed from the session context on any CREATE/ALTER, so a metadata-only ALTER run with the setting on flips the in-memory primary-key type to Date32/Int32 while the storage keeps producing the key column as Date/UInt16. The primary-index serializer is built from the metadata type, so it does assert_cast<ColumnVector<int>> on a ColumnVector<unsigned short> and aborts in calculateAndSerializePrimaryIndexRow.

Reproduced on pristine master (0eb5eba2, debug), no TimeSeries involved:

CREATE TABLE t1 (c0 Int32, c1 DateTime64, c2 Int32) ENGINE = MergeTree() PRIMARY KEY (toMonday(c1))
SETTINGS min_bytes_for_wide_part = 1;
SET enable_extended_results_for_datetime_functions = 1;
ALTER TABLE t1 COMMENT COLUMN c1 'a';
INSERT INTO t1 (c0, c1, c2) VALUES (1, '2020-01-01 00:00:00', 1);

Aborts with the same cast pair and the same frames as CI (SerializationNumber<int>::serializeBinary -> MergeTreeDataPartWriterOnDisk.cpp:235 calculateAndSerializePrimaryIndexRow -> :269 -> MergeTreeDataPartWriterWide::write -> MergeTreeDataWriter::writeTempPartImpl -> MergeTreeSink::consume). Removing the SET line makes it pass, so the setting is the trigger. The verbatim repro from #109181 aborts on the same binary too.

My fix for it is #109196 (pin the setting off when analyzing key and index expressions so the key type is deterministic). It is open and unreviewed, which is why this keeps firing.

Failures on the previous head c02c8632, all green now at 988ab5e2: Stress test (amd_asan_ubsan) was a hung check (898 hits across 371 PRs and 92 master runs in 7 days), and Stress test (arm_asan_ubsan, s3) plus Stress test (arm_tsan) were Sizes of nested column and null map of Nullable column are not equal after deserialization (STID 6726-6444 and 6726-4d8b; that signature family has 245 hits across 205 PRs and 15 master runs in 7 days). Both are wide pre-existing trunk signatures unrelated to StorageTimeSeriesSelector.

The PR's own tests are clean: 04836_time_series_selector_whole_metric_pk_range has zero failures in 7 days, and all 12 Performance Comparison shards are green.

[nikitamikhaylov]

@groeneai investigate the test failures and link (implement if needed) the PRs with fixes.

[groeneai]

Both reds at 97abf72a are pre-existing trunk failures, not from this diff. In each the crashing query is EXPLAIN indexes = 1 SELECT count() FROM test_table WHERE value = '300' from 03702_alter_column_modify_secondary_index_rebuild, AST-fuzzer-mutated; no frame in either stack touches TimeSeries or PromQL.

Check	Failure	Fixing PR
Stress test (amd_msan)	RWLockImpl::getLock(): Cannot acquire exclusive lock while RWLock is already locked (STID 2043-3c5c)	#114420, merged today 06:53Z
Stress test (arm_asan_ubsan, s3)	Sizes of nested column and null map of Nullable column are not equal after deserialization (STID 6726-6874)	#112484, open

First one: CREATE OR REPLACE ran its internal cleanup DROP on a context that was not marked DDL-internal, so ignore_drop_queries_probability rewrote that DROP to TRUNCATE, and the TRUNCATE branch takes an exclusive lock under the outer statement's query id while that id already holds read locks. #114420 marks those contexts internal and closes #110893. It is not an ancestor of this head, so a master merge picks it up.

Second one: a set skip index is deserialized from granules written with the pre-ALTER type. Here the fuzzer turned the test's MODIFY COLUMN value UInt64 into Nullable(UInt64), which is the case #112484's validation matrix records as producing this exact message. That PR replaces the read-time inference (is a mutation entry pending?) with a check of the part's own column types, in the same getDeserializedFormat frame this stack passes through, and closes #112213. It is unreviewed, which is why the signature is still ambient: 116 hits across 102 PRs plus 9 on master in the last 14 days, all under Stress test.

Neither needs new code from me.

[groeneai]

It is not failing on public CI: 1452 runs in 30 days, 0 failures, 0 above the cap. What you saw is the flaky check's wall-clock cap, Reason: Test runs too long (> 180s). Make it faster., not a result mismatch.

Durations for 04836_time_series_selector_whole_metric_pk_range, 250 runs per lane:

check	avg	max
Stateless tests (amd_asan_ubsan, flaky check)	66.5s	115.9s
Stateless tests (amd_debug, flaky check)	57.0s	105.8s
Stateless tests (amd_msan / amd_tsan, flaky check)	21.2s / 16.3s	46.7s / 28.4s

The slowest public lane is already at 115.9s of 180s, so any slower configuration crosses it.

The cost is DDL count, not one slow query: 5 ENGINE = TimeSeries tables, each materialising 3 inner MergeTree tables (Samples/Tags/Metrics), so ~15 inner CREATEs plus 15 DROPs, plus 24 selector evaluations, 8 EXPLAIN actions = 1 and 6 INSERTs. Round-trip bound per DDL, worse where table metadata sits behind a consensus store.

I would not shrink the matrix, since each fixture covers a distinct id layout your change branches on. They are independent though (each ts_* table is used only in its own block, the only shared state is two session SETs), so a split keeps all of it:

• 04836_...whole_metric_pk_range.sql keeps ts_clustered: SQL 16-81, reference 1-44. 16 of the 24 selectors, 1 TimeSeries table.
• a second file, e.g. 04872_time_series_selector_pk_range_id_layouts.sql, takes ts_plain, ts_custom_gen, ts_altered_gen, ts_u64: SQL 83-141, reference 45-61. 8 selectors, 4 TimeSeries tables.

That separates the two cost drivers. Both files keep -- Tags: no-fasttest, no-replicated-database, assertions stay byte-identical, and concatenating the two reference files in that order reproduces the current .reference byte-for-byte (44/45 is a section boundary). Same remedy as 605c7ef4235751e and fa156e8a020469f.

The one-liner alternative is -- Tags: ..., long. I would avoid it: it also cuts this test's flaky-check repeats from 50 to 5 and skips it on coverage lanes passing --no-long, buying the green with the coverage you just added.

Worth picking the number when you apply it, another PR may take 04872 first.