Skip to content

Query tracker rework - #34

Open
fernandodeluret wants to merge 19 commits into
mainfrom
fdeluret/acc-332b
Open

Query tracker rework#34
fernandodeluret wants to merge 19 commits into
mainfrom
fdeluret/acc-332b

Conversation

@fernandodeluret

@fernandodeluret fernandodeluret commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Query tracker rework: persistence-first, rate-based prioritization, latency guard

Summary

Full rewrite of the query tracker from an in-memory, volatile design into a
persistence-first service backed by a single index_patterns table. Every
input a decision needs now lives in Postgres, so the service is fully
restartable and never loses accumulated history. On that foundation this adds
cost/latency-aware prioritization, windowed rate scoring, a value-guarded
eviction pass, and a latency regression guard.

Motivation

The old tracker kept a 10k-entry queue plus a separate prioritized queue in
memory. Consequences: a restart dropped all accumulated demand; the !found
path re-attempted indexes that already existed (create/fail churn); ranking was
frequency-only; and nothing measured whether a created index actually helped.
This rework makes the tracker self-aware of its own state and drives creation
and eviction from one consistent, configurable criterion.

Core model

  • One durable table (index_patterns), one row per IndexIdentity. An
    identity is the index-defining subset of a getProgramAccounts request:
    program + memcmp (offset, length) columns + optional datasize. Everything
    else (commitment, encoding, data slice, and the memcmp values) is
    intentionally collapsed, so demand aggregates per index we would actually
    build
    , not per raw query. Removed the now-superseded auto_index_usage
    table.
  • pattern_id is a fixed-width blake3 hash of the identity (16 hex chars =
    64 bits), used both as the primary key and as the token embedded in physical
    index names. Postgres identifiers are capped at NAMEDATALEN − 1 = 63 bytes,
    and longer names are silently truncated (which would let two patterns
    collide). The hash keeps every generated name (idx_snapshot_accounts_<hash>
    ≈ 38 bytes) safely and deterministically under that limit — the same
    pattern always maps to the same row and the same index across restarts.
  • Human-readable everywhere. A separate human_name is used in logs,
    metrics and debug output; the opaque hash is confined to the DB key and index
    names.

Demand + supply, made comparable

  • Demand (API side) and supply (Postgres idx_scan) are tracked as two
    independent signals. A served GPA is a UNION ALL over accounts +
    snapshot_accounts, so it scans both indexes of a pair — raw idx_scan
    runs ≈2× the request count. Supply is normalized by SCANS_PER_REQUEST = 2
    before any demand/supply comparison (scoring and discrepancy).
  • Failed queries still count. Some patterns simply cannot be served without
    an index. Failures are recorded (using the timeout budget as the cost
    estimate) so an index can be built before / regardless of success, and
    failure-weight can prioritize exactly these patterns.

Prioritization — one criterion, four modes

Creation ranks candidates descending; eviction ranks the same expression
ascending — "what we most want to build" and "what we least mind dropping"
are two ends of one order. priority-mode selects how a row scores:

  • frequency — lifetime request count (demand_count).

  • cost — lifetime DB cost (total_cost_us).

  • cost-per-hit — average cost per request (total_cost_us / demand_count).

  • weighted — a flexible, tunable table that ranks on recent windowed
    activity plus the measured latency win, so decisions track current
    throughput rather than all-time popularity:

    score = (avg_cost · gain) · (1 + demand-weight·demand
                                   + supply-weight·(supply/2)
                                   + failure-weight·failed)
    

    where avg_cost = total_cost_us/demand_count, demand/supply/failed are
    per-rate-window rates, and gain = 1 + latency-weight·ln(without/with)
    scales cost by how much faster the index makes the pattern. All weights default
    0.0, which degenerates cleanly to plain average cost-per-hit (and needs no
    rate roll).

  • Windowed rates + cold-start bootstrap. A background roll task materializes
    per-window demand/supply/failure rates every rate-window; until a pattern's
    first roll, scoring falls back to its running total so a fresh pattern is still
    ranked within its first window.

Eviction — safe by construction

  • Only fires when the capped table crosses eviction-fill-threshold; below it,
    creation just slows instead of dropping anything.
  • A pattern must be idle by both supply and demand and older than
    index-min-age-grace, which prevents the drop→slow→rebuild churn loop.
  • Value guard: an idle index is dropped only if its score is strictly below
    the paired top creation candidate's; by monotonicity the pass stops at the
    first candidate that fails, so we never evict value we have nothing better to
    put in its place.
  • Drops honor the same DDL backpressure as creation (deferred, never discarded,
    when the indexer is under load) and run under a configurable
    drop-lock-timeout with drop-retries.

