perf(cache): scheduler-facing get_cached_request_context + concurrent-init DDL fix - #422
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds cached request-context access, increases cache capacity to 512, synchronizes SQLite initialization per database, and makes aggregation trigger replacement transactional with rollback handling. ChangesCached request context
SQLite initialization safety
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant get_cached_request_context
participant get_reflexio
participant SQLiteStorage
Scheduler->>get_cached_request_context: request organization context
get_cached_request_context->>get_reflexio: retrieve cached Reflexio
get_reflexio-->>get_cached_request_context: shared Reflexio instance
get_cached_request_context-->>Scheduler: return request_context
SQLiteStorage->>SQLiteStorage: acquire database-path initialization lock
SQLiteStorage->>SQLiteStorage: configure connection and run migrations
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reflexio/server/services/storage/sqlite_storage/playbook/_aggregation.py`:
- Line 114: Update the DDL execution around the trigger definitions at the
aggregation setup points on lines 114, 128, and 168 to use an explicit BEGIN
IMMEDIATE transaction covering every DROP/CREATE replacement, then commit only
after all replacements succeed and roll back on failure. Preserve the existing
trigger definitions while ensuring concurrent writers cannot run between
replacement statements.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c25fd725-5ca0-4525-ac37-9192b151375f
📒 Files selected for processing (6)
.claude/rules/reflexio-patterns.mdreflexio/server/cache/__init__.pyreflexio/server/cache/reflexio_cache.pyreflexio/server/services/storage/sqlite_storage/playbook/_aggregation.pytests/server/cache/test_reflexio_cache.pytests/server/services/durable_learning/test_worker.py
…; default cache size 512 Schedulers that fan out per-org work were building RequestContext directly per tick (config decrypt + storage client pools + LLM client each time). The new accessor delegates to get_reflexio() so schedulers share the same cache entry, per-hit config_version eviction, striped construction locks, and every existing invalidate_reflexio_cache call site as the request path. REFLEXIO_CACHE_MAX_SIZE default rises 100 -> 512: fleet-paging schedulers hold two independent 100-org pages, so a 100-slot LRU would evict warm request-path entries on fleets above ~100 orgs.
…a init Concurrent SQLite storage initialization on one db file interleaves the DROP TRIGGER / CREATE TRIGGER pairs across connections, so one connection's CREATE lands between the other's DROP and CREATE and raises 'trigger ... already exists' (or 'database is locked'). CREATE TRIGGER IF NOT EXISTS keeps the drop-and-recreate refresh semantics for single-writer upgrades while making same-script concurrent runs benign. Also hoist the claim-race test's RequestContext construction out of its racing worker threads: parallel cold construction is exactly what get_reflexio's construction lock exists to serialize, and the direct test factory bypasses it. The race under test is the claim-token fence, not construction.
Serialize same-file SQLite setup across cache-miss constructions and replace aggregation triggers inside one explicit transaction so concurrent connections never observe a partial schema.
yyiilluu
force-pushed
the
claude/cache-golden-floyd-plan-60e2e7
branch
from
August 6, 2026 18:19
76c7ad2 to
83b6e6f
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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.
Summary
Adds
get_cached_request_context(), a thin accessor over the existing per-orgReflexiocache, for the two hot playbook-aggregation schedulers in the companion Enterprise PR. The accessor preserves the existing config-version probe, invalidation behavior, construction locks, and cache identity.The PR also fixes the SQLite cold-start races exposed when different org cache keys concurrently initialize the same SQLite database file.
Changes
get_cached_request_context(org_id, storage_base_dir=None).BEGIN IMMEDIATEtransaction; failures roll back the entire replacement, so other connections cannot observe a partial trigger set.Test plan
Related PR