Only fetch required linked states #6049
Merged
+40
−4
Conversation
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
Optimize the linked state loading path in redis mode by only linking states which are already cached and augmenting `get_state` to link in dynamically fetched states. Adds SharedStateBaseInternal as an _always_dirty_substates from the root state. This causes it to be always shallow-cached when loading any states from redis. Therefore it will be available for use with `_modify_linked_states` without explicitly getting it (and by consequence ALL SharedState instances). Only required SharedState classes will be fetched now.
CodSpeed Performance ReportMerging #6049 will not alter performanceComparing Summary
|
Contributor
Greptile OverviewGreptile SummaryThis PR optimizes linked state loading in Redis mode by only fetching states that are already cached, rather than eagerly loading all linked states. Key Changes:
Issues Found:
Confidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as Event Handler
participant State as State
participant Cache as State Cache
participant Redis as Redis Store
participant SharedBase as SharedStateBaseInternal
Note over User,SharedBase: OLD FLOW (before this PR)
User->>SharedBase: Event triggers
SharedBase->>SharedBase: _modify_linked_states()
Note over SharedBase: Sets up _exit_stack and _held_locks
loop For each linked state
SharedBase->>State: get_state(linked_state_cls)
State->>Cache: Try _get_state_from_cache()
alt Not in cache
Cache-->>State: ValueError
State->>Redis: _get_state_from_redis()
Redis-->>State: state_instance
else In cache
Cache-->>State: state_instance
end
State-->>SharedBase: original_state
SharedBase->>SharedBase: _internal_patch_linked_state(token)
Note over SharedBase: Has context from _modify_linked_states
end
Note over User,SharedBase: NEW FLOW (this PR)
User->>SharedBase: Event triggers
SharedBase->>SharedBase: _modify_linked_states()
Note over SharedBase: Sets up _exit_stack and _held_locks
loop For each linked state
SharedBase->>Cache: _get_state_from_cache(linked_state_cls)
alt Not in cache
Cache-->>SharedBase: ValueError
Note over SharedBase: Skip this state (continue)
else In cache
Cache-->>SharedBase: original_state
SharedBase->>SharedBase: _internal_patch_linked_state(token)
Note over SharedBase: Has context from _modify_linked_states
end
end
Note over User,Redis: ISSUE: When get_state() called later
User->>State: get_state(LinkedState)
State->>Cache: Try _get_state_from_cache()
Cache-->>State: ValueError
State->>Redis: _get_state_from_redis()
Redis-->>State: state_instance
Note over State: Checks if state should be linked
State->>State: state_instance._internal_patch_linked_state(token)
Note over State,Redis: ERROR! No _exit_stack or _held_locks context
State--xUser: ReflexRuntimeError
|
| ) | ||
| is not None | ||
| ): | ||
| return await internal_patch_linked_state(linked_token) |
Contributor
There was a problem hiding this comment.
logic: _internal_patch_linked_state requires being called inside a _modify_linked_states context (it checks self._exit_stack and self._held_locks on line 260-262 of shared.py). However, _get_state_from_redis can be called from get_state() outside of this context, which will cause a ReflexRuntimeError at runtime.
The state_instance fetched from Redis won't have _exit_stack or _held_locks set (these are explicitly excluded in __getstate__ on line 127-128 of shared.py), so calling this method will fail.
Prompt To Fix With AI
This is a comment left during a code review.
Path: reflex/state.py
Line: 2496:2496
Comment:
**logic:** `_internal_patch_linked_state` requires being called inside a `_modify_linked_states` context (it checks `self._exit_stack` and `self._held_locks` on line 260-262 of shared.py). However, `_get_state_from_redis` can be called from `get_state()` outside of this context, which will cause a `ReflexRuntimeError` at runtime.
The `state_instance` fetched from Redis won't have `_exit_stack` or `_held_locks` set (these are explicitly excluded in `__getstate__` on line 127-128 of shared.py), so calling this method will fail.
How can I resolve this? If you propose a fix, please make it concise.
adhami3310
approved these changes
Dec 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Optimize the linked state loading path in redis mode by only linking states which are already cached and augmenting
get_stateto link in dynamically fetched states.Adds SharedStateBaseInternal as an _always_dirty_substates from the root state. This causes it to be always shallow-cached when loading any states from redis. Therefore it will be available for use with
_modify_linked_stateswithout explicitly getting it (and by consequence ALL SharedState instances). Only required SharedState classes will be fetched now.