Keep disk state reads, sidecar stats and redis version lookup off the event loop - #7065
Keep disk state reads, sidecar stats and redis version lookup off the event loop#7065FarhanAliRaza wants to merge 3 commits into
Conversation
Runs a representative app in-process via AppHarness with blockbuster in a recording mode, drives the common event paths with Playwright per state manager (and prod static serving), and reports every blocking call that ran on the backend event loop with its stack.
… event loop Found with the blockbuster audit: - StateManagerDisk.load_state opened and unpickled the state file synchronously on every cache miss, once per substate. Move the read into a worker thread like the write already was, and drop the per-write states_directory.exists() by handling FileNotFoundError in the thread. - PrecompressedStaticFiles stat'd each sidecar candidate on the loop inside file_response, per accepted encoding per request. Starlette runs its own lookup in a thread; do the sidecar selection there too, from get_response, and finish the response (Vary/Content-Encoding, conditional check) after. - redis-py >= 7 resolves its own version through importlib.metadata for every new connection, scanning sys.path and reading METADATA on the loop. Create the client with the version pinned (driver_info on 7.x, lib_version before). Each change has a regression test that fails on the previous code.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR moves three sources of blocking work away from the backend event loop and adds regression coverage:
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness, security, or repository-rule violations identified. The disk-state, static-serving, and Redis changes preserve their existing contracts while moving the identified blocking operations off the event loop, and targeted regression tests cover each optimization.
|
| Filename | Overview |
|---|---|
| reflex/istate/manager/disk.py | Moves state-file reads and directory-recovery writes off the event loop without changing missing or invalid state handling. |
| reflex/utils/precompressed_staticfiles.py | Defers sidecar filesystem inspection to a worker thread and centralizes conditional response handling. |
| reflex/utils/prerequisites.py | Passes redis-py's library version through the version-appropriate connection argument. |
| scripts/blockbuster_audit.py | Adds a reusable development and pytest audit for recording blocking calls on active event loops. |
| tests/units/istate/manager/test_disk.py | Verifies state-file reads and writes do not execute on the event-loop thread. |
| tests/units/utils/test_precompressed_staticfiles.py | Verifies sidecar stats occur off-loop while retaining compressed and fallback serving behavior. |
| tests/units/utils/test_prerequisites.py | Verifies Redis connections use the supplied driver version without performing per-connection metadata lookup. |
Reviews (1): Last reviewed commit: "Name news fragment after PR #7065" | Re-trigger Greptile
![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: 106a871815
ℹ️ 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".
| ad hoc; ``uv run`` would re-sync them away, hence the direct interpreter):: | ||
|
|
||
| uv pip install blockbuster aiosqlite | ||
| REFLEX_STATE_MANAGER_MODE=memory .venv/bin/python scripts/blockbuster_audit.py |
There was a problem hiding this comment.
Anyone following these instructions will invoke the virtualenv interpreter directly, bypassing the repository's required environment-management workflow. Use uv run --with blockbuster --with aiosqlite scripts/blockbuster_audit.py (and its pytest equivalent) so the extra dependencies remain available without violating the explicit prohibition on bare Python execution.
AGENTS.md reference: AGENTS.md:L11-L13
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="news/7065.performance.md">
<violation number="1" location="news/7065.performance.md:1">
P3: The middle item of the list has no verb: 'precompressed static file serving stats sidecar files off the loop' is an orphaned noun phrase, while the other two clauses each have a subject and verb. Give it one, e.g. '...sidecar files are resolved off the loop'.</violation>
</file>
<file name="scripts/blockbuster_audit.py">
<violation number="1" location="scripts/blockbuster_audit.py:14">
P2: Run the audit through uv instead of documenting direct `.venv/bin/python` execution. Use `uv run --with blockbuster --with aiosqlite ...` for these commands and the equivalent pytest invocation so the extra dependencies remain available while following the repository's required environment workflow.</violation>
<violation number="2" location="scripts/blockbuster_audit.py:121">
P2: On Windows, `root` never prefixes the backslash-separated frame filenames, so the report misclassifies every Reflex and test finding as `3RDPARTY`. Normalize frame paths with `Path.resolve()` and compare them using `Path.relative_to()` or equivalent platform-independent logic.</violation>
<violation number="3" location="scripts/blockbuster_audit.py:375">
P1: When `reflex.db` exists in the caller's working directory, this unconditional unlink deletes it before the audit and can destroy unrelated data. Use an isolated database under `args.root`, configure both URLs to it, and only remove that isolated path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| args.root.mkdir(parents=True, exist_ok=True) | ||
| os.environ.setdefault("REFLEX_DB_URL", "sqlite:///reflex.db") | ||
| os.environ.setdefault("REFLEX_ASYNC_DB_URL", "sqlite+aiosqlite:///reflex.db") | ||
| Path("reflex.db").unlink(missing_ok=True) |
There was a problem hiding this comment.
P1: When reflex.db exists in the caller's working directory, this unconditional unlink deletes it before the audit and can destroy unrelated data. Use an isolated database under args.root, configure both URLs to it, and only remove that isolated path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/blockbuster_audit.py, line 375:
<comment>When `reflex.db` exists in the caller's working directory, this unconditional unlink deletes it before the audit and can destroy unrelated data. Use an isolated database under `args.root`, configure both URLs to it, and only remove that isolated path.</comment>
<file context>
@@ -0,0 +1,405 @@
+ args.root.mkdir(parents=True, exist_ok=True)
+ os.environ.setdefault("REFLEX_DB_URL", "sqlite:///reflex.db")
+ os.environ.setdefault("REFLEX_ASYNC_DB_URL", "sqlite+aiosqlite:///reflex.db")
+ Path("reflex.db").unlink(missing_ok=True)
+
+ activate()
</file context>
| Returns: | ||
| Number of distinct findings written. | ||
| """ | ||
| root = str(REPO_ROOT) + "/" |
There was a problem hiding this comment.
P2: On Windows, root never prefixes the backslash-separated frame filenames, so the report misclassifies every Reflex and test finding as 3RDPARTY. Normalize frame paths with Path.resolve() and compare them using Path.relative_to() or equivalent platform-independent logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/blockbuster_audit.py, line 121:
<comment>On Windows, `root` never prefixes the backslash-separated frame filenames, so the report misclassifies every Reflex and test finding as `3RDPARTY`. Normalize frame paths with `Path.resolve()` and compare them using `Path.relative_to()` or equivalent platform-independent logic.</comment>
<file context>
@@ -0,0 +1,405 @@
+ Returns:
+ Number of distinct findings written.
+ """
+ root = str(REPO_ROOT) + "/"
+ venv = root + ".venv/"
+
</file context>
|
|
||
| uv pip install blockbuster aiosqlite | ||
| REFLEX_STATE_MANAGER_MODE=memory .venv/bin/python scripts/blockbuster_audit.py | ||
| REFLEX_STATE_MANAGER_MODE=disk .venv/bin/python scripts/blockbuster_audit.py |
There was a problem hiding this comment.
P2: Run the audit through uv instead of documenting direct .venv/bin/python execution. Use uv run --with blockbuster --with aiosqlite ... for these commands and the equivalent pytest invocation so the extra dependencies remain available while following the repository's required environment workflow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/blockbuster_audit.py, line 14:
<comment>Run the audit through uv instead of documenting direct `.venv/bin/python` execution. Use `uv run --with blockbuster --with aiosqlite ...` for these commands and the equivalent pytest invocation so the extra dependencies remain available while following the repository's required environment workflow.</comment>
<file context>
@@ -0,0 +1,405 @@
+
+ uv pip install blockbuster aiosqlite
+ REFLEX_STATE_MANAGER_MODE=memory .venv/bin/python scripts/blockbuster_audit.py
+ REFLEX_STATE_MANAGER_MODE=disk .venv/bin/python scripts/blockbuster_audit.py
+ REFLEX_STATE_MANAGER_MODE=redis REFLEX_REDIS_URL=redis://localhost:6379 \\
+ .venv/bin/python scripts/blockbuster_audit.py
</file context>
| @@ -0,0 +1 @@ | |||
| Keep blocking filesystem and metadata work off the backend event loop: the disk state manager now reads state files in a worker thread, precompressed static file serving stats sidecar files off the loop, and the redis client is created with its library version pinned so redis-py no longer scans `sys.path` on every new connection. | |||
There was a problem hiding this comment.
P3: The middle item of the list has no verb: 'precompressed static file serving stats sidecar files off the loop' is an orphaned noun phrase, while the other two clauses each have a subject and verb. Give it one, e.g. '...sidecar files are resolved off the loop'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At news/7065.performance.md, line 1:
<comment>The middle item of the list has no verb: 'precompressed static file serving stats sidecar files off the loop' is an orphaned noun phrase, while the other two clauses each have a subject and verb. Give it one, e.g. '...sidecar files are resolved off the loop'.</comment>
<file context>
@@ -0,0 +1 @@
+Keep blocking filesystem and metadata work off the backend event loop: the disk state manager now reads state files in a worker thread, precompressed static file serving stats sidecar files off the loop, and the redis client is created with its library version pinned so redis-py no longer scans `sys.path` on every new connection.
</file context>
| Keep blocking filesystem and metadata work off the backend event loop: the disk state manager now reads state files in a worker thread, precompressed static file serving stats sidecar files off the loop, and the redis client is created with its library version pinned so redis-py no longer scans `sys.path` on every new connection. | |
| Keep blocking filesystem and metadata work off the backend event loop: the disk state manager now reads state files in a worker thread, precompressed static file serving stats sidecar files are resolved off the loop, and the redis client is created with its library version pinned so redis-py no longer scans `sys.path` on every new connection. |
1 participant
All Submissions:
Type of change
Changes To Core Features:
What
I ran a representative app under blockbuster (patched into a recording mode so one run collects every blocking call instead of raising on the first) through
AppHarness, driving hydrate/on_load, plain events, computed vars,yieldstreaming, background tasks, uploads, client storage,rx.session/rx.asession,rx.call_script, redirects and re-hydration, once per state manager and once in prod mode with the backend serving the exported frontend. The memory state manager is clean on the event path. Three Reflex-internal blocking calls showed up in the other configurations, fixed here:Disk state manager read state files on the loop.
StateManagerDisk.load_statedidexists()+open()+pickle.loadsynchronously on every cache miss, once per substate. The read now runs in a worker thread like the write already did. The per-writestates_directory.exists()on the loop is gone too; the thread handlesFileNotFoundErrorby recreating the directory.Precompressed static serving stat'd sidecars on the loop.
PrecompressedStaticFiles._select_sidecarcalledPath.stat()inside the syncfile_responsehook, once per accepted encoding per request. Starlette runs its own path lookup in a thread; the sidecar selection now runs in a thread fromget_response, which then finishes the response (Vary/Content-Encoding, conditional 304 check). This also replaces the previous special-case re-route for the 404.html fallback, since the same path covers it.redis-py resolved its version per connection. redis-py >= 7 calls
importlib.metadata.version("redis")in every newConnection.__init__unless the version is passed in, which walkssys.pathand readsMETADATAon the loop. It fired 10 times in one short session (pool growth, lock pubsub, keyspace notifications, token manager).get_redis()now pins it viadriver_infoon 7.x (lib_versionon older releases, imported dynamically so the min-version pyright check stays clean).Each fix has a regression test that fails on the previous code by recording filesystem/metadata calls made on the event loop thread.
scripts/blockbuster_audit.pyis the audit itself (dev per state manager,--prod, and a pytest-plugin mode) so this can be re-run after touchingistate/, the upload path or static serving. Blockbuster only sees syscall-shaped blocking; CPU work on the loop (pickle,deepcopyof defaults,get_type_hintsper event) is out of its scope.Not changed:
rx.get_upload_dir()does amkdir(exist_ok=True)per call from handlers (microseconds; caching it would change behaviour if the directory is removed at runtime), and the docs still showwith rx.session()inside event handlers far more often thanrx.asession(), which for user apps is the biggest real blocker.Testing
tests/units/istate,tests/units/test_state.py,tests/units/utils,tests/units/test_health_endpoint.py: all pass excepttests/units/istate/test_data.py::test_reflex_url_serializes_when_nested_in_router_data, which fails identically onmainin this environment.ruff check/ruff formatclean;pyrightclean on the changed files.🤖 Generated with Claude Code
https://claude.ai/code/session_01CMzuxXkRe8q3t4sUxjrggy
Generated by Claude Code