Skip to content

Decide: port upstream SQLite planner statistics (ANALYZE in the ingestor)? #100

Closed

Description

@dborup

Decision needed

Should we port upstream's SQLite planner-statistics work (ANALYZE in the ingestor) to this fork? This issue tracks the decision only; nothing is implemented yet.

Upstream

  • Kpa-clawbot/CoreScope#2072 (merged 2026-09-24): the ingestor refreshes SQLite planner statistics periodically with a bounded ANALYZE. Its first run happens 2 minutes after start.
  • Kpa-clawbot/CoreScope#2074 (open at the time of writing): builds the statistics at startup when sqlite_stat1 is missing, and logs a warning explaining the pause.
  • Both address upstream Kpa-clawbot/CoreScope#2058: without statistics, the planner picks a bad plan for GetChannels / GetEncryptedChannels, and a cold query takes about 10–13 s instead of under 1 s.

Why it matters here

  • This fork has neither change. There is no ANALYZE or sqlite_stat1 handling on master (fa5bf4f7).
  • The affected queries back the Channels page. That is the same area as the channels-cache fix and the shared-channel-proposals work.
  • It is unknown whether our staging or production databases already contain sqlite_stat1.

Known cost

Upstream measured the first ANALYZE on a 9.4 GB database at 3 min 44 s cold, holding the write connection.

  • Ingest stalls and buffers for about 4 minutes. No data was lost in their test.
  • The statistics persist in the database file, so this happens once per database, not on every restart.
  • cmd/server opens the database read-only (mode=ro) and reads the statistics fine. All writing stays in cmd/ingestor/, as the read/write invariant requires.

Proposed plan (if we decide yes)

  1. Wait until Kpa-clawbot/CoreScope#2074 is merged or closed upstream.
  2. Check read-only whether staging and production databases already have sqlite_stat1.
  3. Port #2072 and #2074 together as one PR in cmd/ingestor/.
  4. Measure the channel queries before and after on a copy of the staging database, and document the one-time ingest pause.
  5. Test on staging with a rollback image and DB snapshot, and check MQTT and write errors afterwards.
  6. Plan the production rollout separately, accounting for the one-time pause.

Decision options

  • Port as proposed after upstream settles.
  • Port only #2072 and accept that the first refresh happens 2 minutes after start.
  • Skip if our databases already have statistics and the channel queries are fast enough.

Activity

  1. dborup commented on Sep 26, 2026

    @dborup
    OwnerAuthor

    Update: another query in this fork is exposed to the same missing-statistics risk

    While fixing review findings on #97 (Recent Adverts split), a related planner issue turned up.

    Finding

    • Without ANALYZE statistics (sqlite_stat1), SQLite's index choice for "a node's ADVERT rows" depends on the SQLite version and index order.
    • It can choose the payload_type index instead of idx_transmissions_from_pubkey. With payload_type, the query visits every advert in the database instead of only that node's rows.
    • The SQLite build used by the server (3.46, modernc driver) happened to choose from_pubkey correctly on all real databases tested. So nothing is known to be slow today.

    What #97 does about it

    • The new per-node advert scan is pinned to from_pubkey (the payload_type term is written as +payload_type, so the planner cannot use that index).
    • A plan test covers it.

    Still exposed on master

    Relevance to this issue

    Suggested addition to the plan, whichever option is chosen:

    1. Add a plan test for CountFloodAdvertsForNode like the one in feat(nodes): Recent Adverts split into flood / zero-hop / mixed with per-class counts #97, or pin its index the same way.
    2. When measuring before/after on a staging copy, include the node-detail advert queries, not only the Channels queries.
  2. added 2 commits that reference this issue on Sep 27, 2026
  3. dborup commented on Sep 28, 2026

    @dborup
    OwnerAuthor

    Decision: do not port the upstream ANALYZE planner statistics (Kpa-clawbot/CoreScope#2072 / #2074). The affected queries are pinned instead, merged in #107 (1bdf088e). #106 (the ANALYZE port) is closed unmerged.

    Why:

    • Cold ANALYZE on the 5 GB staging database: 63–67 s, reading every index in full even with analysis_limit (it only limits sampling, not I/O), under the ingestor's write lock.
    • Index pinning gives the statistics plan without that cost.

    Staging-iso measurement of #107 (copy of the staging DB, staging hardware, master b3e44761 vs 588a52ba, 126 interleaved runs, identical results by sha256 in every case, 0 pin fallbacks):

    GetChannels master #107
    all regions, cold 6,703 ms 4,431 ms (−34 %)
    AAR, cold 14,610 ms 12,607 ms (−14 %)
    CPH, cold 14,133 ms 12,270 ms (−13 %)
    all regions, warm 345 ms 245 ms (−29 %)
    AAR, warm 1,108 ms 872 ms (−21 %)
    CPH, warm 974 ms 785 ms (−19 %)

    CountFloodAdvertsForNode already uses idx_transmissions_from_pubkey on staging, so the change is neutral there. It protects databases where idx_transmissions_payload_type is the newer index; that case was not measured on real data.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions