feat: isolate the full-text search index from the session index and the main thread - #2701
Merged
Merged
Conversation
Add MiniDb.lifecycleStatus() exposing the no-generation/generation-load/ wal-catch-up/full-rebuild/ready/degraded state machine plus per-phase timings (generation candidate load, store/non-text/text image load, postings integrity check, WAL scan/apply, full recovery, text rebuild hosting), so snapshot load, WAL catch-up and full rebuild can be told apart in diagnostics. Also add a repeatable open-lifecycle bench (small data, large WAL delta, large full-text generation, corrupt generation) and fixtures proving a healthy generation open performs no full-corpus tokenization while a corrupt or missing generation falls back. Log search-index and query-store open diagnostics in kap-server and agent-core-v2 so a listSessions call can be attributed to the database it touches. No persistence format or product behavior change.
… index Harden the separation between the session read model and the full-text search index so session operations never depend on search availability: - Reject text index definitions in MiniDbQueryStore at definition level, keeping the session query-store a structural-only read model with no postings/tokenizer artifacts, and assert its generation carries no full-text files. - Share one authoritative scan between the first list and the initial projection (single-flight) instead of scanning twice; reads may only join an in-flight scan, and every fallback read folds the mirror's pending queue so read-your-writes holds while preparing. - Keep withReadModel() fallback semantics pinned by tests: uninitialized/preparing reads hit authoritative metadata immediately, ready reads use the read model, degraded keeps falling back with a diagnosable status reason. - Guard session metadata writes so a mirror failure degrades only the read model and never fails the session lifecycle. - Prove via tests that listSessions/--resume/--continue never open the global search DB (including when search-index is unopenable), and that only real full-text search requests report building/stale/degraded.
Make the whole generation-open path cooperative: - Replace the synchronous postings/store CRC verification with chunked async variants (readGenerationFileCheckedAsync, verifyFileIntegrityAsync) that keep the exact bytes/crc-mismatch error semantics. - Give the WAL-delta apply a primitive-op + wall-clock budget (walApplySlicer), so a batch frame unrolling into thousands of ops can no longer run as one uninterruptible slice; torn-tail, corrupt-batch and read-only behaviors are unchanged. - Slice the big attach loops: Store.bulkLoadRefsAsync + SkipList.bulkLoadAsync for the store image, async parsers and loadImageAsync for secondary/compound images, and TextIndex.attachImageAsync for the docs/dictionary map construction. - Queue text builds on worker-slot pressure (WorkerSlots.acquireBounded, bounded by MiniDb.textBuildSlotWaitMs, abort-aware) instead of falling back to an unbounded inline build; a persisted drought hosts the bounded inline core as the explicit last resort with stats accounting. Bench (bench/open-lifecycle, seed 42): event-loop delay max across the four open scenarios drops from 45/734/331/492 ms to ~12-28 ms with wall time flat or better.
Move the whole search-index MiniDb lifecycle (open, generation load, WAL replay, sync, rebuild, compaction) off the main thread into a long-lived worker_threads host, so it never shares the event loop with TUI input: - Add a versioned request/response protocol and worker entry hosting a host-agnostic SearchIndexCore; the same core also backs an inline backend kept as the explicit rollback (KIMI_CODE_EXPERIMENTAL_SEARCH_WORKER=false, flag default ON). - The worker exclusively owns the search-index handle. The lock token is reported at acquire time (new MiniDb OpenOptions.onLockAcquired hook) and reaped on dirty exit; an orphan-lock detector (same-pid lock row whose token no live holder owns) recovers the window where the token report is lost, so a mid-open crash can never freeze the index into a silent permanent read-only. - Crash handling: in-flight requests are rejected with typed errors, respawn uses capped exponential backoff, per-request watchdogs terminate wedged workers, and beginClose propagates into the worker so dispose stays bounded during a long sync. Page tokens pin a boot-salted generation, so tokens issued before a transparent worker restart fail closed with invalid_page_token. - The main process keeps the sync coordinator (debounce/coalescing/ single-flight), live transcript routing, query normalization and page-token codec; searches keep reading the published generation and report building/stale/degraded instead of waiting for sync/rebuild. - Wire the worker into the CLI packaging: self-contained worker bundles for npm dist and the SEA asset manifest/installer/smoke check, plus a dev runtime (type-stripping + .ts resolve hook) scoped to worker execArgv.
Consolidate the two-index separation into explicit, diagnosable lifecycles: - Surface the global search state machine (stopped / opening / building / ready / degraded / closing) end to end: SearchIndexCore.lifecycleState, SearchWorkerHost lifecycle snapshots cached from RPC responses (and invalidated across worker generations), a never-throwing status() carrying the lifecycle, and a synchronous lifecycleReport() that neither kicks the open nor spawns the worker. Corrupt search-index rebuilds are announced with a dedicated warn log so building, stale, degraded, corrupt and worker-unavailable stay distinguishable. - Turn MiniDb read-only replica catch-up fully cooperative: catchUpWalAsync scans frames with the windowed async scanner and yields per primitive op on the shared walApplySlicer budget, while a per-instance catchUpChain serializes concurrent catch-ups so each caller keeps its atomic watermark advance. The stale synchronous implementations are removed. - Pin the dependency direction and availability timing with tests: session list/create/resume survive a corrupt or unopenable search index (also end-to-end with a dead query-store), search generation reuse and stale-serving keep working across restarts, concurrent cold callers open the index / spawn the worker exactly once, resume-then- fetchSessions performs no duplicate authoritative scan, and a clean dispose releases the lock and settles at stopped. - Document the experimental flag surface (persistence_minidb_readmodel, search_worker) in the root guide.
…he separation Rollout and validation for the index separation plan: - Flip persistence_minidb_readmodel to default ON (rollback via KIMI_CODE_EXPERIMENTAL_PERSISTENCE_MINIDB_READMODEL=false or the experimental config section); session list/--resume/--continue now always go through the isolated session read model with the authoritative fallback. Test harnesses pin the flag off where shared fixtures require hermetic homes, while the dedicated suites keep explicit on/off coverage. - Add a probe proving the main thread stays responsive while the search worker rebuilds and swaps a generation (reindex), completing the TUI responsiveness matrix. - Record the rollout state in the agent-core-v2 guide (session index section) and the root flag line. - Add changesets for the CLI (worker isolation, session index independence) and minidb (cooperative open lifecycle). Validation: full suites green across minidb (551), agent-core-v2 (4760), kap-server (1005), node-sdk (343), klient (91) and the CLI app (2567); open-lifecycle bench event-loop delay max is down from 45/734/331/492 ms to ~16-22 ms across the four scenarios with wall time flat or better.
🦋 Changeset detectedLatest commit: 6198f75 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…aration # Conflicts: # packages/agent-core-v2/src/session/sessionMetadata/sessionMetadataService.ts
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37a48325d9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const byId = new Map(collected.map((summary) => [summary.id, summary])); | ||
| for (const summary of pending) { | ||
| if (workspaceIds !== undefined && !workspaceIds.includes(summary.workspaceId)) continue; | ||
| byId.set(summary.id, summary); |
There was a problem hiding this comment.
When the read model is enabled, a session that is created or updated and then deleted before the 100ms mirror flush can still be returned by list/count reads: the authoritative scan no longer sees the deleted directory, but this pending fold re-adds the stale SessionSummary, while sessionLifecycle.delete() only calls FileSessionIndex.remove() and never clears mirror.pending(). Please clear/tombstone pending entries on delete before merging them into results so deleted sessions stop appearing immediately.
Useful? React with 👍 / 👎.
Comment on lines
+415
to
+417
| if (++n % sliceEvery === 0) await new Promise((r) => setImmediate(r)); | ||
| } | ||
| this.order = await SkipList.bulkLoadAsync(orderEntries, { compareKey: cmpString }, { sliceEvery }); |
There was a problem hiding this comment.
During generation open for a large store image with TTL records that expire while this sliced load is yielding, the Store active-expire timer can run before this.order is rebuilt: it removes the key from map and the old empty order, then the final bulk load rebuilds order from stale orderEntries. That leaves missing/expired keys in the ordered index and can produce duplicate scan results if the same key is later set again; disable active expiry during the bulk load or rebuild the order from the live map after the yields.
Useful? React with 👍 / 👎.
…drain the index on close Two issues surfaced by the read-model default in the acp-server suite: - ISessionIndex.remove only deleted from the query store, but a summary still queued in the mirror was folded back into reads (and re-written by the next flush), resurrecting a deleted session in listings. The mirror now exposes evict(id): drop the queued summary and wait out an in-flight flush before the store delete. - RunningAcpServer.close and SDKRpcClientV2.close disposed the engine without awaiting the asynchronous mirror flush / query-store close, so a host removing homeDir right after close() raced in-flight shard closes (ENOTEMPTY). Both now follow the kap-server shutdown order: drain the mirror while the store is open, dispose, then await the drains.
The store's active-expire timer is armed at construction, so during a sliced bulkLoadRefsAsync a tick can fire mid-load: it reaps a TTL key from the map while the order skiplist is still the old empty one, and the final bulkLoadAsync then rebuilds order from the stale orderEntries snapshot — resurrecting the expired key in the ordered index (and duplicating it if the key is later set again). The sync bulkLoadRefs had no yield windows, so guard the async path with a bulkLoading flag that defers expiry ticks until the load settles (finally-safe).
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
1 participant
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.
Related Issue
No linked issue — the problem is explained below.
Problem
The global full-text search index (
<home>/search-indexMiniDb) shares the Node main process — and its event loop — with the TUI. Opening a large search generation, replaying a large WAL delta, or rebuilding postings ran as long synchronous slices on that main thread: a repeatable bench measured event-loop delays of 331 ms while loading a 200k-doc generation, 492 ms recovering a corrupt one, and 734 ms replaying a 3 MiB WAL delta, which users feel as typing/render freezes at startup or while idle. Session listing (listSessions,--resume,--continue) was entangled with the same process lifecycle, and the MiniDB open path had no structured timing to tell a snapshot load apart from a WAL catch-up or a full rebuild.What changed
1. Observability baseline (minidb)
MiniDb.lifecycleStatus(): an explicitno-generation → generation-load → wal-catch-up → full-rebuild → ready/degradedstate machine plus per-phase timings (candidate load, store/non-text/text image load, postings CRC, WAL scan/apply, full recovery, text rebuild hosting).pnpm -C packages/minidb bench:open) over four scenarios (small corpus, large WAL delta, large full-text generation, corrupt generation) reporting wall time, event-loop delay, and input-latency percentiles.2. Session index isolation (agent-core-v2)
listSessions/--resume/--continuenever open the global search DB; a corrupt or unopenable search index never fails session list/create/resume/replay, and only real full-text search requests report building/stale/degraded.persistence_minidb_readmodelis now on by default (rollback via env/config).3. Cooperative open path (minidb)
4. Search worker isolation (kap-server + CLI packaging)
SearchIndexCorealso backs an inline host kept as the explicit diagnostic rollback (search_workerflag, default ON;KIMI_CODE_EXPERIMENTAL_SEARCH_WORKER=false).OpenOptions.onLockAcquiredhook) and reaped on dirty exit; an orphan-lock detector recovers the window where the token report is lost, so a mid-open crash can never freeze the index into a silent permanent read-only.beginClosepropagates into the worker so dispose stays bounded during a long sync. Page tokens pin a boot-salted generation, so tokens issued before a transparent worker restart fail closed withinvalid_page_token..tsresolve hook) scoped to workerexecArgv.5. Explicit lifecycles and consistency
stopped/opening/building/ready/degraded/closing) is surfaced end to end through a never-throwingstatus()and a synchronouslifecycleReport()that neither kicks the open nor spawns the worker; corrupt-index rebuilds get a dedicated warn log, keeping building, stale, degraded, corrupt, and worker-unavailable distinguishable in diagnostics.6. Validation
oxlint --type-aware0 errors; typecheck clean on all touched packages.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. (Explicit decision: no user-doc change — the TUI does not consume the global search state, the docs have no global-search page to update, and session list/resume behavior is unchanged for end users.)