Issue 10870

Title
Lock table is not enough to ensure to read last committed values
Priority
bug
Status
testing
Nosy list
acaubet, albertca, ced, reviewbot, yangoon
Assigned to
ced
Keywords
review

Created on 2021-10-18.00:52:57 by ced, last changed 2 weeks ago by reviewbot.

Messages

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-04.15:43:15
On 2022-10-04 15:26, Albert Cervera i Areny wrote:
> - It gives little granularity on locking. We already have problems when there're table-level locks (using version 6.0 locks the whole sale_sale table and that produces too much contention).

Of course the proposed design does not change the lock granularity.

> - It still does not prevent the issue because when the lock occurs the transaction has already been started

It is by design impossible to lock outside a transaction.

> and so we can still have a scenario like the one described in issue11763: when the transaction starts the concurrent transaction is still running but when we try to acquire the lock the concurrent transaction has finished. This means that the second transaction can continue but the data it is seeing is already old.

No this is wrong. As explained by PostgreSQL documentation, it is not
the case as long as the lock is done before any SELECT or data
modification.
Author: [hidden] (albertca)
Date: 2022-10-04.15:31:31

I mistakenly stated that in 6.0 a complete lock is held. That is not correct, as SELECT FOR UPDATE is used. Still, I think the other arguments apply.

Author: [hidden] (albertca)
Date: 2022-10-04.15:26:41

For me, the proposal of locking all the tables at the beginning of the transaction has two problems:

  • It gives little granularity on locking. We already have problems when there're table-level locks (using version 6.0 locks the whole sale_sale table and that produces too much contention).
  • It still does not prevent the issue because when the lock occurs the transaction has already been started and so we can still have a scenario like the one described in issue11763: when the transaction starts the concurrent transaction is still running but when we try to acquire the lock the concurrent transaction has finished. This means that the second transaction can continue but the data it is seeing is already old.

So the only way I can think we can solve the issue is by locking before the transaction has been started, probably by using lock_id() in another transaction within the same RPC call.

It could be defined something like this:

cls.__rpc__['method'].locks = lambda ids: ids

If the dispatcher, found that locks is not empty it creates a transaction and inside it calls "lock_id(id)" for each of the ids returned by the method (calling it with all the IDs of the RPC request).

If all succeeded, then the usual transaction is called for the 'method'.

At the very end, the transaction with the locks is rolled back (or committed).

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-10-18.01:22:34

This implementation could solve definitely the problem of issue6134.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-10-18.00:52:55

For now we lock tables during the execution of the code but as the doc explains it is not enough to ensure to read the latest values. Indeed we should lock the needed tables before any SELECT or UPDATE. The problem is that with the modularity and the OOP design of Tryton we do not know when starting the transaction which tables need to be locked.

I think a possible design would be to add an argument to Transaction.start as a list of tables to lock. Now when the backend is called to lock a table, it checks if it is a table already locked by the start of the transaction. If it is not than it raises an exception that is catched by the starter of the transaction which is responsible for adding the table to the list to lock when re-starting the transaction.

Those difficulty could be avoided by using SERIALIZABLE isolation. This mode is not available for now (we replaced by REPEATABLE READ) but maybe we could allow to declare the need for such transaction (maybe depending on the tables to lock) and in this case the LOCK calls and SELECT FOR could be ignored.

History
Date User Action Args
2022-11-18 19:52:50reviewbotsetmessages: + msg80109
nosy: + reviewbot
2022-11-18 19:47:26cedsetassignedto: ced
component: + proteus
keyword: + review
reviews: 441761003
status: chatting -> testing
2022-10-04 15:43:16cedsetmessages: + msg78571
2022-10-04 15:31:31albertcasetmessages: + msg78570
2022-10-04 15:26:41albertcasetmessages: + msg78569
nosy: + albertca
2022-10-04 14:58:49acaubetsetnosy: + acaubet
2022-10-04 14:50:19cedlinkissue11763 superseder
2022-02-05 10:05:09cedsetmessages: - msg71064
2022-02-05 10:05:05cedsetmessages: - msg71063
2021-10-18 09:03:46yangoonsetmessages: + msg71064
2021-10-18 09:00:51yangoonsetmessages: + msg71063
nosy: + yangoon
status: unread -> chatting
2021-10-18 01:22:34cedsetmessages: + msg71059
2021-10-18 00:52:57cedcreate