From c25d518418971788633b9687f7002c81875a0aed Mon Sep 17 00:00:00 2001 From: James Kane Date: Wed, 29 Jul 2026 07:01:29 -0500 Subject: [PATCH] perf(str): precompute the marker report instead of aggregating per request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /str-markers page aggregated every stored profile on every request: ~1.9s over the 876-profile dev corpus, growing linearly, and worse in production. Profiling put the cost somewhere indexes cannot reach. The JSONB expansion is 36ms for 514k observations, and the two reference joins — the only index-servable part — are 13ms. The remaining ~1.84s is aggregate machinery: mode() WITHIN GROUP, three ordered array_agg()s and two DISTINCT counts, all over every expanded observation. Raising work_mem changed nothing despite a 52MB spill; dropping both DISTINCT aggregates reached 1.39s; pre-aggregating by distinct value before the ordered-set aggregates reached 1.45s. None of that is enough, and none of it stops the growth. So the rollup is precomputed into genomics.str_marker_stat, refreshed by `du-jobs run-once str-marker-stats`, following the coverage-norms / discovery-consensus / tree-samples-recompute pattern already in this crate. A plain table rather than a MATERIALIZED VIEW: the schema uses none, and the refresh belongs in the job's transaction. Read path is now 0.94ms and the page serves in 13ms, independent of corpus size. The refresh is a full replace in one transaction — a single new profile can move a marker's min, max and mode at once, so there is no incremental form, and readers see one whole snapshot or the other. It also runs automatically after an applied `ftdna-str` import, since a bulk load invalidates every row. marker_stats falls back to computing live while the table is empty, so the report is correct in the window between deploying this migration and the first refresh — slow, and it says so in the log rather than going quietly blank. The report now displays when it was last recomputed, because the figures are no longer live. The integration test asserts the live and precomputed paths return identical rows, so the two definitions cannot drift. Co-Authored-By: Claude Opus 5 (1M context) --- rust/crates/du-db/src/ystr.rs | 80 ++++++++++++++++--- rust/crates/du-db/tests/str_marker_stats.rs | 24 +++++- rust/crates/du-jobs/src/main.rs | 15 +++- rust/crates/du-web/src/routes/str_markers.rs | 9 +++ rust/crates/du-web/templates/str/markers.html | 1 + rust/locales/en.txt | 1 + rust/locales/es.txt | 1 + rust/locales/fr.txt | 1 + rust/migrations/0071_str_marker_stat.sql | 64 +++++++++++++++ 9 files changed, 185 insertions(+), 11 deletions(-) create mode 100644 rust/migrations/0071_str_marker_stat.sql diff --git a/rust/crates/du-db/src/ystr.rs b/rust/crates/du-db/src/ystr.rs index 9387a26..8aa46a1 100644 --- a/rust/crates/du-db/src/ystr.rs +++ b/rust/crates/du-db/src/ystr.rs @@ -1124,9 +1124,7 @@ pub struct MarkerStat { pub age_model_status: String, } -/// Corpus-wide observed range per Y-STR marker: min, modal and max across every -/// profile we hold, joined to the marker registry (motif/period) and the active -/// mutation rate. Ordered by observation count, descending. +/// The corpus-wide aggregation behind the marker report, computed from scratch. /// /// Both profile sources feed it — the federated mirror and the locally-imported /// vendor exports — the same union [`build_str_inputs`] uses, minus the tree join @@ -1136,9 +1134,11 @@ pub struct MarkerStat { /// zero-repeat count, so it is excluded from min/modal/max and counted in /// `null_alleles`; without that, DYS448 reads min 0 instead of 17. The same rule /// applies per copy of a multi-copy vector. -pub async fn marker_stats(pool: &PgPool) -> Result, DbError> { - let rows = sqlx::query_as::<_, MarkerStat>( - "WITH obs AS ( \ +/// +/// Expensive — ~1.9s over 876 profiles, growing linearly with the corpus, in the +/// aggregates rather than the JSONB expansion (see migration 0071). Callers want +/// [`marker_stats`], which reads the precomputed table; this runs on refresh. +const MARKER_STATS_AGGREGATION: &str = "WITH obs AS ( \ SELECT p.sample_guid::text AS sid, e->>'marker' AS marker, e->'value' AS v \ FROM genomics.biosample_str_profile p, jsonb_array_elements(p.markers) e \ UNION ALL \ @@ -1206,11 +1206,73 @@ pub async fn marker_stats(pool: &PgPool) -> Result, DbError> { OR mr.marker_name = ANY(COALESCE(m.aliases, '{}'))) \ ORDER BY (mr.marker_name <> a.marker) LIMIT 1 \ ) r ON true \ - ORDER BY a.observations DESC, a.marker", - ) + ORDER BY a.observations DESC, a.marker"; + +/// Column list shared by the refresh insert and the read path, in table order. +const MARKER_STAT_COLUMNS: &str = "marker_name, multi_copy, observations, samples, \ + min_value, modal_value, max_value, min_combination, modal_combination, max_combination, \ + distinct_values, null_alleles, complex_count, motif, period, coordinates, \ + mutation_rate, rate_ci_low, rate_ci_high, rate_method, rate_source, age_model_status"; + +/// Recompute `genomics.str_marker_stat` from every stored profile. Returns the +/// number of marker rows written. +/// +/// A full replace inside one transaction: the corpus summary has no incremental +/// form (a single new profile can move a marker's min, max and mode at once), and +/// readers either see the whole previous snapshot or the whole new one. +pub async fn refresh_marker_stats(pool: &PgPool) -> Result { + let mut tx = pool.begin().await?; + sqlx::query("DELETE FROM genomics.str_marker_stat").execute(&mut *tx).await?; + let n = sqlx::query(&format!( + "INSERT INTO genomics.str_marker_stat ({MARKER_STAT_COLUMNS}) {MARKER_STATS_AGGREGATION}" + )) + .execute(&mut *tx) + .await? + .rows_affected(); + tx.commit().await?; + Ok(n) +} + +/// Corpus-wide observed range per Y-STR marker: min, modal and max across every +/// profile we hold, with its motif and active mutation rate. Ordered by +/// observation count, descending. +/// +/// Reads the precomputed table ([`refresh_marker_stats`]). Falls back to computing +/// live only while that table is empty — the window between deploying migration +/// 0071 and the first `str-marker-stats` job run — so the report is never blank +/// merely because the job has not run yet. That path is slow by design and says so +/// in the log rather than failing quietly. +pub async fn marker_stats(pool: &PgPool) -> Result, DbError> { + let rows = sqlx::query_as::<_, MarkerStat>(&format!( + "SELECT {MARKER_STAT_COLUMNS} FROM genomics.str_marker_stat \ + ORDER BY observations DESC, marker_name" + )) .fetch_all(pool) .await?; - Ok(rows) + if !rows.is_empty() { + return Ok(rows); + } + // Empty could mean "never refreshed" or "no STR profiles at all"; the live + // query answers both correctly and costs nothing in the latter case. + let live = sqlx::query_as::<_, MarkerStat>(MARKER_STATS_AGGREGATION).fetch_all(pool).await?; + if !live.is_empty() { + tracing::warn!( + markers = live.len(), + "genomics.str_marker_stat is empty — computed the marker report live; \ + run `du-jobs run-once str-marker-stats`" + ); + } + Ok(live) +} + +/// When the precomputed marker statistics were last refreshed (`None` before the +/// first run), so the report can show its own freshness. +pub async fn marker_stats_refreshed_at( + pool: &PgPool, +) -> Result>, DbError> { + Ok(sqlx::query_scalar("SELECT max(refreshed_at) FROM genomics.str_marker_stat") + .fetch_one(pool) + .await?) } /// A ranked STR→branch prediction. diff --git a/rust/crates/du-db/tests/str_marker_stats.rs b/rust/crates/du-db/tests/str_marker_stats.rs index ca5af03..d6e8904 100644 --- a/rust/crates/du-db/tests/str_marker_stats.rs +++ b/rust/crates/du-db/tests/str_marker_stats.rs @@ -119,7 +119,29 @@ async fn marker_stats_reports_range_motif_and_rate_basis() { ) .await; - let stats = ystr::marker_stats(&pool).await.expect("marker_stats"); + // Before any refresh the table is empty, so this exercises the live fallback. + let live = ystr::marker_stats(&pool).await.expect("marker_stats (live fallback)"); + + // …and after a refresh it is served from genomics.str_marker_stat. The report + // is precomputed for speed, so the two paths must agree exactly — a drift here + // would mean the page shows something the aggregation never produced. + let written = ystr::refresh_marker_stats(&pool).await.expect("refresh"); + assert_eq!(written as usize, live.len(), "refresh writes one row per marker"); + let stats = ystr::marker_stats(&pool).await.expect("marker_stats (precomputed)"); + assert_eq!(stats.len(), live.len()); + for (a, b) in stats.iter().zip(live.iter()) { + assert_eq!(a.marker_name, b.marker_name, "precomputed order matches live"); + assert_eq!( + (a.min_value, a.modal_value, a.max_value, a.null_alleles, &a.age_model_status), + (b.min_value, b.modal_value, b.max_value, b.null_alleles, &b.age_model_status), + "precomputed row differs from the live aggregation for {}", + a.marker_name + ); + } + assert!( + ystr::marker_stats_refreshed_at(&pool).await.expect("refreshed_at").is_some(), + "refresh stamps a timestamp for the report to display" + ); // Simple marker: min/modal/max, and the motif + rate reached through the alias. let dys19 = find(&stats, "DYS19"); diff --git a/rust/crates/du-jobs/src/main.rs b/rust/crates/du-jobs/src/main.rs index baa2efc..8d32da0 100644 --- a/rust/crates/du-jobs/src/main.rs +++ b/rust/crates/du-jobs/src/main.rs @@ -210,6 +210,13 @@ async fn main() -> anyhow::Result<()> { auto_promoted = rep.auto_promoted, "discovery-consensus complete" ); } + // Recompute the corpus-wide Y-STR marker report (the /str-markers page). + // Aggregating every stored profile per request cost ~1.9s on 876 profiles + // and grew with the corpus, so it is precomputed here — see migration 0071. + "str-marker-stats" => { + let markers = du_db::ystr::refresh_marker_stats(&pool).await?; + tracing::info!(markers, "str-marker-stats complete"); + } "coverage-norms" => { let rep = du_db::coverage::recompute_norms(&pool).await?; tracing::info!(test_types = rep.test_types, pruned = rep.pruned, "coverage-norms complete"); @@ -284,6 +291,12 @@ async fn main() -> anyhow::Result<()> { anyhow::anyhow!("set FTDNA_STR_DIR + COHORT_MANIFEST") })?; ftdna_str::run(&pool, &cfg).await?; + // The import changes every marker's range, so refresh the report + // rather than leaving it stale until the next timer. + if cfg.apply { + let markers = du_db::ystr::refresh_marker_stats(&pool).await?; + tracing::info!(markers, "refreshed str-marker-stats after import"); + } } // Backfill vendor kit identifiers (FTDNA/YSEQ/Dante/FGC/Nebula) from the // cohort manifest into core.biosample_identifier — the background dedup key @@ -382,7 +395,7 @@ async fn main() -> anyhow::Result<()> { ena::enrich_studies(&pool, &client).await?; } other => anyhow::bail!( - "unknown run-once job '{other}' (known: ybrowse, reconcile, variant-representatives, naming-status-normalize, variant-name-reconcile, normalize-placeholder-contig, mint-batch, yregions, branch-age, ftdna-str, sequencer-consensus, discovery-consensus, coverage-norms, ibd-discovery-recompute, exchange-expire, tree-samples-recompute, dedup-candidates, consolidate-donors, link-federated-subjects, import-kit-identifiers, mt-rcrs-lift, variant-coord-lift, crawl-project, publication-topic-prune, name-private-nodes, publication-update, publication-discovery, publication-pubmed-update, ena-study-enrichment)" + "unknown run-once job '{other}' (known: ybrowse, reconcile, variant-representatives, naming-status-normalize, variant-name-reconcile, normalize-placeholder-contig, mint-batch, yregions, branch-age, ftdna-str, str-marker-stats, sequencer-consensus, discovery-consensus, coverage-norms, ibd-discovery-recompute, exchange-expire, tree-samples-recompute, dedup-candidates, consolidate-donors, link-federated-subjects, import-kit-identifiers, mt-rcrs-lift, variant-coord-lift, crawl-project, publication-topic-prune, name-private-nodes, publication-update, publication-discovery, publication-pubmed-update, ena-study-enrichment)" ), } return Ok(()); diff --git a/rust/crates/du-web/src/routes/str_markers.rs b/rust/crates/du-web/src/routes/str_markers.rs index 3af0822..2b9dab4 100644 --- a/rust/crates/du-web/src/routes/str_markers.rs +++ b/rust/crates/du-web/src/routes/str_markers.rs @@ -86,6 +86,8 @@ struct MarkersTemplate { sort: String, /// The rate used where a marker has none, shown in the legend. default_rate: String, + /// When the precomputed statistics were last refreshed; empty if never. + refreshed_at: String, } /// Integer with thousands separators (e.g. `12,345`). @@ -197,6 +199,12 @@ async fn page( Query(query): Query, ) -> Result { let all = du_db::ystr::marker_stats(&st.pool).await?; + // These figures are precomputed (du-jobs run-once str-marker-stats), so the + // page states its own age rather than implying it is live. + let refreshed_at = du_db::ystr::marker_stats_refreshed_at(&st.pool) + .await? + .map(|t| t.format("%Y-%m-%d %H:%M UTC").to_string()) + .unwrap_or_default(); // Totals describe the whole corpus, not the filtered view — they are the // reference-coverage headline (how much of what we observe we can explain). @@ -241,6 +249,7 @@ async fn page( q, sort, default_rate: fmt_rate(Some(du_db::ystr::DEFAULT_STR_RATE)), + refreshed_at, })) } diff --git a/rust/crates/du-web/templates/str/markers.html b/rust/crates/du-web/templates/str/markers.html index f23af9f..b94decf 100644 --- a/rust/crates/du-web/templates/str/markers.html +++ b/rust/crates/du-web/templates/str/markers.html @@ -84,4 +84,5 @@

