perf(state): narrow FTS UPDATE triggers with AFTER UPDATE OF (salvage #73639) - #77628
Merged
kshitijk4poor merged 4 commits intoAug 3, 2026
Merged
Conversation
kshitijk4poor
enabled auto-merge (rebase)
August 3, 2026 11:59
alt-glitch
added
type/perf
Collaborator
Author
|
/rerun |
Retarget NousResearch#73639 onto the SessionDB mixin split (hermes_state_common / hermes_state_schema). Fresh installs create UPDATE OF content/tool_* triggers; existing broad AFTER UPDATE triggers are inspected and replaced under schema init without an FTS rebuild (WHEN clauses already guarded content correctness; OF skips non-content status writes that saturated disk I/O on large state.db). Tests: tests/test_fts_update_of_narrowing.py (4)
_ensure_fts_cjk_schema never raises on OperationalError; post-condition after dropping messages_fts_cjk_update now requires a narrowed UPDATE trigger or durable fts_cjk_stale + unavailable. Covers the production soft-fail path the raise-only handler missed.
Simplify-pass fold: to_drop names come from the literal update_names\nallowlist via IN binding, so the [A-Za-z0-9_]+ fullmatch could never\nfail — and if it somehow did, its `continue` would miscount (the\nskipped trigger stayed in len(to_drop)/the log while CREATE TRIGGER\nIF NOT EXISTS silently kept the broad variant). Delete the guard and\nits function-local re import; keep the invariant as a comment.
kshitijk4poor
force-pushed
the
salvage/73639-fts-update-of
branch
from
August 3, 2026 14:40
836fc0d to
044a0ba
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Every
UPDATE messages SET ...fires the FTS sync triggers today, because they are broadAFTER UPDATE ON messages. Status-only writes — marking rows inactive/compacted/observed during compaction, recovery, and housekeeping — pay trigger WHEN-clause evaluation for every row touched even though no indexed column changed, which on large state.dbs saturates I/O (#68858). WHO benefits: anyone with a large session DB, during compaction/optimize passes and bulk status updates.Measured impact
10,000 status-only UPDATEs on a synthetic FTS-enabled DB (sqlite trace-counted):
Honest caveat: the win applies to status-only UPDATE statements; content/tool_name/tool_calls updates still (correctly) fire the triggers. Every
UPDATE messages SETsite on main was enumerated — none can change indexed columns without listing them in SET, soAFTER UPDATE OFis semantically safe.Provenance
Salvage of #73639 by @smfworks (3 commits, authorship preserved, incl. the co-contributed fail-closed CJK migration handling). Migration audited:
_migrate_broad_fts_update_triggersis idempotent (second open drops 0), costs two µs-scale sqlite_master point reads per writer open in steady state, and the legacy-layout branch recreates everything it drops. The CJK quarantine path uses the existing in-transactionset_meta(..., cursor=)variant — no nested-transaction hazard. Simplify-pass fold: removed an unreachable regex allowlist guard whosecontinuewould have miscounted a skipped trigger (names come from a literal allowlist via IN binding).Verification
-p no:randomly.role <> 'tool')._drop_fts_triggersis a false consolidation target — it drops insert/delete triggers too).Closes #73639.