morningman commented on issue #65423:
URL: https://github.com/apache/doris/issues/65423#issuecomment-4933938651
Two suggestions on where this feature should go beyond fixing the gaps
listed above, plus one item @morningman is taking.
## 1. Expose the counters as `information_schema` tables rather than `SHOW`
statements
`SHOW QUERY STATS` today has five hard-coded output shapes, each with its
own `ShowResultSetMetaData` and its own `QueryStatsUtil` entry point. None can
be filtered, sorted, joined, or aggregated. The questions users actually have
are all one `WHERE` clause away, and none is expressible:
- *"every column in this database with `filter_count > 100` and `query_count
= 0`"*
- *"join against `information_schema.columns` to find wide `STRING` columns
nobody reads"*
- *"which rollups have served zero queries"*
Moving the data into system tables makes all of that ordinary SQL and lets
the `SHOW` statements survive as thin views. It also matches the broader
direction of consolidating ad-hoc `SHOW` commands into queryable metadata.
### The two grains
The counters have two natural grains, and conflating them is the mistake to
avoid. An object's `query_count` is "how many queries touched this object" —
one increment per query, held today in `IndexStats.queryHit`. A column's
counters are per-column booleans folded into that same query. A table's count
is therefore **not** derivable by summing its columns; it is the sum over its
indexes. So: one table per grain.
**`information_schema.column_query_stats`** — one row per `(frontend,
catalog_name, table_schema, table_name, index_name, column_name)`, with
`query_count` and `filter_count`.
**`information_schema.object_query_stats`** — one row per `(frontend,
catalog_name, table_schema, table_name, table_type, index_name,
partition_name)`, with `query_count`. `index_name` is `NULL` for external
tables; `partition_name` is `NULL` when the table is unpartitioned.
**`information_schema.query_stats_status`** — one row per frontend:
`enabled`, `stats_since`. Small, but it closes a real hole: the counters
accumulate since FE start or the last `CLEAN`, with no window and no decay, and
today nothing tells you which. It also turns the most common failure — one FE
left at `enable_query_hit_stats = false` because someone used `ADMIN SET
FRONTEND CONFIG` instead of `ADMIN SET ALL FRONTENDS CONFIG` — into a visible
row rather than mysteriously low numbers.
Note that `column_query_stats` deliberately carries **no** `partition_name`.
Whether a column is filtered on is a property of the plan, not of a partition;
pushing that dimension down to column grain multiplies rows by the partition
count for a signal nobody has asked for. Partition hotness lives in
`object_query_stats`; column hotness lives in `column_query_stats`.
### Row-count budget, and why a pre-filter is mandatory
This scale question decides the design rather than being an afterthought.
Take 10,000 tables × 10,000 partitions: `object_query_stats` is 10<sup>8</sup>
rows at full materialization. `column_query_stats` is tables × indexes ×
columns — 10,000 tables with 100 columns and 3 indexes is another
3×10<sup>6</sup>. A schema-table scan serializes those rows over thrift from FE
to BE. Naively that is tens of gigabytes of `TRow` for a query whose answer is
one line.
The good news: **both mechanisms this needs already exist in the
schema-table framework.** They have simply never been treated as a framework.
**(a) Predicate pushdown into the FE generator.** `SchemaScanNode` already
serializes arbitrary conjuncts into
`TSchemaTableRequestParams.frontend_conjuncts` (thrift field 7), and
`FrontendConjunctsUtils.convertToExpression()` / `isFiltered()` let a generator
evaluate them **per candidate row before serializing it**. `sql_block_rule` and
`view_dependency` already use this. So
```sql
SELECT * FROM information_schema.object_query_stats
WHERE table_schema = 'test_db' AND table_name = 'orders';
```
can be answered by walking only that table's `IndexStats`, never
materializing the other 9,999.
The critical design consequence: **the generator must consult the conjuncts
while walking the in-memory `catalogStats` tree, not after building the full
row list.** Filtering after materialization buys nothing — the cost being
avoided is row construction and serialization, not the network.
To make that reliable rather than best-effort, the leading columns should be
exactly the ones that prune the walk: `catalog_name` → `table_schema` →
`table_name` → `index_name` → `partition_name`. Those map onto the
`catalogStats → dataBaseStats → tableStats → indexStats` nesting, so an
equality predicate on any prefix short-circuits a level of the traversal.
Predicates on `query_count` / `filter_count` cannot prune the walk and are just
row filters.
**(b) FE fan-out already has a working precedent.**
`MetadataGenerator.forwardToOtherFrontends()` does exactly this, driven by
`TSchemaTableRequestParams.replay_to_other_fe`.
`information_schema.active_queries` uses it: the BE scanner sets the flag
(`be/src/information_schema/schema_active_queries_scanner.cpp:63`), the
receiving FE produces its own rows, then re-sends the request with the flag
cleared to every peer and concatenates the results.
That is precisely the shape Query Stats needs — each FE only counts the
queries it planned itself, so any read must fan out and merge. It means the
work is *generalizing an existing mechanism*, not inventing one. Today, by
contrast, every feature that needs a cross-FE merge hand-rolls it against
`ClientPool.frontendPool`: `QueryStatsUtil.getStats()` (with its own
per-`TQueryStatsType` merge branches), `ShowProcessListCommand`,
`StatisticsCache`, `FollowerColumnSender`, `FESessionMgr`.
Two things to fix while generalizing:
- `forwardToOtherFrontends()` swallows per-FE failures with a `LOG.warn` and
continues — the same silent undercount `QueryStatsUtil` has today. With
`frontend` as a real column, an unreachable FE becomes a *missing row*, which a
join against `query_stats_status` makes visible. Worth preserving deliberately
rather than inheriting by accident.
- `frontend_conjuncts` must be forwarded along with the relayed request, so
peers prune too. Otherwise the pre-filter saves the coordinating FE's work and
none of the others'.
**(c) When pushdown is absent, degrade loudly rather than silently.** For a
bare `SELECT * FROM information_schema.object_query_stats` on a
10<sup>8</sup>-row cluster the options are: stream in batches, cap and warn, or
refuse. I'd argue for a **required-predicate rule** on `object_query_stats`
once the estimated row count exceeds a threshold — reject with "add a
`table_schema` or `table_name` predicate" — because the alternative is an
unbounded FE-side allocation the user cannot cancel. `column_query_stats` and
`query_stats_status` are small enough to serve unfiltered.
Batching is the complementary answer if unfiltered scans must work at all.
`TFetchSchemaTableDataResult` returns a single `dataBatch`, and
`TSchemaTableRequestParams` has no offset/limit today, so streaming needs a
cursor field added. That is strictly more work than pushdown and helps only the
unfiltered case, so it should follow pushdown rather than precede it.
### What this buys
With `frontend` as a real column and pushdown into the walk, summation moves
out of Java and into the SQL layer:
```sql
-- Today's `SHOW QUERY STATS FROM t ALL`, but now filterable and joinable
SELECT index_name, SUM(query_count)
FROM information_schema.object_query_stats
WHERE table_schema = 'test_db' AND table_name = 'stats_table'
GROUP BY index_name;
-- Cold partitions, pruned at the FE before a single row is serialized
SELECT partition_name, SUM(query_count) AS hits
FROM information_schema.object_query_stats
WHERE table_schema = 'ods' AND table_name = 'orders'
GROUP BY partition_name HAVING hits = 0;
-- Wide string columns nobody ever reads
SELECT s.table_name, s.column_name, c.data_type
FROM information_schema.column_query_stats s
JOIN information_schema.columns c
ON c.table_schema = s.table_schema
AND c.table_name = s.table_name
AND c.column_name = s.column_name
WHERE c.data_type IN ('string', 'varchar')
GROUP BY s.table_name, s.column_name, c.data_type
HAVING SUM(s.query_count) = 0;
-- Prefix-key candidates: heavily filtered, rarely projected
SELECT column_name, SUM(filter_count) AS f, SUM(query_count) AS q
FROM information_schema.column_query_stats
WHERE table_schema = 'ods' AND table_name = 'orders'
GROUP BY column_name ORDER BY f DESC;
```
The five `QueryStatsUtil` merge branches, one per `TQueryStatsType`,
disappear. The `SHOW` statements can survive as views for compatibility.
## 2. Statistics dimensions: table / index / partition / MV / column
Correcting the current state, because it is easy to misread:
| Dimension | Status | Evidence |
|---|---|---|
| Table | supported | `StatsDelta` keyed by `catalogId_dbId_tableId_indexId`
|
| Column | supported | `IndexStats.columnQueryStats` / `columnFilterStats` |
| **Index (ROLLUP)** | **already supported** | The delta key includes
`PhysicalOlapScan.getSelectedIndexId()`; `SHOW QUERY STATS FROM t ALL
[VERBOSE]` breaks down by index name via `OlapTable.getIndexNameById()`. A
**sync MV is an index**, so it rides the same path. |
| **MTMV (async MV)** | **recorded — but the attribution is wrong** | `MTMV
extends OlapTable`, and `BindRelation` binds `TableType.MATERIALIZED_VIEW`
through `makeOlapScan()` → `PhysicalOlapScan`. An MTMV scan *is* counted, under
the MTMV's own table id. The problem is **transparent rewrite**: the base
table's scan is replaced by the MTMV's, so the base table's columns record
nothing and the table looks cold precisely because it is being queried
successfully. |
| **Partition** | **missing, and never existed** |
`PhysicalOlapScan.getSelectedPartitionIds()` is right there at plan time. Note
#18805's original `StatsDelta` had no partition dimension either — only
catalog/db/table/index plus replicas. |
| Tablet / replica | dead code | Distinct from partition: the old
`OlapScanNode.genStatsDelta()` passed `scanReplicaIds`, and replica selection
under Nereids happens after fragment assignment, so this cannot be a plan-tree
hook. |
Two concrete asks follow.
**Add the partition dimension.** The ids are already on the scan node, so
the recorder change is small. The cost is cardinality: a table with thousands
of partitions multiplies its counter entries accordingly, and today those
counters are an unbounded `ConcurrentHashMap` in `Env` with no TTL. That is an
independent argument for suggestion 1 — a schema table backed by an explicitly
managed structure, rather than a map that only ever grows.
**Decide the MTMV attribution semantics.** When a query against a base table
is transparently rewritten onto an MTMV, I'd argue the base table's columns
should still record `queryHit`: the user's intent was to read those columns,
and the entire purpose of the counter is to tell you which columns matter for
schema design. The MTMV should of course also record its own hit, so `WHERE
table_type = 'MTMV'` answers "which MVs are earning their keep." Under the
schema above both rows coexist naturally; under the current `SHOW`-only model
there is nowhere to put them.
## 3. External tables (@morningman will take this)
Currently out of scope by design — `walkPlan` recognizes only
`PhysicalOlapScan`. Worth noting that the structural work is smaller than it
looks: `PhysicalOlapScan`, `PhysicalFileScan`, and `PhysicalOdbcScan` all
extend **`PhysicalCatalogRelation`**, which already exposes `getTable()` and
`getDatabase()` — the two things `getOrCreateDelta()` needs. `PhysicalFileScan`
even carries `selectedPartitions`, so Hive/Iceberg partition hotness lands in
the same `object_query_stats` shape proposed above.
Two things fall out once external tables are in:
- The `index_name` column becomes genuinely nullable, which retires the
`TableStats.DEFAULT_INDEX_ID = -1` sentinel still lurking in the read path
(`QueryStats.getTblStats()`) as leftover from #18805.
- `catalog_name` stops being decorative. Today every row is the internal
catalog; the delta key already carries `catalogId` and the `SHOW` statements
already accept a catalog level, so the plumbing exists and has simply never had
a second catalog to hold.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
