fsmx icon indicating copy to clipboard operation
fsmx copied to clipboard

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

Open a3kov opened this issue 2 years ago • 2 comments

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.

a3kov avatar Aug 12 '23 11:08 a3kov

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 avatar Feb 08 '25 00:02 nikitalocalhost

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

nikitalocalhost avatar Feb 08 '25 00:02 nikitalocalhost