{{ t.get("str.markers.title") }}

{{ t.get("str.markers.legend.null") }}

{{ t.get("str.markers.legend.default") }} {{ default_rate }}.

{{ t.get("str.markers.legend.motif") }}

+{% if refreshed_at != "" %}

{{ t.get("str.markers.refreshed") }} {{ refreshed_at }}.

{% endif %} {% endblock %} diff --git a/rust/locales/en.txt b/rust/locales/en.txt index e86dbf5..9f77712 100644 --- a/rust/locales/en.txt +++ b/rust/locales/en.txt @@ -807,3 +807,4 @@ str.markers.none=No STR markers match. str.markers.legend.null=A reported value of 0 is a null allele (a deleted locus), not a zero repeat count, so it is excluded from the range and counted under Notes. Partial repeats (e.g. 10.2) are preserved but not scored. str.markers.legend.default=Markers without a published mutation rate are scored in the branch-age model at the default rate of str.markers.legend.motif=Motifs and repeat lengths are shown only where a published source gives one; most extended markers have none. +str.markers.refreshed=Figures last recomputed diff --git a/rust/locales/es.txt b/rust/locales/es.txt index cc7c1d1..ea7be9b 100644 --- a/rust/locales/es.txt +++ b/rust/locales/es.txt @@ -801,3 +801,4 @@ str.markers.none=Ningún marcador STR coincide. str.markers.legend.null=Un valor de 0 es un alelo nulo (un locus eliminado), no un recuento de cero repeticiones, por lo que se excluye del rango y se cuenta en Notas. Las repeticiones parciales (p. ej. 10,2) se conservan pero no se puntúan. str.markers.legend.default=Los marcadores sin tasa de mutación publicada se puntúan en el modelo de edad de rama con la tasa por defecto de str.markers.legend.motif=Los motivos y las longitudes de repetición solo se muestran cuando una fuente publicada los proporciona; la mayoría de los marcadores extendidos carecen de ellos. +str.markers.refreshed=Cifras recalculadas por última vez el diff --git a/rust/locales/fr.txt b/rust/locales/fr.txt index e7a1bba..1c4e302 100644 --- a/rust/locales/fr.txt +++ b/rust/locales/fr.txt @@ -801,3 +801,4 @@ str.markers.none=Aucun marqueur STR correspondant. str.markers.legend.null=Une valeur de 0 correspond à un allèle nul (locus supprimé), et non à un compte de zéro répétition ; elle est donc exclue de la plage et comptée dans les Notes. Les répétitions partielles (p. ex. 10,2) sont conservées mais non évaluées. str.markers.legend.default=Les marqueurs sans taux de mutation publié sont évalués dans le modèle d'âge de branche au taux par défaut de str.markers.legend.motif=Les motifs et longueurs de répétition ne sont affichés que lorsqu'une source publiée les fournit ; la plupart des marqueurs étendus n'en ont pas. +str.markers.refreshed=Chiffres recalculés le diff --git a/rust/migrations/0071_str_marker_stat.sql b/rust/migrations/0071_str_marker_stat.sql new file mode 100644 index 0000000..4d5c94b --- /dev/null +++ b/rust/migrations/0071_str_marker_stat.sql @@ -0,0 +1,64 @@ +-- Precomputed corpus-wide Y-STR marker statistics (the /str-markers report). +-- +-- The report was aggregating every profile on every request. Profiling the +-- 876-profile dev corpus put that at ~1.9s, and it scales linearly with the +-- number of profiles, so production was worse. The cost is NOT the JSONB +-- expansion (36ms for 514k observations) and it is NOT the reference joins +-- (13ms) — it is the aggregate machinery: mode() WITHIN GROUP, three ordered +-- array_agg()s and two DISTINCT counts, all over every expanded observation. +-- Indexes cannot touch any of that, and the obvious rewrites recovered only +-- ~25% (pre-aggregating by distinct value: 1.45s; raising work_mem: no change). +-- +-- So it is precomputed instead, following the coverage-norms / +-- discovery-consensus / tree-samples-recompute pattern already in du-jobs: a +-- plain table refreshed by `du-jobs run-once str-marker-stats`, leaving the +-- report a ~829-row scan that no longer grows with the corpus. (A plain table +-- rather than a MATERIALIZED VIEW because this schema uses none, and because +-- the refresh needs to run inside the job's transaction alongside its logging.) +-- +-- Refresh cadence: after the `ftdna-str` importer, and on a timer for the +-- federated path (fed.str_profile arrives continuously via Jetstream). The +-- content is a population summary, so brief staleness is acceptable — a newly +-- imported kit appears at the next refresh, not instantly. +-- +-- Left empty here on purpose: the aggregation lives in du_db::ystr so it has one +-- definition, rather than being duplicated into this file where the two would +-- drift. du_db::ystr::marker_stats falls back to computing live while the table +-- is empty, so the page is correct before the first refresh — just slow. + +CREATE TABLE genomics.str_marker_stat ( + marker_name TEXT PRIMARY KEY, + multi_copy BOOLEAN NOT NULL, + observations BIGINT NOT NULL, + samples BIGINT NOT NULL, + -- Simple markers carry repeat counts; multi-copy markers carry rendered + -- copy vectors ("11-15"). Exactly one pair is populated per marker. + min_value INTEGER, + modal_value INTEGER, + max_value INTEGER, + min_combination TEXT, + modal_combination TEXT, + max_combination TEXT, + distinct_values BIGINT NOT NULL, + null_alleles BIGINT NOT NULL, + complex_count BIGINT NOT NULL, + -- Denormalized from genomics.str_marker / str_mutation_rate at refresh time, + -- so the read path is a single-table scan with no alias folding to redo. + motif TEXT, + period SMALLINT, + coordinates JSONB, + mutation_rate DOUBLE PRECISION, + rate_ci_low DOUBLE PRECISION, + rate_ci_high DOUBLE PRECISION, + rate_method TEXT, + rate_source TEXT, + age_model_status TEXT NOT NULL, + refreshed_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- The report's default ordering. +CREATE INDEX str_marker_stat_observations_idx + ON genomics.str_marker_stat (observations DESC, marker_name); + +COMMENT ON TABLE genomics.str_marker_stat IS + 'Precomputed /str-markers report rows; refreshed by du-jobs run-once str-marker-stats';