Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue DetailsJump to Notes ] Issue History ] Print ]
ID
0039740
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Modules] Advanced Warehouse Operationsmajoralways2018-12-04 11:332018-12-11 14:03
ReporteregoitzView Statuspublic 
Assigned Tovmromanos 
PriorityurgentResolutioninvalidFixed in Version
StatusclosedFix in branchFixed in SCM revision
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Regression date
Regression introduced by commit
Regression level
Review Assigned Tovmromanos
Regression introduced in release
Summary

0039740: Database contention on the BatchOfTasksGenerator class

DescriptionWhen executing the method createBatchOfTasksUsingItt the sequence of the document is obtained. That is locked until the process finishes.
Steps To Reproduce-Execute the process simultaneously with two users and one process should wait till the other finishes to be able to get the documentno.


Check the code.
Proposed Solution-Set a dummy documentno during the execution and set the final and correct sequence number at the end.
TagsPerformance
Attached Files

- Relationships Relation Graph ] Dependency Graph ]
related to design defect 0039786 acknowledgedvmromanos Too aggressive lock in Batch Of Tasks Generator 

-  Notes
(0108382)
vmromanos (manager)
2018-12-11 11:17

The lock is absolutely necessary at this point and it avoids many possible issues due to concurrency. Removing or relaxing the lock is very dangerous and may corrupt the task generation or stock status.

The CentralBroker must act as a Singleton, with only one request running at the same time. The lock at BatchOfTasks sequence is actually the responsible of this.
If we allow multiple instances of the batch of tasks generator they can create conflicts between them meanwhile they are running, specially if we implement the proposed solution.

Let's see an example if we remove the lock:
Imagine we only have 1 unit of "MyProduct" available in the warehouse. No overissue is allowed.

Two sales orders (SO-1 and SO-2) have 1 unit of MyProduct in any of their lines.

User A launches the picking for SO-1 and user B launches the picking for SO-2 almost at the same time.

SO-1 reaches first the task generation for MyProduct, so it creates the task, and continues with the rest of the lines.

SO-2 reaches MyProduct line. If we relax the lock as in the proposed solution, the task generation will find also the MyProduct unit and it will create another task, which is wrong because there is only 1 unit in the stock and it has been already associated to SO-1.

You could also think in committing the SO-1 task generation so SO-2 wouldn't find the stock. This would imply a performance penalty in each of the tasks commits and it won't really ensure the commit is done just in time. Another lock would be necessary meanwhile the individual task is being generated and committed. So the performance improvement compared with the current global lock at bathoftasks would be very low (if any) and the complexity of the process will drastically increase.

Let's continue with the example supposing we implement the individual lock explained above instead of the current global lock. Imagine SO-2 finishes first (for example because it has less lines). The MyProduct task won't be generated, because SO-1 have found it first.
Now imagine SO-1 task generation throws an exception, for example due to a configuration error. In this case the AWO general rule is very clear: the whole transaction is rollback. So the task generated for MyProduct will be deleted.

At the end no picking task for MyProduct will be created neither for SO-1 nor for SO-2, regardless the product is available in the warehouse.




The same example with the current global lock would work fine:
If SO-1 is the first one that blocks the task generation, the exception will be thrown, so MyProduct will be available for SO-2.

If SO-2 is the first one that blocks the task generation, SO-1 won't find it because the task is already created for SO-2 (regardless the exception that we know it is thrown).


Summary: the stock is a unique physical resource, so it must be controlled by a unique Task Generator. That's why the lock is required.
The task generation in general is very fast and the lock shouldn't affect too much from a performance POV, specially if compared with the benefits it adds.
(0108389)
hgbot (developer)
2018-12-11 14:03

Repository: erp/pmods/org.openbravo.warehouse.advancedwarehouseoperations
Changeset: 69bc88d902f94af3a1fdf8e538858483cb7a2842
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Tue Dec 11 14:02:58 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.warehouse.advancedwarehouseoperations/rev/69bc88d902f94af3a1fdf8e538858483cb7a2842 [^]

Related to issue 39740: Added code documentation to clarify behavior

---
M src/org/openbravo/warehouse/advancedwarehouseoperations/centralbroker/BatchOfTasksGenerator.java
---

- Issue History
Date Modified Username Field Change
2018-12-04 11:33 egoitz New Issue
2018-12-04 11:33 egoitz Assigned To => Triage Finance
2018-12-04 11:33 egoitz Resolution time => 1545087600
2018-12-04 11:56 galderromo Issue Monitored: galderromo
2018-12-04 12:15 Sandrahuguet Assigned To Triage Finance => vmromanos
2018-12-05 08:08 Sandrahuguet Tag Attached: Performance
2018-12-05 16:02 vmromanos Status new => acknowledged
2018-12-11 10:35 vmromanos Status acknowledged => scheduled
2018-12-11 11:17 vmromanos Review Assigned To => vmromanos
2018-12-11 11:17 vmromanos Note Added: 0108382
2018-12-11 11:17 vmromanos Status scheduled => closed
2018-12-11 11:17 vmromanos Resolution open => invalid
2018-12-11 14:03 hgbot Checkin
2018-12-11 14:03 hgbot Note Added: 0108389
2018-12-12 09:30 vmromanos Relationship added related to 0039786
2022-09-06 17:18 caristu Category Advance Warehouse Operations => Advanced Warehouse Operations


Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker