Repository navigation
fix(telemetry): stop re-walking stores too large for dbstat - #4166
Conversation
GuardedStoreTelemetryPort in tracedecay-maintenance is the production store-size and table-growth telemetry port. The rusqlite-runtime copy had no production caller; only its own tests used it. The behavior those tests covered that the production port did not is now tested on GuardedStoreTelemetryPort: observed store-size pragmas, a table created after the baseline reported as baseline-pending, a denied read for another store, and a wait on a busy health reader yielding the poller.
Per-table growth walks every table page through dbstat, so its cost tracks the store size. On the live daemon the 20 GB sessions.db and the 5.5 GB user-sessions.db can never finish inside the 5 s reader deadline (the same walk takes 48-95 s on a copy of user-sessions.db). Every full maintenance tick spent 5 s per store and Doctor reported unknown.deadline_exceeded for both. A walk that runs out of its deadline now records the store page count. Later passes read page_count first, which is cheap, and skip the walk while the store is at least that large. They report the new exceeds_measurement_budget reason. A smaller store, or a mark older than a day, is walked again. Measured through GuardedStoreTelemetryPort on a 1.44M-page (5.9 GB) fixture: the first pass stops at its 5.006 s deadline and marks the store. The next two passes return in 2.5 ms and 0.25 ms.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| page_count >= self.page_count | ||
| && now.0.saturating_sub(self.observed_at.0) < TABLE_SIZE_WALK_REPROBE_MICROS |
There was a problem hiding this comment.
🟡 Shrunken tables stay unmeasured for a day
After large tables lose rows, still_applies skips their walks while the file's page count stays unchanged. SQLite retains freed pages on its freelist, so a newly affordable dbstat walk remains suppressed for a day.
Learn more
SQLite normally retains pages freed by row deletion in its database file. The new suppression check compares page_count with the count recorded at the timeout. Deleting most rows can therefore shrink the b-trees that dbstat traverses without reducing page_count; the next pass still skips the read for 24 hours. The same store_size_telemetry result already includes freelist_pages, which can distinguish this change.
Example: A 20 GB sessions database times out at five million pages. Retention removes most sessions and releases four million pages to SQLite's freelist without running VACUUM. Its page_count remains five million; the walk is skipped despite only one million occupied pages remaining.
Recommended fix: Record and compare occupied pages (page_count - freelist_pages) from the existing store-size sample, or clear the mark on a material freelist increase. Preserve an expiry for cases where the page count stays stable but workload conditions change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Err(TableGrowthUnavailableReasonV1::DeadlineExceeded) => { | ||
| Ok(TableSizeWalk::OverBudget(OverBudgetTableSizeWalk { | ||
| page_count, | ||
| observed_at, | ||
| })) |
There was a problem hiding this comment.
🟡 Transient deadline suppresses later table reads
When table_size_telemetry times out for a transient reason, settle_table_size_walk marks the store over budget. A healthy store then skips table measurements for 24 hours unless its page count drops.
Learn more
The table-size worker returns DeadlineExceeded whenever its reply takes longer than the bound, including time queued behind prior worker activity or temporarily slow storage. That error alone does not distinguish an unaffordable dbstat walk from a transient delay. Once recorded, the new mark suppresses further walks for a full day on an unchanged store.
Example: A modest database briefly stalls on disk IO for five seconds. Its table_size_telemetry call times out; later reads would finish quickly, but maintenance skips them until the mark ages out.
Recommended fix: Treat a single timeout as an uncertain failure and require repeated timeouts before suppressing walks, or obtain a distinct indication that the dbstat query itself exhausted its budget. Keep the transient deadline reason visible until the evidence supports the over-budget classification.
Was this helpful? React with 👍 or 👎 to provide feedback.
…-store-growth # Conflicts: # plugin/chatgpt-extension/embedded/server.mjs
1 participant
Summary
Doctor on the live daemon reports two table-growth unknowns:
storage.table_growth.sessions.db.unknown.deadline_exceededandstorage.table_growth.user-sessions.db.unknown.deadline_exceeded. These are the20 GB and 5.5 GB session stores. Per-table growth walks every table page through
dbstat, so the walk cost tracks the store size. On these stores the walk cannever finish inside the 5 s reader deadline. Every full maintenance tick still
spent 5 s per store and measured nothing.
This PR has two commits.
simplify(telemetry)deletesSqliteStoreSizeTelemetryPortfromtracedecay-rusqlite-runtime. It was a second implementation of thestore-size telemetry port, and only its own tests called it. The production
port is
GuardedStoreTelemetryPortintracedecay-maintenance. The coveredbehavior that the production port did not test now has tests on
GuardedStoreTelemetryPort:fix(telemetry)stops re-walking stores that are too large fordbstat.When a walk runs out of its deadline, the port records the store's page
count. Later passes first read
page_count, which is a cheap pragma. Whilethe store is at least that large, the pass skips the walk and reports the new
reason
exceeds_measurement_budget. A smaller store, or a mark older than oneday, is walked again. A successful walk clears the mark. The mark is part of
the persisted table-growth state, so it survives a daemon restart.
Doctor now reports
storage.table_growth.<store>.unknown.exceeds_measurement_budgetfor thesestores. Store-level growth for them is still observed through
page_count.Alternatives measured
The alternatives ran on a copy of the live
user-sessions.db(1,356,185 pages,200 tables). The copy was deleted afterwards.
dbstatwithaggregate=TRUEaggregate=TRUEwithmmap_sizesetNo
dbstatform fits 5 s.aggregate=TRUEis also not adopted for anotherreason. In aggregate mode one
xNextwalks a whole b-tree before control returnsto the VDBE, so
sqlite3_interruptwaits for the current table to finish. Aftera SIGINT at 2 s, the aggregate query exited at 3.2 s and the current query
exited at 2.0 s. The current per-page form keeps the reader interruptible.
Two other options were rejected. The tables that matter most are the largest
ones, so walking only some tables does not help. A fixed page threshold does not
work either, because the cost per page varied 20x between a hot ext4 page cache
(1.7 to 4 µs) and this ZFS host (36 µs). The over-budget mark adapts to the host.
Verification
GuardedStoreTelemetryPortwith the production 5 sdeadline on a 1,438,536-page (5.9 GB) fixture. The scratch harness was not
committed. The first pass stopped at its deadline (5.006 s) and recorded the
mark. The second pass took 2.5 ms and the third took 0.25 ms. Both reported
exceeds_measurement_budget.an_over_budget_store_is_not_walked_until_it_shrinks_or_the_mark_ages_outfails without the skip, because the walk runs and reports
BaselineEstablished.a_telemetry_wait_yields_the_polling_threadfails when the wait runs inlineinstead of through
native_runtime::spawn_blocking.bazel testpassed for//crates/tracedecay-maintenance:unit_test,//crates/tracedecay-rusqlite-runtime:unit_test,//crates/tracedecay-contracts:all,//crates/tracedecay-dashboard-api:all,//crates/tracedecay:daemon_suite, and//crates/tracedecay:unit_testfiltered todaemon::maintenance(19 tests).bazel build --config=clippypassed on the maintenance, rusqlite-runtime andcontracts crates.
rustfmt_check_2024passed for all three.contracts:generateupdated onlysdks/typescript/src/operations.ts, andcontracts:checkreports the contracts are up to date.The reader side is unchanged.
reader/worker.rstable_sizeskeeps its queryand its interrupt-on-timeout behavior.