Skip to content
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

Naive DB state handling and no way to reload the DB state in transaction #19

Open
a3kov opened this issue Aug 12, 2023 · 2 comments
Open

Comments

@a3kov
Copy link

a3kov commented Aug 12, 2023

The current implementation is naive - its assuming that DB state is equal to loaded (runtime) state. A correct implementation would need to assure that with some form of locking. For example, before transitioning, we could lock the row (via a write lock or a simple advisory lock) and re-read the state column, making sure our copy of it is up to date.

With non-transactional transition its ignored completely, but maybe that is fine for people who use it (????).
Now, a user of the library might think it's possible to achieve correctness with transition_multi. However, it has no way to accept the loaded schema inside transaction, and doesn't fetch the record itself. It accepts the schema argument which must be known (loaded) outside of transaction context.

An alternative approach would be to use a transition query with WHERE state=^previous_state, but then it wouldn't work with a changeset.

@nikitalocalhost
Copy link

nikitalocalhost commented Feb 8, 2025

I think Ecto's support for lock and optimistic lock is sufficient, and IMHO from design standpoint it's not up to finite-state machine library to check if app and database is consistent

Correcting myself: I forgot that transition_multi is a function inside fsmx. In that sense i don't see why transition_multi should be here or why it's not locking.

If you need locking, just use optimistic lock or don't use transition_multi and use lock

@nikitalocalhost
Copy link

Maybe best solution is to add something like transition_multi_check(multi, schema, id, new_state, opts) that gets row by Ecto.Multi.one and then checks if supplied state is equal to state returned from query in Ecto.Multi.run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants