mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-05 20:18:00 +00:00
efitenandClaude Opus 5
a5aa3cdc41
Closes #2058. `ANALYZE` has never run against these databases, so `sqlite_stat1` does not exist and the planner works from built-in guesses. On the channel queries it guesses wrong: it drives from the plain `idx_transmissions_payload_type` instead of `idx_tx_channel_hash`, the partial index (`WHERE payload_type = 5`) the schema already carries for that exact filter. @anieto's report did the diagnosis and the arithmetic. This adds the maintenance operation that was missing, at a value measured rather than assumed. ## The diagnosis transfers, the remedy needed measuring Measured on our 9.4 GB staging database: 1,250,489 transmissions, 14,169,329 observations, 2.7x and 7x the reported database. Region-filtered `GetChannels` produces the identical plan reported in #2058, down to both temp b-trees, so the problem is the same one. Wall time is the wrong metric here. The same query and the same plan measure **56.7s cold and 0.80s warm** on that file, so the OS page cache dominates. Counting page-cache misses instead: | analysis_limit | ANALYZE | driving index | page misses | |---|---|---|---| | none (no statistics) | - | `idx_transmissions_payload_type` | 143,442 | | 400 | 171 ms | `idx_transmissions_payload_type` | 143,449 | | 1000 | 171 ms | `idx_transmissions_payload_type` | 143,450 | | **10000** | **2.0 s** | **`idx_tx_channel_hash`** | **107,429** | | 0 (unbounded) | 242.9 s | `idx_tx_channel_hash` | 107,429 | 400, the value SQLite's documentation offers for the bounded form, changes nothing on this data: it samples too few rows to separate the 126,336-row partial index from the 920,700-row plain one. 10000 buys the entire plan change for 2.0 s, and the four-minute unbounded `ANALYZE` buys nothing beyond it. ## What it is worth, as measured 25% fewer pages read per query, 143,442 to 107,429, about 147 MB less at a 4 KB page. Warm wall time does not move: 0.80s either way. The gain lands on the cold path, the one that measured 56.7s, so the claim here is fewer pages read, not a warm speedup. This is smaller and differently shaped than the 3-4x in #2058. I cannot reproduce that ratio on a database of this size and am not claiming it. ## The change - `Store.RefreshPlannerStats(analysisLimit)` in `cmd/ingestor/db.go`: `PRAGMA analysis_limit=N` then `ANALYZE`, logging the duration and whether this was the first run. - Wired in `cmd/ingestor/main.go` next to the existing WAL checkpoint ticker: 24h, staggered 2 minutes past startup because it takes the write lock. - `db.analysisLimit` in `internal/dbconfig`, default 10000, negative disables it. It runs in the ingestor, not the server: `cmd/server/db.go:145` opens `mode=ro`, and `ANALYZE` writes. This respects the read/write separation invariant in AGENTS.md. `analysis_limit=0` means *no* limit to SQLite rather than "use a default", so an unset config maps to 10000 and a test covers that specific case. ## Two faults the measurement caught in my own first commit Both are in the history rather than hidden, because the second commit is the one that measured: 1. **`PRAGMA optimize` was the wrong statement.** It analyzes only tables the calling connection has itself queried during the session, and a maintenance call has queried none. Run against staging it wrote nothing and left `sqlite_stat1` absent; `PRAGMA optimize(0x03)` returned no statements at all. Verified on an empty database too (SQLite 3.45.1): `ANALYZE` creates `sqlite_stat1`, `PRAGMA optimize` does not. That difference is what makes the behavioural test a guard instead of a no-op. 2. **`analysis_limit=400` was the wrong value**, per the table above. ## Tests Six cases in `cmd/ingestor/refresh_planner_stats_test.go`: - statistics are actually written (the guard against returning to `PRAGMA optimize`) - the pragma reaches the connection, read back through `PRAGMA analysis_limit` - a negative limit leaves `sqlite_stat1` absent - two consecutive refreshes, since a ticker calls this repeatedly - the config default and the JSON round trip - the default is above the range measured ineffective, so lowering it back to 400 fails ## Not done, and one caveat - **Correction to an earlier version of this description**, which said the Go tests could not run locally because this box has no C compiler. That was wrong: `CGO_ENABLED=0` and `gcc` being absent from `PATH` is not the same as no compiler, and a mingw-w64 toolchain is installed here. Run properly, all six tests pass locally, and so does the rest of `cmd/ingestor` apart from `TestWriteStatsAtomic_SymlinkAtDestIsReplaced`, which fails on `os.Symlink` with "A required privilege is not held by the client" on Windows without elevation and lives in `stats_file_test.go`, a file this branch does not touch. CI agrees: Go Build & Test and the ingestor race detector are both green. - The SQLite behaviours above were measured against 3.45.1 on the server, not against the amalgamation `mattn/go-sqlite3` bundles. - **No query rewrite.** #2058 explicitly left that out and so does this; the correctness caveats it lists (per-channel most-recent-message semantics, v2/v3 branches, `enc_` exclusion) are untouched here. - Staging carries limit-10000 statistics, matching what this code produces. Reversible: `DROP TABLE sqlite_stat1` was verified on a scratch database before any of it ran. - Whether a cold-start `ANALYZE` should also run before the 2 minute stagger is not addressed. The first query after a restart is the expensive one, and it can arrive first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_013YAR8fdNTzqjtsggq4xCX6 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>