feat: split runtime and migration stores#589
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_85f2e986-c7e2-40e1-b0fb-d22fc1be61a9) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review @cursor review @BugBot review @coderabbitai review |
|
✅ Action performedReview finished.
|
| "idx_chunks_simhash_band_0": "index", | ||
| "idx_chunks_simhash_band_1": "index", | ||
| "idx_chunks_simhash_band_2": "index", | ||
| "idx_chunks_simhash_band_3": "index", |
There was a problem hiding this comment.
🟡 Medium brainlayer/runtime_store.py:62
_REQUIRED_SCHEMA_OBJECTS omits kg_entities_fts and its sync triggers, so _validate_runtime_schema accepts a database where the KG FTS table was dropped or corrupted. Later KG-backed search paths in search_repo.py and mcp/search_handler.py execute kg_entities_fts queries directly and throw SQLite no such table errors at runtime. Add kg_entities_fts and its sync triggers to _REQUIRED_SCHEMA_OBJECTS (and corresponding hashes to _REQUIRED_SQL_HASHES) so the schema contract rejects databases missing KG FTS.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/brainlayer/runtime_store.py around line 62:
`_REQUIRED_SCHEMA_OBJECTS` omits `kg_entities_fts` and its sync triggers, so `_validate_runtime_schema` accepts a database where the KG FTS table was dropped or corrupted. Later KG-backed search paths in `search_repo.py` and `mcp/search_handler.py` execute `kg_entities_fts` queries directly and throw SQLite `no such table` errors at runtime. Add `kg_entities_fts` and its sync triggers to `_REQUIRED_SCHEMA_OBJECTS` (and corresponding hashes to `_REQUIRED_SQL_HASHES`) so the schema contract rejects databases missing KG FTS.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dfef1f9e5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with _without_connection_maintenance_hooks(): | ||
| self.conn = apsw.Connection(str(self.db_path), flags=apsw.SQLITE_OPEN_READWRITE) | ||
| try: | ||
| self.conn.setbusytimeout(_write_busy_timeout_ms()) |
There was a problem hiding this comment.
Restore the default busy timeout after runtime open
When the MCP cold path opens the writer inside temporary_write_busy_timeout_ms (see _store around store_handler.py lines 862-864), _write_busy_timeout_ms() returns only the current store budget (default 400 ms). This line persists that short value on the cached WriterRuntimeStore connection, and unlike the legacy VectorStore init path there is no reset to the default timeout after the schema probe. If the first writer open is a brain_store call, later tools that reuse the singleton writer (for example brain_digest, brain_tags, or stats/maintenance paths) inherit the 400 ms timeout and can fail or defer under normal write contention instead of waiting the intended default.
Useful? React with 👍 / 👎.
Summary
ReadonlyStore, fastWriterRuntimeStore, and copy-onlyOfflineMigratorBRAINLAYER_RUNTIME_STORE=legacyas the rollback switchruntime_openwith Phase 0 telemetry and schema-fingerprint metadataIncident / urgency
Incident
brainbar-c07ade22-eccwedged writer startup twice, caused about 1h15 of outage, and reduced ingest from roughly 1,500/min to zero. This change removes the startup scan/mutation path by construction.Gates
runtime_opentelemetry events, fingerprint5071698596284721e85760ef30234ce836d79ab377f2d70d627b28e3213b0f9aruff check src/ tests/,ruff format --check src/ tests/, and diff checks passedSafety / rollback
BRAINLAYER_RUNTIME_STORE=legacyBRAINLAYER_OFFLINE_MIGRATOR_GATED_SWAP=1Review
Local CodeRabbit review findings were addressed (telemetry completeness, configured DB path, strict semantic schema validation, stale read-only rejection, failure-path close). Re-review was rate-limited, so the documented red/blue fallback audit was completed; it additionally found and fixed telemetry-path clobbering, symlink resolution/TOCTOU, and the legacy migrate canonical-default path.
Note
High Risk
Changes core writer/reader open paths, indexing, MCP store behavior, and migration/repair CLI; misconfiguration or fingerprint drift could block writes or defer MCP stores until offline migration.
Overview
Phase 1 writer store split removes open-time migrations and corpus work from normal indexing and MCP writes, targeting incident
brainbar-c07ade22-ecc(writer startup wedging on large DBs).New
runtime_storeaddsReadonlyStore,WriterRuntimeStore, andOfflineMigrator, plusopen_writer_store()(default runtime;BRAINLAYER_RUNTIME_STORE=legacyrollback). Runtime opens validate a deterministic schema fingerprint from cheap metadata reads only—no DDL/DML, FTS reconciliation, or chunk scans—and emitruntime_opentelemetry with fingerprint metadata.SchemaFingerprintMismatchfails closed; MCP treats it like a lock and DEFERRED-queues writes.CLI / paths:
migrate,repair-fts, and newmigrate-storerequire an explicit offline DB copy; canonical is refused unlessBRAINLAYER_OFFLINE_MIGRATOR_GATED_SWAP=1. Read-only CLI/search paths no longer bootstrap missing schema—they error with migrate-store instructions. Fast index opens one shared writer for the whole file loop via injectedstoreonindex_chunks_to_sqlite.Supporting changes: APSW best-practice hooks skip WAL/optimize during runtime connect; writer telemetry gains
sample_fts/add_metadata.scripts/benchmark_runtime_store_open.pybenchmarks copy-only opens and checks for forbidden SQL.Extensive tests cover fail-closed behavior, single index connection reuse, and benchmark safety.
Reviewed by Cursor Bugbot for commit 6dfef1f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Split runtime and migration store paths to enforce schema validation before reads and writes
WriterRuntimeStoreandReadonlyStoreinruntime_store.pythat validate a SHA-256 schema fingerprint on open and fail closed if the DB is missing, stale, or mismatched — replacing implicit bootstrap viaVectorStore.OfflineMigrator(aVectorStoresubclass) that is the only class permitted to create or migrate schemas, restricted to non-canonical DB paths unlessBRAINLAYER_OFFLINE_MIGRATOR_GATED_SWAP=1is set.open_writer_storefactory that selectsWriterRuntimeStoreby default or falls back to legacyVectorStoreviaBRAINLAYER_RUNTIME_STORE=legacy.index-fast,migrate,migrate-store,repair-fts) and MCP store handler to use the new store types;index-fastnow reuses a single writer store connection across all indexed files.scripts/benchmark_runtime_store_open.pythat measures writer-open latency and emits JSON with p50/p95/p99 timing, schema fingerprints, and statement-violation checks.stats,search, and MCP search paths now raiseSchemaFingerprintMismatchinstead of auto-creating a DB when the configured DB is absent or has a stale schema.📊 Macroscope summarized 6dfef1f. 11 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.