You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
With no lock around these reads and writes, it is very easy to get into a situation where this code runs simultaneously on different processes, and puts the user in a strange state.
When using cookies, I don't think there is a solution for this that will be stable. But when using Redis for persistence, it should be possible to be consistent in a concurrent environment.
Honestly after looking into this, I'm not sure that is a problem worth solving for my use case. That's why this is an issue and not a PR, just wanted to capture this problem so it can be discussed.
The text was updated successfully, but these errors were encountered:
Indeed, mixing concurrency/writes/reads without some kind of locks may lead to inconsistency.
Probably most users of Split are using Redis as a default for persistence, but as it's possible to create custom adapters too. So we'd need a way to not couple the solution with Redis or any other database. But still, be able to implement a lock mechanism.
In the meantime I'm planning a refactor around Split::Trial, maybe I can think of something on the way.
The "choose trial" logic is not atomic, so it is easy to run it multiple times for the same user.
Specifically, there is a read here:
split/lib/split/trial.rb
Line 71 in d3ef9d7
With a matching write here:
split/lib/split/trial.rb
Line 83 in d3ef9d7
With no lock around these reads and writes, it is very easy to get into a situation where this code runs simultaneously on different processes, and puts the user in a strange state.
When using cookies, I don't think there is a solution for this that will be stable. But when using Redis for persistence, it should be possible to be consistent in a concurrent environment.
Honestly after looking into this, I'm not sure that is a problem worth solving for my use case. That's why this is an issue and not a PR, just wanted to capture this problem so it can be discussed.
The text was updated successfully, but these errors were encountered: