2015-06-03 12:40 GMT+02:00 Cédric Krier <[email protected]>: > On 03 Jun 12:12, Albert Cervera i Areny wrote: >> 2015-06-03 11:21 GMT+02:00 Cédric Krier <[email protected]>: >> > On 02 Jun 11:46, Cédric Krier wrote: >> >> On 02 Jun 11:06, Albert Cervera i Areny wrote: >> >> > 2015-05-29 15:39 GMT+02:00 Cédric Krier <[email protected]>: >> >> > > On 29 May 14:40, Albert Cervera i Areny wrote: >> >> > >> Also note that tryton locks the whole table even when two users are >> >> > >> not competing for the same product. >> >> > >> >> >> > >> I created a review [1] some time ago to fix that although it >> >> > >> eventually was commited something much simpler, you can dig in the >> >> > >> reviews, where per product advisory locks were used. >> >> > > >> >> > > Because it doesn't work. It will require a lot of extrat code to be >> >> > > managed correctly: lock at creation, on both side of the write, on >> >> > > delete etc. >> >> > > >> >> > >> >> > I've been revisiting this issue, I don't understand why a lock is >> >> > necessary outside assign_try. >> >> > >> >> > Let me put an scenario: >> >> > >> >> > There's 1 unit of product A on warehouse X. >> >> > >> >> > There are two concurrent transactions: >> >> > >> >> > a) does assign_try to try to ship the product to a customer >> >> > b) moves the product from warehouse X to warehouse Z without >> >> > assign_try (that may include a write, create, or delete, doesn't >> >> > really matter for the case) >> >> > >> >> > So let's see what happens with and without those locks. >> >> > >> >> > If we just have the lock on assign_try there can be two possibilities: >> >> > >> >> > 1.- a) is executed *after* b) so as the product has been moved from X >> >> > to Z and there are 0 units of product A in X, assign_try "fails" and >> >> > stock can not be picked >> >> > 2.- a) is executed *before* b) so we deliver the goods to the customer >> >> > *and* also make the move from X to Z which means that the stock of >> >> > product A is no -1 on warehouse X >> >> > >> >> > We probably don't want the second scenario so we would add the >> >> > suggested lock on write/create/delete. >> >> > >> >> > BUT what happens if those transactions are NOT concurrent and we DO >> >> > have the lock? >> >> > >> >> > Take the second possibility where a) is executed *before* b). What >> >> > happens is that we end up having -1 units of product A on warehouse X >> >> > anyway. >> >> > >> >> > What I see is that as long as you want to ensure that the reservation >> >> > process works as expected *all* stock moves must go through >> >> > *assign_try* because it is this workflow that ensures the expected >> >> > behaviour, not the lock mechanism. >> >> > >> >> > So I don't see any reason why we should add the lock anywhere else >> >> > apart from assign_try. >> >> > >> >> > What am I missing? >> >> >> >> Maybe but there are no proof that finer grain locking system is better >> >> or faster than a big one. For sure, finer grain is more complex and >> >> could have dead locks. >> > >> > Also your analysis takes in consideration only the Tryton's modules. But >> > we could imagine a valid case where you don't use assignation to make >> > move. Like I tried to explain before you could build different workflow >> > for picking which could write directly "done" move and there is no >> > reason to not allow both working at the same time. >> >> But in that case the alternative workflow would have to take care of >> locking anyway. So that doesn't make one option better than the other. >> >> With current implementation it would have to do: >> >> Transaction().cursor.lock(cls._table) >> >> but if an alternative locking mechanism we could provide a function to >> be used by those alternatives to use: >> >> StockMove.lock(products) > > I don't agree. There are no reason such code that create a "done" move > to call lock. It doesn't need to prevent anything from happening. It > will be a design mistake. > Indeed we already have such code in the stock module, it is the > inventory. >
I see your point. >> Even more, it could be considered to add such API even without >> changing the default locking mecanism. >> >> As you said, probably Raimon's scenario may not benefit from my >> proposed locking mechanism but as there are several needs and >> scenarios, maybe it would be interesting to have a common API to try >> to improve compatibility between several alternatives. >> >> Also, maybe it would improve Raimon's situation if instead of raising >> a backtrace a proper message such as "Currently other users are making >> lots of stock moves. Please, try again later." was shown. That >> probably would need to be handled at "trytond" and "stock" module >> level, by providing a new kind of exception which did the retries and >> only after those retries it was risen. > > I don't agree we will be just hidding the problem. Also why asking the > user to try later when trytond can do it with the retry configuration. > Also the problem is not the exception (it is just the revealing), the > problem is the long transaction. > There is always the possibility that the given retry configuration is not enough in an exceptional case. So for me it is better to send an error message that can be understood instead of a backtrace. The exact message could be completely different, I just meant to avoid the backtrace. >> >> More over it will not solve the initial issue as >> >> assigning 300 shipments in one transaction will probably activate the >> >> locks on all products. >> >> Anyway, I will not go further here as far as the pointed performance >> >> enhancements are not solved. > -- > Cédric Krier - B2CK SPRL > Email/Jabber: [email protected] > Tel: +32 472 54 46 59 > Website: http://www.b2ck.com/ -- Albert Cervera i Areny Tel. 93 553 18 03 @albertnan www.NaN-tic.com
