Bound the cycle, not just the index, in pg_index_bloat (#2617) - #2618
Conversation
Dogfooding v99 on the Aurora target showed this collector had never returned a single row: rows_ever = 0, failing every cycle with "Exception while reading from stream". I bounded the wrong dimension. pgstatindex reads every page of the index it is pointed at, and MeasureCeilingBytes bounds one INDEX while nothing bounded the STATEMENT. On that target: 1,517 indexes totalling 461 GB in a single query on the default command timeout. It never finished and the connection dropped mid-read. A cycle work budget now measures the 200 largest indexes per run. A count rather than a byte cap, because the cost is per page and index sizes are wildly uneven - a byte cap measures three indexes on one server and four hundred on another, and no operator could predict which they were getting. Ranked largest-first, because bloat worth reclaiming is concentrated in big indexes. Everything past the budget is still RETURNED with skipped_reason, never dropped. An index missing from the result reads as one that does not exist; an index present with a stated reason cannot be mistaken for healthy. Same argument that put skipped_reason on the size ceiling. The budget gates the LATERAL, not just the label - gating only the reason would name rows correctly while still reading every page. CommandTimeoutSecondsOverride = 300, so a slow single index yields a classified timeout rather than the unclassified stream failure this surfaced as. index_object_stats took the same override for the same reason (#1135). Verified against 212 real indexes: 200 measured, 12 returned with the reason, none dropped. The rig that verified #2561 had two indexes on one table, so the question of total work never arose. That is the class of defect only a real instance shows. Fixes #2617. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| WHEN k.size_rank > " + BudgetLiteral + @" | ||
| THEN 'not measured this cycle (work budget): pgstatindex reads every page, so only the ' | ||
| || 'largest ' || " + BudgetLiteral + @" || ' indexes are measured per run. This one is ' | ||
| || 'recorded at its size so it is never mistaken for healthy.' | ||
| END::text AS skipped_reason | ||
| FROM candidates AS k | ||
| FROM ranked AS k | ||
| LEFT JOIN LATERAL public.pgstatindex(k.index_oid::regclass) AS s | ||
| ON k.index_bytes < " + CeilingLiteral + @" | ||
| ON k.index_bytes < " + CeilingLiteral + @" | ||
| AND k.size_rank <= " + BudgetLiteral + @" | ||
| ORDER BY k.index_bytes DESC"; | ||
|
|
||
| private const string CeilingLiteral = "21474836480"; | ||
|
|
||
| /* How many indexes one cycle will actually MEASURE, largest first. 200 rather than a byte budget | ||
| because the cost is per PAGE and the sizes are wildly uneven: a byte cap would measure three | ||
| large indexes on one server and four hundred small ones on another, and neither operator could | ||
| predict what they were getting. A count is legible, and the ORDER BY means the 200 measured are | ||
| always the ones where bloat is worth reclaiming. */ | ||
| private const string BudgetLiteral = "200"; |
There was a problem hiding this comment.
The work budget is deterministic and largest-first with no rotation across cycles: size_rank is recomputed from index_bytes DESC every run, so an index that lands at rank 201+ today will land there again tomorrow (index sizes don't reorder that fast). For any database with more candidate btree indexes than the budget, everything past rank 200 is not merely "not measured this cycle" as the message says — it is never measured, permanently, until enough of the top 200 shrink or get dropped.
That compounds with the reader/UI, which this PR doesn't touch but whose assumptions this collector now breaks:
DarlingPgIndexBloatReader.PgIndexBloatSqlsorts(skipped_reason IS NOT NULL) DESCfirst — written when skip was the rare over-20GB-ceiling case, so surfacing it first made sense ("skipped indexes sort to the top, not out of sight").ViewerServerTab.Postgres.cscalls it withPgGridRowLimit = 200— the exact same number asBudgetLiteral.
So on any database with more than ~400 qualifying btree indexes, every skipped row (i.e. every index below rank 200, always the same ones) sorts ahead of every measured row, and the 200-row grid limit is entirely consumed by "not measured this cycle" placeholders. The actually-measured bloat data for the top 200 largest indexes — the whole point of this collector — never appears in the UI at all for those databases, and the smaller/mid-sized indexes where bloat is easiest to miss are permanently invisible too.
Worth considering a rotating or sampled budget (e.g. offset by collection cycle, or a random sample from beyond the top N) so coverage eventually reaches every index, and/or fixing the skipped_reason wording so it doesn't promise something ("this cycle") the current implementation can't deliver.
|
Reviewed. The fix itself is sound: bounding the statement (not just the per-index ceiling) is the right diagnosis for the 461 GB / 1,517-index Aurora failure, Lite/Darling parity: no drift. One substantive design issue, left as an inline comment on No SQL injection, secrets, or missing-index-DMV concerns; T-SQL style guide doesn't apply here (Postgres collector), and the embedded-SQL conventions used match the rest of the file. |
Found by dogfooding v99 on the Aurora target. This collector had never returned a single row since it shipped —
rows_ever = 0, failing every cycle withException while reading from stream.I bounded the wrong dimension
pgstatindexreads every page of the index it is pointed at — that is what makes it a measurement rather than an estimate, and why #2561 chose it over an estimator that returns nothing underpg_monitor.MeasureCeilingBytesbounds one index. Nothing bounded the statement:One query, 461 GB of index pages, default command timeout. It never finished and the connection dropped mid-read — which is also why it surfaced as an unclassified stream failure rather than a named miss.
The fix
A cycle work budget: measure the 200 largest indexes per run.
A count, not a byte cap. The cost is per page and index sizes are wildly uneven, so a byte cap would measure three indexes on one server and four hundred on another with no way for an operator to predict which. A count is legible, and largest-first means the 200 measured are where bloat is actually worth reclaiming.
Everything past the budget is still returned, carrying
skipped_reason:An index missing from the result reads as one that does not exist. An index present with a stated reason cannot be mistaken for healthy — the same argument that put
skipped_reasonon the size ceiling.The budget gates the LATERAL, not just the label. Gating only the reason would name the rows correctly while still reading every page — which would have fixed nothing.
CommandTimeoutSecondsOverride = 300so a slow single index yields a classified timeout instead of a dropped connection;index_object_statstook the same override for the same reason (#1135).Verified
Against 212 real btree indexes:
Worth saying plainly
The rig that verified #2561 had two indexes on one table. The measurement logic was correct and the question how much work is this in total never arose, because the answer was none. That is the class of defect a container structurally cannot show you, and it is the argument for this dogfood pass.