diff --git a/quartz/plans/2026-07-04-profiles-query-plan.md b/quartz/plans/2026-07-04-profiles-query-plan.md index d261164f02..fde7c347b8 100644 --- a/quartz/plans/2026-07-04-profiles-query-plan.md +++ b/quartz/plans/2026-07-04-profiles-query-plan.md @@ -1,8 +1,16 @@ # The `profiles` query: why kind-0 + authors scans, and how to fix it -**Status: diagnosed + two fixes measured, not yet applied** (the fix is a -core-QueryBuilder behavior change with an ordering trade-off — wants a -maintainer call). Follow-up to the 1M relayBench run, where `profiles` +**Status: shipped — Fix B applied** (the order-preserving one, chosen over +Fix A because dropping the ORDER BY changed observable result ordering — it +broke `FsParityTest`'s ordered-parity assertions, i.e. it's client-visible). +`QueryBuilder.makeSimpleQuery` now pins `INDEXED BY query_by_kind_pubkey_created` +for the exact broken shape — multi-author (`pubkey IN (…)`) + `kinds`, no +`limit`, no d-tags — keeping the newest-first ORDER BY. The REQ plan for +`profiles` seeks the composite index instead of scanning, ~8× at 42k profiles +and growing with profile count (the ~100× at 1M), with **zero behavior +change** (identical rows, identical order). Guarded by +`ProfilesQueryPlanBenchmark`. Follow-up to the 1M relayBench run, where +`profiles` (`Filter(kinds=[0], authors=[…50…])`, profile hydration, **no limit**) was the single query geode lost badly on: **99.5 ms p50 vs strfry 0.94 ms (~100×)**, while geode won or tied the other nine scenarios. @@ -51,15 +59,29 @@ At 2k profiles / 42k events (the gap widens with profile count): | **Fix A** — drop `ORDER BY` when `limit == null` | planner picks `query_by_kind_pubkey_created` itself | **0.17 ms (7×→~100× at 1M)** | grouped by author | | **Fix B** — force composite index, keep `ORDER BY` | `query_by_kind_pubkey_created` + TEMP B-TREE sort | **0.19 ms** | newest-first (unchanged) | -**Fix A** (recommended): in `makeSimpleQuery`, emit `ORDER BY created_at -DESC` only when `limit != null`. Without a limit the relay returns the whole -match set and ordering is a NIP-01 *SHOULD* (clients re-sort); dropping it -lets SQLite choose the selective index on its own — no hint, no fragility — -and speeds up **every** no-limit `kinds + authors` query (reactions, -metadata, relay lists by a follow set), not just profiles. Trade-off: -multi-author no-limit results come back grouped by author rather than -globally newest-first. Single-author and kind-only no-limit queries keep -their order (their chosen index is already `created_at`-ordered). +**Fix A (rejected):** emit `ORDER BY` only when `limit != null || authors == +null`. Fastest (0.15 ms) and no hint, but dropping the order for multi-author +no-limit queries is **client-visible** — it broke `FsParityTest`'s +ordered-parity assertions, i.e. it changes what a REQ returns on the wire. +Not worth a semantic change for this. + +**Fix B (applied):** in `makeSimpleQuery`, pin `INDEXED BY +query_by_kind_pubkey_created` when the filter is multi-author (`authors.size > +1`) with `kinds`, no `ids`, no d-tags, and **no `limit`** — keeping the ORDER +BY. SQLite seeks the authors and sorts the (small) result: same rows, same +newest-first order, ~8× here / ~100× at 1M. The scoping is exact: + +- **single author** (`pubkey = ?`) is already costed right and seeks — no pin; +- **with a limit** (the 150-author home feed) the `created_at` scan + early + `LIMIT` is the better plan — no pin; +- **d-tag / addressable** filters have their own index — no pin; +- **authors-only, no kinds** keeps the planner's choice (needs the + flag-gated `query_by_pubkey_created`; it was only a ~3× minor loss, not the + 100× regression) — left for later. + +`query_by_kind_pubkey_created` is created unconditionally, so the hint never +dangles. No existing test changed — the only multi-author cases in +`QueryAssemblerTest` carry a `limit` or a `search`, both excluded. **Fix B** (zero behavior change): when the filter has `kinds` + `authors` and **no limit**, add `INDEXED BY query_by_kind_pubkey_created` and keep the diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt index 2e42a2cf39..fb723bced0 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt @@ -963,6 +963,29 @@ class QueryBuilder( } } + // A multi-author query (`pubkey IN (…)`) with no limit, combined with + // `ORDER BY created_at DESC`, mis-costs in SQLite: it satisfies the + // order for free off `query_by_kind_created` (kind, created_at) by + // scanning an *entire* kind, rather than doing N seeks on the + // selective `query_by_kind_pubkey_created` (kind, pubkey, …) and + // sorting the (small) result — the ~100× `profiles` regression at 1M. + // Pin the selective index for exactly that shape: + // - single author (`pubkey = ?`) is costed correctly and already + // seeks — no pin needed; + // - *with* a limit, the `created_at` scan + early LIMIT is the better + // plan (the 150-author home feed), so leave it to the planner; + // - d-tag/addressable filters have their own index. + // Order is preserved (the ORDER BY stays); this only redirects the + // index. See quartz/plans/2026-07-04-profiles-query-plan.md. + val pinKindPubkeyIndex = + project && + kinds != null && + authors != null && + authors.size > 1 && + ids == null && + dTags == null && + limit == null + val sql = buildString { if (project) { @@ -970,6 +993,9 @@ class QueryBuilder( } else { append("SELECT row_id FROM event_headers") } + if (pinKindPubkeyIndex) { + append(" INDEXED BY query_by_kind_pubkey_created") + } if (clause.conditions.isNotEmpty()) { append("\nWHERE ") append(clause.conditions) diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/prodbench/ProfilesQueryPlanBenchmark.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/prodbench/ProfilesQueryPlanBenchmark.kt index e3d61cbd55..a35f420fab 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/prodbench/ProfilesQueryPlanBenchmark.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/prodbench/ProfilesQueryPlanBenchmark.kt @@ -29,6 +29,8 @@ import com.vitorpamplona.quartz.utils.EventFactory import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue /** * relayBench's `profiles` scenario — `Filter(kinds=[0], authors=[…50…])`, @@ -45,15 +47,17 @@ import kotlin.test.assertEquals * * `ANALYZE` does **not** help — even a connection that reads fresh stats * keeps the scan, because with the ORDER BY the scan genuinely avoids a - * sort. Two fixes work (see `plans/2026-07-04-profiles-query-plan.md`): - * - **drop the ORDER BY when there is no limit** — the planner then picks - * the composite index itself, ~16×, at the cost of result order across - * authors (a NIP-01 SHOULD; clients re-sort); - * - **force the composite index and keep the ORDER BY** — ~10×, seeks then - * sorts the small result, newest-first order preserved. + * sort. **Applied fix** (`QueryBuilder.makeSimpleQuery`, see + * `plans/2026-07-04-profiles-query-plan.md`): for a **multi-author** + * (`pubkey IN (…)`) filter with `kinds`, no `limit`, and no d-tags, pin + * `INDEXED BY query_by_kind_pubkey_created` — SQLite then seeks the authors + * and sorts the small result, keeping the newest-first ORDER BY (zero + * behavior change). Single-author queries already seek; limited feeds keep + * the `created_at` scan + early LIMIT. * - * This is a diagnosis + regression artifact; it prints the plans and timings - * and asserts only correctness (the row counts match across shapes). + * This is now a regression guard: it asserts the live REQ plan for this + * shape seeks the composite index (not the kind scan), and prints the + * before/after timings. */ class ProfilesQueryPlanBenchmark { private val hex = "0123456789abcdef" @@ -155,29 +159,35 @@ class ProfilesQueryPlanBenchmark { println("─ ProfilesQueryPlanBenchmark: $total events, $authorCount kind-0 profiles, filter kinds=[0]+50 authors ─") - // 1. Current REQ shape: ORDER BY created_at DESC, no limit. - val (baseN, baseMs) = timeRaw("$base ORDER BY created_at DESC") - println(" current (ORDER BY, no limit):") + // The old (bad) shape: ORDER BY created_at DESC forces the kind scan. + val (oldN, oldMs) = timeRaw("$base ORDER BY created_at DESC") + println(" old shape (ORDER BY, no limit):") println(planTree(store.store.explainQuery("$base ORDER BY created_at DESC"))) - println(" → $baseN events, ${"%.3f".format(baseMs)} ms/run") + println(" → $oldN events, ${"%.3f".format(oldMs)} ms/run") - // 2. Fix A: drop ORDER BY (no limit) — planner picks composite itself. - val (fixaN, fixaMs) = timeRaw(base) - println(" Fix A — no ORDER BY (no limit), planner's own choice:") - println(planTree(store.store.explainQuery(base))) - println(" → $fixaN events, ${"%.3f".format(fixaMs)} ms/run (${"%.1f".format(baseMs / fixaMs)}×)") + // The fix, as the REQ path now builds it: INDEXED BY the composite + // index for this multi-author, no-limit shape — seeks the authors, + // keeps the newest-first ORDER BY (sorts the small result). + val reqPlan = store.store.planQuery(profiles) + val fixedSql = "SELECT $cols FROM event_headers INDEXED BY query_by_kind_pubkey_created WHERE kind = 0 AND pubkey IN ($inList) ORDER BY created_at DESC" + val (newN, newMs) = timeRaw(fixedSql) + println(" fixed REQ path (INDEXED BY composite, ORDER BY kept):") + println(planTree(reqPlan)) + println(" → $newN events, ${"%.3f".format(newMs)} ms/run (${"%.1f".format(oldMs / newMs)}× faster, grows with profile count)") - // 3. Fix B: force composite index, keep ORDER BY (order preserved). - val hinted = "SELECT $cols FROM event_headers INDEXED BY query_by_kind_pubkey_created WHERE kind = 0 AND pubkey IN ($inList) ORDER BY created_at DESC" - val (fixbN, fixbMs) = timeRaw(hinted) - println(" Fix B — INDEXED BY composite + ORDER BY (newest-first preserved):") - println(planTree(store.store.explainQuery(hinted))) - println(" → $fixbN events, ${"%.3f".format(fixbMs)} ms/run (${"%.1f".format(baseMs / fixbMs)}×)") - - // Correctness: every shape returns the same events (all 50 here). - assertEquals(baseN, fixaN, "Fix A must return the same rows") - assertEquals(baseN, fixbN, "Fix B must return the same rows") - assertEquals(store.query(profiles).size, baseN, "REQ path matches raw SQL") + // Regression guards: the live REQ path must seek the composite + // index (not scan the kind-only one) and preserve the ORDER BY. + assertEquals(oldN, newN, "row count unchanged by the fix") + assertEquals(newN, store.query(profiles).size, "REQ path returns the match set") + assertTrue( + reqPlan.contains("query_by_kind_pubkey_created"), + "REQ plan should seek the (kind, pubkey) index, was:\n$reqPlan", + ) + assertFalse( + reqPlan.contains("query_by_kind_created "), + "REQ plan must not scan the kind-only index, was:\n$reqPlan", + ) + assertTrue(reqPlan.contains("ORDER BY"), "newest-first order is preserved, was:\n$reqPlan") store.close() }