-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Race condition when registering votes #31
Comments
Pretty surely it comes from commit 4c740e. |
Hmmm, yeah, you're right, there is a chance of race condition, definitely. But I think the commit you mentioned has nothing to do with it. There's always been a chance of race condition, even before that commit. Just when an in-memory database is used, there is no chance of race condition. However, I'm using Redis in production. There is always a network-delay; in case of race condition, the latest request wins. Feel free to look it up @loehnertz, really appreciated for that. But I guess this issue is a tough one 😅 |
Yeah, I think it needs a bigger refactor then to upsert the votes separately from the entire state or do it transactionally. |
By using in-memory db for reading & writing sessions and eventually persist them to redis with a second debounce
I've just released |
We are using the bot weekly in remote pokering in these current times. We overall really like it 👍🏻
However, since the latest release, we are struck by a severe race condition when pokering.
It seems like the bot only registers a single vote at a time, overwriting the internal
Session::votes
state in some way.The effect is also visually noticeable: When the votes come in in quick succession, the checkmarks behind the names of other people that already voted disappear again. The only solution for now is just to try to get the vote into the state by continuously retrying until one's own vote won the current race. This makes it a bit annoying.
I will investigate this myself a bit. It was definitely introduced in one of the last few commits (I think in the latest release).
I'll update this issue if I find something and possibly turn in a PR.
The text was updated successfully, but these errors were encountered: