Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 6525149 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Duplicate code analysis1 new clone introduced by this PR.
If one of these duplicates a helper that already exists, import it instead jscpd compared this branch against |
E2E Test Results✅ All tests passed • 464 passed • 1 skipped • 1130s
Tests ran across 4 shards in parallel. |
64ef09d to
7fae5c3
Compare
Summary
"All values" lookups that read a text index now return each key's most frequent values with
topKWeighted, and run in parallel batches of at most 10 keys, so a key with very many values delays only its own batch.Stacked on #3614.
The problem
getMapTextIndexKeyValues(Map key=value indexes, e.g.idx_log_attr_items) andgetTextIndexKeyValues(text indexes on plain columns, e.g.TraceId):groupUniqArray(limit)over a sorted dictionary), not the common ones.read_overflow_mode: 'break') also stops the sorted read partway, so keys that sort after a high-cardinality key came back empty.The fix
topKWeighted(limit)(value, cardinality).cardinalityis the token's row count in the part, so the values are the most frequent, most frequent first.limitis inlined as a literal like the other parametric aggregates in this file.TEXT_INDEX_KEYS_PER_QUERY), and plain-column text indexes into queries of at most 10 columns. They already stream in as each query finishes. Materialized-view and raw-table lookups keep batches of 100, because each raw-table batch scans the table.What I tried and dropped: one branch per key
Giving each key its own
UNION ALLbranch withLIMIT 100000cut tokens read about 12× on a local benchmark. On ClickHouse Cloud it was much slower. Each branch reads the index of every part separately, and an open has a fixed cost no matter how few tokens it reads. On a 26-partotel_logstable with a warm cache:system.partslookup aloneThat's about 0.18 CPU-s and 10–30 MiB of filesystem-cache reads per part per open, apparently because
mergeTreeTextIndex()loads and decodes a per-part structure sized to the whole dictionary, and doesn't use the text index header cache. With per-key branches, a page load did about (keys × parts) ≈ 2,600 opens instead of (Map columns × parts × batches), and the server ran out of CPU. The fixed cost per open should be raised upstream.Cost and known limits
main. Batches of 10 mean up to 10× more queries, and each query opens every part of the index once per Map column. That's still far fewer opens than per-key branches. The batch size trades total CPU against how long one slow key can hold up the others.topKWeighteduses about 1.6× the CPU per token ofgroupUniqArray. Memory is the same.How to reproduce
Starting point:
yarn devagainst ClickHouse 26.3+, with the OTelotel_logsschema (key=value text indexesidx_log_attr_items,idx_res_attr_items), and a Map key with one rare value that sorts first and one common value, e.g.map('freq.key', if(number = 0, 'a-rare', 'z-common'))fromnumbers(4).On
main:freq.keylistsa-rarefirst, andsystem.query_logshows one values query for all keys.On this branch:
freq.keylistsz-commonfirst, and there is one values query per 10 keys.References