redis hash for StateManagerRedis #4459
Conversation
|
ci failure is the typical dynamic routes flakyness |
remove oxbow code now that REDIS_URL must be a real redis url
Continue to support redis < 7.2 Instead of updating individual substate expiration, in older versions of redis any update to any substate will cause the entire state expiration to be refreshed, which is better than not supporting older redis versions.
redis-py 5.1.0 adds support for the .hexpire() command
|
@benedikt-bartscher i really like this one 👍 |
|
@masenf thanks for the feedback and compability improvements you have pushed. I really appreciate it. We are already using this since some weeks and did not experience any issues. Will update our fork to the latest changes and keep you posted if everything still works. |
|
No issues so far, thanks again @masenf! |
|
|
Now that all substates are persisted together, there is only 1 warning message expected, not 7
pushed a fix; we only need to expect one warning message now, since we're only writing to redis once per event 😎 |
|
@benedikt-bartscher have you profiled this? i notice that it's making fewer redis calls, but the state updates on reflex-web when bumping the counter on Not clear why it's taking twice as long to get/set the state with the new approach. |
@masenf i have not profiled it yet. Only checked the amount of redis calls. I wonder if the impact is from the actual redis calls or the computation of which states need to be fetched. If this is due to redis hash, we can migrate back to normal redis keys using Pipelines to reduce the amount of requests. Will take a look on this over the weekend. |
|
@masenf I did some basic benchmarking with a redis server running on localhost (which is a bit unfair). Most state manager operations with this branch took longer than on main. I pushed the benchmark if you want to try or investigate. |
Merge remote-tracking branch 'upstream/main' into redis-hash
benedikt-bartscher
force-pushed
the
redis-hash
branch
from
2f52cd7 to
604e96a
Compare
It is not. |
| Set of full state names that may need to be fetched to recalc computed vars. | ||
| """ | ||
| fetch_substates = cls._potentially_dirty_substates() | ||
| for substate_cls in cls.get_substates(): |
There was a problem hiding this comment.
this is not optimal, we only need to loop over the potentially dirty substates
How did you measure this? Just watch the ws timestamps? |
Drastically reduces the amount of redis calls for apps with many states
Keeps all states of a session in one redis hashmap instead of separate redis keys