Latency regression guard

Cost is routed into with-index vs without-index buckets by status at record
time. An index older than index-min-age-grace that is slower with the
index than without (by index-regression-ratio) is flagged: warn logs and
reports a gauge; evict drops the pair and marks the pattern rejected. A
rejected pattern is not rebuilt by demand alone — only after
index-regression-retry-delay and fresh without-index evidence that the index
would now help. The same latency signal optionally feeds weighted via the
ln-smoothed gain term.

Supporting features

  • Variety (HyperLogLog). Distinct memcmp values per index are estimated
    cheaply with the hyperloglogplus crate; the client sends a bounded set of
    value fingerprints that the tracker folds into a per-row sketch. (We track the
    variety of index-relevant filter values; all other request dimensions are
    aggregated away.)
  • Example request per row. The representative RpcProgramAccountsConfig is
    stored once (example_request) so the optional EXPLAIN probe uses real
    filter values instead of zeros and runs on both tables; it is also shown on
    the debug views for documentation/sampling.
  • Discrepancy detection & EXPLAIN sampling are observational only: they
    flag/log when demand and idx_scan diverge past a configurable delta, or when
    the planner would not use an index, surfacing on /debug/* and /metrics.
  • HTTP, not JSON-RPC. A single POST /track batch ingest endpoint (a single
    query is a batch of length 1, so legacy callers reuse it) plus operational
    endpoints (/debug/candidates, /debug/created, /debug/discrepancies,
    /metrics, /health) on one port, split into endpoints/ (functional) and
    operational/ (introspection). Typed request/response structs shared in
    core.
  • Client-side buffering. The API aggregates observations per identity,
    dedups value fingerprints in a HashSet (bounded), and on a flush failure
    re-merges the batch instead of dropping it — demand is never silently lost.
  • Robustness & clarity. Every error path logs with a target; invalid stored
    program bytes surface an error instead of being silently skipped; large
    functions are split into small, documented modules (ingest, creation,
    eviction, store, prioritization, stats/*, server/*).

Configuration

priority-mode (with the weighted table: demand-weight, supply-weight,
failure-weight, latency-weight, rate-window), index-regression-guard
(off/warn/evict), index-regression-ratio, index-regression-retry-delay,
eviction-fill-threshold, drop-lock-timeout, drop-retries, plus the existing
eviction knobs. All documented in README.md and
example.cloudbreak.query-tracker.toml; defaults preserve prior behavior.

Edit:

Update: value guard moved to creation time (c9d3811)

Reworked the index value guard from eviction time to creation time. Previously eviction weighed each drop against the best pending creation (pairing eviction/creation candidates rank-for-rank) and stopped as soon as a swap wasn't worth it. That coupled two loops, left the table sitting above the fill target whenever no creation was queued, and — in practice — meant eviction never fired in staging because the table stayed below the hard cap and nothing was ever queued to justify a drop.

New model — the fill target is the operating size, the cap is the real ceiling:

  • Below the fill target (floor(eviction-fill-threshold × max-auto-indexes)): the top candidate is built freely.
  • In the buffer band (target, cap]: a new index is built only if it out-scores the index it would displace. That boundary is not the single worst index — since over = current − target indexes are already destined for the next trim, the candidate competes with the first index that would survive it (position over in ascending-score eviction order). Example (max=200, fill=0.7 ⇒ target 140): at current=140 (over=0) it must beat the single worst eligible index; at current=176 (over=36) the 36 worst are leaving anyway, so it must beat the 37th-worst.
  • Nothing reclaimable to displace (fewer eligible indexes than the overflow): build anyway up to the hard cap and let eviction reclaim later — don't stall creation behind indexes that aren't yet droppable.
  • At the cap: creation pauses; candidates stay queued.
  • Eviction is now an unconditional trim: drop the least-valuable eligible pairs (idle by both signals, past age grace) in ascending score order back down to the target. The value decision already happened at creation.

New config: value-guard-creation-bias (f64, default 1.0). Multiplies the candidate's score in the guard comparison to tune stickiness. A created index carries realized signal a fresh candidate can't (its latency gain and idx_scan supply), which structurally favors incumbents; >1.0 builds new indexes more readily (less sticky), <1.0 favors incumbents (stickier). Only consulted in the buffer band.

Notable changes

  • store: added eviction_boundary_score(...); factored the eligibility gate into a shared eviction_gate() so the eviction trim and the creation guard can't drift apart; removed the now-unused top_candidates wrapper.
  • creation: added value_guard_allows(...) (documents the over logic + stickiness).
  • eviction: removed the eviction-time guard (pending_creation_scores + paired-score stop), now trims unconditionally to target.
  • Docs: updated README, module docs (lib.rs, creation, eviction), and example.cloudbreak.query-tracker.toml; verified no leftovers from the old approach.

@Mctursh

Mctursh commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

read through the whole rework. this is a lot of careful engineering and most of it holds up. the scary numeric stuff is all guarded: every divisor is GREATEST(x,1) or the constant 2, ln only ever sees a CASE-guarded ratio ≥1, scores are NOT NULL so they never poison an ORDER BY, and the rust-side sorts use total_cmp. the identity collapse is exactly the set of inputs that define the physical index, the /2 supply normalization is applied once at every compare site and stored raw, and the store is genuinely restartable. the counter + HLL concurrency in record_demand is done right: atomic column = column + EXCLUDED, and the HLL read-modify-write holds the row lock to commit so merges can't be lost. the partition-tree idx_scan summing, the non-CONCURRENTLY drops under lock_timeout, the "skip the pass when track_counts is off" guard, and the reset-absorbing clamps all carry over faithfully from the old eviction path. a few things stood out, two are blockers.

blocker: deleting the m20260618 migration makes the migrator hard-fail on every existing deployment. sea-orm-migration records applied migrations by name in seaql_migrations and, in get_migration_with_status (migrator.rs:122-134 in 1.1.19), it computes migration_in_db - migration_in_fs and returns Err(DbErr::Custom("... has been applied but its file is missing")) when that's non-empty. get_pending_migrations routes through it and exec_up calls it before applying anything, so on any DB that already ran m20260618_create_auto_index_usage (base b54e259 is upstream/main, which ships it, so this is every deployment) the whole migrator errors on up/down/status, and index_patterns never gets created. the tracker then has no table. this is the same class as the #33 finding but harder-failing. the fix is the sea-orm-correct one: put m20260618 back untouched and add a new m2026xxxx_drop_auto_index_usage whose up is DROP TABLE IF EXISTS auto_index_usage, which clears the missing-file error and drops the now-orphaned table in one go (right now nothing drops it either).

idle eviction can run with its own master switch off. the eviction task now also hosts the regression guard, so it's spawned when index-eviction-enabled OR index-regression-guard != off (lib.rs:117-118). but run_pass gates the idle trim only on track_counts + a cap + the fill threshold, there's no index_eviction_enabled check anywhere in eviction.rs. so set index-regression-guard = warn (documented as observe-only, config.rs:809 "keep the index in place") with a max-auto-indexes cap and the capped table over the fill threshold, and the pass runs the guard and then falls straight through into the idle trim and drops index pairs, even though eviction is off. that contradicts both the master-switch doc (config.rs:890, "when off the eviction task is not spawned") and the warn contract. the old path gated the idle pass in the task body too (if !enabled return); the rework widened the spawn condition but didn't re-add that gate. a if !config.index_eviction_enabled { return Ok(()) } right after the regression guard restores it (leave the read-only supply/discrepancy refresh running, that part's harmless).

batch ingest can double-count demand on a partial failure. apply_batch (ingest.rs:31-70) commits each observation in its own record_demand transaction and ?-propagates the first db error, so on a mid-batch failure the prefix is already committed and track returns 500. the client treats any non-2xx as "keep everything" and re-buffers the whole chunk (query_tracker_client.rs:189-192), and since record_demand is purely additive, the committed prefix gets applied again on the next flush. a transient timeout inflates demand/cost for that prefix; a deterministic poison observation re-applies the prefix every flush and never drains. wrapping apply_batch in one transaction makes the whole-chunk retry idempotent, which is what the no-data-loss client design already assumes.

a rebuilt index reads as zero-supply right when it's most evictable. mark_created (store/mod.rs:210-222) resets last_idx_scan = 0 but leaves idx_scan_prev (and demand_prev) at their pre-eviction values. so after an evicted pattern is resurrected and rebuilt, the next roll_scores computes supply_rate = GREATEST(last_idx_scan - idx_scan_prev, 0) against a stale high baseline and reads 0 until the new index re-accumulates past the old scan count, which is exactly the window where, in weighted mode with supply-weight > 0, a fresh rebuild looks worthless and is first in line to be dropped again. drop→rebuild→drop churn. re-anchoring the three *_prev columns (and the supply baseline) in mark_created alongside last_idx_scan fixes it.

a create that half-succeeds leaks against the cap. create_pair (creation.rs:352-371) builds both index sides, then mark_created. if both DDLs succeed but mark_created errors, the physical pair exists while the row stays candidate; it counts toward max-auto-indexes (the cap counts physical indexes) but eviction only looks at created rows, so nothing ever reclaims it, and if demand later drops under the creation threshold it's never re-picked to retry the mark either. same shape if one side's CREATE INDEX succeeds and the other fails, leaving the succeeded side orphaned. a reconcile step (you already enumerate physical auto-indexes in read_auto_index_supply) that drops or adopts indexes with no created row would close both.

smaller: the weighted config is unguarded in two ways. the weights and compensation factor are {}-formatted straight into the score SQL, so a non-finite value from TOML (latency-weight = inf, ... = nan) renders as the bareword inf/NaN, which postgres reads as a missing column and fails every create/evict pass. and nothing constrains weights >= 0, so a negative weight silently flips the volume factor and inverts the ranking (highest-demand pattern sorts as least valuable). a finite + non-negative check at config load for the weighted-mode knobs turns both into a clear startup error instead of a silent runtime break.

none of this touches the core accounting or the served query path, and the carryover from the old tracker is otherwise faithful. the migration and the eviction master-switch are the two that block merge outright. the ingest double-count and the rebuild-supply baseline don't block it, but they skew the demand/supply signal the tracker makes decisions from, so they'll bite once it's actually creating and evicting indexes.

@fernandodeluret

Copy link
Copy Markdown
Contributor Author

read through the whole rework. this is a lot of careful engineering and most of it holds up. the scary numeric stuff is all guarded: every divisor is GREATEST(x,1) or the constant 2, ln only ever sees a CASE-guarded ratio ≥1, scores are NOT NULL so they never poison an ORDER BY, and the rust-side sorts use total_cmp. the identity collapse is exactly the set of inputs that define the physical index, the /2 supply normalization is applied once at every compare site and stored raw, and the store is genuinely restartable. the counter + HLL concurrency in record_demand is done right: atomic column = column + EXCLUDED, and the HLL read-modify-write holds the row lock to commit so merges can't be lost. the partition-tree idx_scan summing, the non-CONCURRENTLY drops under lock_timeout, the "skip the pass when track_counts is off" guard, and the reset-absorbing clamps all carry over faithfully from the old eviction path. a few things stood out, two are blockers.

blocker: deleting the m20260618 migration makes the migrator hard-fail on every existing deployment. sea-orm-migration records applied migrations by name in seaql_migrations and, in get_migration_with_status (migrator.rs:122-134 in 1.1.19), it computes migration_in_db - migration_in_fs and returns Err(DbErr::Custom("... has been applied but its file is missing")) when that's non-empty. get_pending_migrations routes through it and exec_up calls it before applying anything, so on any DB that already ran m20260618_create_auto_index_usage (base b54e259 is upstream/main, which ships it, so this is every deployment) the whole migrator errors on up/down/status, and index_patterns never gets created. the tracker then has no table. this is the same class as the #33 finding but harder-failing. the fix is the sea-orm-correct one: put m20260618 back untouched and add a new m2026xxxx_drop_auto_index_usage whose up is DROP TABLE IF EXISTS auto_index_usage, which clears the missing-file error and drops the now-orphaned table in one go (right now nothing drops it either).

idle eviction can run with its own master switch off. the eviction task now also hosts the regression guard, so it's spawned when index-eviction-enabled OR index-regression-guard != off (lib.rs:117-118). but run_pass gates the idle trim only on track_counts + a cap + the fill threshold, there's no index_eviction_enabled check anywhere in eviction.rs. so set index-regression-guard = warn (documented as observe-only, config.rs:809 "keep the index in place") with a max-auto-indexes cap and the capped table over the fill threshold, and the pass runs the guard and then falls straight through into the idle trim and drops index pairs, even though eviction is off. that contradicts both the master-switch doc (config.rs:890, "when off the eviction task is not spawned") and the warn contract. the old path gated the idle pass in the task body too (if !enabled return); the rework widened the spawn condition but didn't re-add that gate. a if !config.index_eviction_enabled { return Ok(()) } right after the regression guard restores it (leave the read-only supply/discrepancy refresh running, that part's harmless).

batch ingest can double-count demand on a partial failure. apply_batch (ingest.rs:31-70) commits each observation in its own record_demand transaction and ?-propagates the first db error, so on a mid-batch failure the prefix is already committed and track returns 500. the client treats any non-2xx as "keep everything" and re-buffers the whole chunk (query_tracker_client.rs:189-192), and since record_demand is purely additive, the committed prefix gets applied again on the next flush. a transient timeout inflates demand/cost for that prefix; a deterministic poison observation re-applies the prefix every flush and never drains. wrapping apply_batch in one transaction makes the whole-chunk retry idempotent, which is what the no-data-loss client design already assumes.

a rebuilt index reads as zero-supply right when it's most evictable. mark_created (store/mod.rs:210-222) resets last_idx_scan = 0 but leaves idx_scan_prev (and demand_prev) at their pre-eviction values. so after an evicted pattern is resurrected and rebuilt, the next roll_scores computes supply_rate = GREATEST(last_idx_scan - idx_scan_prev, 0) against a stale high baseline and reads 0 until the new index re-accumulates past the old scan count, which is exactly the window where, in weighted mode with supply-weight > 0, a fresh rebuild looks worthless and is first in line to be dropped again. drop→rebuild→drop churn. re-anchoring the three *_prev columns (and the supply baseline) in mark_created alongside last_idx_scan fixes it.

a create that half-succeeds leaks against the cap. create_pair (creation.rs:352-371) builds both index sides, then mark_created. if both DDLs succeed but mark_created errors, the physical pair exists while the row stays candidate; it counts toward max-auto-indexes (the cap counts physical indexes) but eviction only looks at created rows, so nothing ever reclaims it, and if demand later drops under the creation threshold it's never re-picked to retry the mark either. same shape if one side's CREATE INDEX succeeds and the other fails, leaving the succeeded side orphaned. a reconcile step (you already enumerate physical auto-indexes in read_auto_index_supply) that drops or adopts indexes with no created row would close both.

smaller: the weighted config is unguarded in two ways. the weights and compensation factor are {}-formatted straight into the score SQL, so a non-finite value from TOML (latency-weight = inf, ... = nan) renders as the bareword inf/NaN, which postgres reads as a missing column and fails every create/evict pass. and nothing constrains weights >= 0, so a negative weight silently flips the volume factor and inverts the ranking (highest-demand pattern sorts as least valuable). a finite + non-negative check at config load for the weighted-mode knobs turns both into a clear startup error instead of a silent runtime break.

none of this touches the core accounting or the served query path, and the carryover from the old tracker is otherwise faithful. the migration and the eviction master-switch are the two that block merge outright. the ingest double-count and the rebuild-supply baseline don't block it, but they skew the demand/supply signal the tracker makes decisions from, so they'll bite once it's actually creating and evicting indexes.


Thanks for taking the time for reading through it, as you can see this a big topic and is still a work in progress, there are many areas to polish, I expect more updates coming overall to the score related part once we collect more data out of this 1st version. Regarding your comments:

blocker: deleting the m20260618 migration makes the migrator hard-fail on every existing deployment.

This is a similar comment that in this PR #33, with the same response. We added this clarification already in the docs https://github.com/solana-rpc/cloudbreak/blob/main/README.md?plain=1#L180-L188

idle eviction can run with its own master switch off.

You are right, this is something I didn't realize, I'm changing the behavior, the master switch controls everything now 97a205e

a rebuilt index reads as zero-supply right when it's most evictable.

You are right about this, this is a better more fair measure of the index being recreated, implemented here c54c62c (for more context, in general this is just the 1st version but scores will require more updates after this 1st iteration probably)

batch ingest can double-count demand on a partial failure

a create that half-succeeds leaks against the cap

For these 2 I'd argue that the are errors that I would prefer not to handle particularly (maybe only logging is too weak of a response but I don't think they deserve to kill the whole system, so at least logs keep them visible), they are unexpected cases that would require further investigation if they happen

smaller: the weighted config is unguarded in two ways

Agreed this could have a better cheking to don't allow crazy inputs (maybe you can indulge me with this for another PR)

@Mctursh

Mctursh commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the detailed response, and I understand it being a 1st iteration with more score tuning to come.

on the migration: fair. i was reading it as a persisted-db upgrade, but with the ephemeral-db / unlogged-tables design and a fresh schema each run that's the wrong frame. withdrawn.

checked the master-switch and mark_created changes, both look right.

on the two i pushed on:

the create-half-succeeds leak, agreed, it already logs on the mark_created failure, so investigate-if-it-happens is reasonable.

the batch double-count i'd frame a little differently. it's not about killing the system, and the catch is that it's silent: the double-apply lands on the retry, so nothing logs at the point the counters inflate (you get the re-buffering warning, but nothing saying the prefix already committed). one transaction around apply_batch doesn't add a failure mode, it just makes the whole-chunk retry idempotent, which is what the client's re-buffer already assumes. not blocking, your call, just didn't want it landing as an error-handling thing when it's really a consistency one.

i can take the weighted-config validation as a separate PR if you want it off your plate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants