This question has been flagged
1 Reply
5167 Views
ormlock
Avatar
Alexandre Fayolle - Camptocamp

I have  a Model called distribution.plan. When the state attribute is "draft", I can add to a distribution.plan record a number of distribution.entitlement records to the plan. Then the user interface has a "confirm" button, when when that button is clicked, a method of the distribution plan model is called to create stock.moves for each entitlement.

My problem is that if 2 users click (or maybe with some browsers if 1 users double clicks)  on the confirm button almost simultaneously, I will get 2 stock move generated for each entitement in the end, which is incorrect.

I can work around this with a SELECT [...] FOR UPDATE NOWAIT statement in the code of the method, but this feels wrong (and it is not an idiom I've encountered in the official addons, it's only used once in stock, and this is not the same use case). So what is the correct (framework-wise) way of dealing with this?

Avatar
Discard
Avatar
Olivier Dony (odo)
Best Answer

If the generation of the stock moves only occurs when the confirmation happens, then the change of `state` of the `distribution.plan` is your safeguard to prevent the transaction happening twice.

Provided that your confirm() method verifies the state is not already confirmed, then the database will prevent two transactions from writing state = 'confirmed' after reading state = 'draft' in parallel. Technically one of the transactions will be rolled back, replayed, and will notice that the plan is already confirmed.

Of course this is based on a number of assumptions regarding the lifecycle of your distribution plan, so your mileage may vary ;-)

Avatar
Discard
Alexandre Fayolle - Camptocamp
Author

OK, I was writing the state at the end of the method, once everything was really done. In case some action does not change the state of my main object (e.g. partial processing of the stock moves), then I'll have no other choice but lock, correct?

Olivier Dony (odo)

Actually, writing the state at the end of the method should work too, as long as it is done in a single transaction. The key part is the "read state" + "write state" sequence of operations that the database should detect as a serialization conflict ("one transaction looked at the state, then updated it, while another one was doing the same thing...obviously one of them should have seen what the other had done first.") As for partial processing, you should perhaps consider having a state on each "entitlement" and use the same mechanism. If you partially process data without maintaining any intermediary state and write the result on unrelated records (new stock moves) then indeed the system would have a hard time detecting anything as a "duplicate" operation, and you would need custom protections such as explicit locking.

Alexandre Fayolle - Camptocamp
Author

You are totally right. After further analysis, it turned out that the server-side code had no check for the current state and would only write the new state, relying on the client-side visibility of the button to prevent the actions from being triggered in the wrong state, which is obviously a very bad idea.