|View Issue Details|
|Type||Category||Severity||Reproducibility||Date Submitted||Last Update|
|design defect||[Modules] Remittance||major||always||2015-11-10 18:43||2016-03-17 10:56|
|Priority||urgent||Resolution||fixed||Fixed in Version|
|Status||closed||Fix in branch||Fixed in SCM revision||975b020c2587|
|OS Version||Database version||Ant version|
|Product Version||SCM revision|
|Regression introduced by commit|
|Review Assigned To||dmiguelez|
|Regression introduced in release|
0031419: Processing an invoice with more than 450 lines takes around 7 minutes
|Description||Processing an invoice with more than 450 lines takes around 7 minutes|
|Steps To Reproduce||Create a remittance with more than 450 lines and process it choosing option "no grouping"|
|Attached Files|| 31419_core.diff [^] (17,737 bytes) 2015-12-28 18:09 [Show Content]
31419_remittance_1.diff [^] (6,447 bytes) 2015-12-28 18:09 [Show Content]
31419_remittance_2.diff [^] (8,065 bytes) 2015-12-28 18:09 [Show Content]
31419_final_remittance.diff [^] (2,577 bytes) 2016-02-16 19:10 [Show Content]
Moved to design defect, but attached a solution to be applied under the customer responsibility.
The performance problems for this issue are related to:
1. A few queries in remittance module (almost no impact)
2. Some redundant flushes in remittance module (small impact)
3. Redundant flushes in APRM code called by the remittance module (important impact). This includes: creating a payment, processing it, etc. These performance problems might affect also other flows (not only remittances).
Removing "flushes" is something really dangerous, as it might work in the scenario we are testing but it can create important regressions in other flows (even in 3rd party modules).
I have attached 3 patches to the bug which drastically improve the performance (one for core and the other two for the remittance module).
Some figures: in my computer it originally takes around 900ms to process a remittance line. After my patches it takes around 15ms.
My solution, which is great from a performance point of view as you can see, doesn't really attack the root cause of the issue, but it somehow hides it. The idea is to add a new parameter "doFlush" to the affected methods so I can control when to flush. When called from the remittance flows, the APRM methods don't flush because it is controlled from the Remittance flow.
Besides I have removed also redundant flushes in the remittance code.
This solution should be safe from a regression point of view (only small risk in remittance module), because in other flows the behavior is exactly the same as before (flushing inside the methods). However, I don't want to include it in Core/Remittance because:
1. it doesn't fix the root cause, but it only hides it.
2. it creates more mess in the APRM methods because I need to duplicate the methods to add the new parameter in order to avoid API changes.
|Note that you need to apply the 3 patches at the same time to get a performance improvement|
Author: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com>
Date: Wed Dec 02 19:14:47 2015 +0100
URL: http://code.openbravo.com/erp/mods/org.openbravo.module.remittance/rev/16ea9df2dd32485b3ce9ff5788de4d700b9a5953 [^]
Fixes issue 31419: Processing a remittance with many lines is slow
Modify getRemittanceLines method and create a new one in REM_RemittanceProcess to retrieve only remittance line ids and do not load many entities in memory.
Modify existLineInSettledRemittance method in REM_AddRemittance to retrieve only one object.
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Tue Jan 26 10:09:18 2016 +0100
URL: http://code.openbravo.com/erp/mods/org.openbravo.module.remittance/rev/a2cce8a0ec6e7727078c5d5be18e1f626fdc58c1 [^]
Related to issue 31419: Usage of APRM control of flushes
This changeset makes use of the flush control in several methods called from the remittance process, which drasticall
y recudes the time to process a line from 900ms to 15ms.
Removed useless flushes inside the process.
Avoid quering the database to retrieve the next line No. Instead we use a variable stored in memory.
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Tue Jan 26 17:24:27 2016 +0100
URL: http://code.openbravo.com/erp/devel/pi/rev/975b020c2587eb2e46ef6078b0463e6e88fcf445 [^]
Fixed bug 31419: Add parameter to control flushes in some APRM methods
Added a parameter doFlush to some APRM methods to control whether to flush inside the method. By default it continues to have the same behavior as before (to flush inside the method) so we should avoid any possible regression.
These flushes inside the method makes the process of several payments in a batch really slow, which is a common scenario for the remittance module. With this change (and with the necessary modifications in the remittance module), the time to process a remittance line has been decreased from 900ms to 15ms.
The idea is to create this "self-protected" methods and adapt the flows with performance problems. When the time goes by, we should clean the duplicated methods.
|Code Review + Testing Ok|
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.
Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/b22fb0500156 [^]
Maturity status: Test
|2015-11-10 18:43||maite||New Issue|
|2015-11-10 18:43||maite||Assigned To||=> Triage Finance|
|2015-11-10 18:43||maite||Tag Attached: Performance|
|2015-11-10 18:43||maite||Relationship added||related to 0028809|
|2015-11-10 18:45||maite||Issue Monitored: networkb|
|2015-12-02 18:41||aferraz||Assigned To||Triage Finance => aferraz|
|2015-12-02 18:41||aferraz||Status||new => scheduled|
|2015-12-28 18:06||vmromanos||Assigned To||aferraz => vmromanos|
|2015-12-28 18:06||vmromanos||Status||scheduled => acknowledged|
|2015-12-28 18:09||vmromanos||File Added: 31419_core.diff|
|2015-12-28 18:09||vmromanos||File Added: 31419_remittance_1.diff|
|2015-12-28 18:09||vmromanos||File Added: 31419_remittance_2.diff|
|2015-12-28 18:13||vmromanos||Note Added: 0083024|
|2015-12-28 18:13||vmromanos||Type||defect => design defect|
|2015-12-28 18:15||vmromanos||Note Added: 0083025|
|2016-01-26 10:15||hgbot||Note Added: 0083608|
|2016-01-26 10:15||hgbot||Status||acknowledged => resolved|
|2016-01-26 10:15||hgbot||Resolution||open => fixed|
|2016-01-26 10:15||hgbot||Fixed in SCM revision||=> http://code.openbravo.com/erp/mods/org.openbravo.module.remittance/rev/16ea9df2dd32485b3ce9ff5788de4d700b9a5953 [^]|
|2016-01-26 10:15||hgbot||Note Added: 0083609|
|2016-01-26 10:27||vmromanos||Status||resolved => scheduled|
|2016-02-16 19:10||vmromanos||File Added: 31419_final_remittance.diff|
|2016-02-18 13:24||migueldejuana||Relationship added||blocks 0032044|
|2016-02-26 12:57||hgbot||Note Added: 0084561|
|2016-02-26 12:57||hgbot||Status||scheduled => resolved|
|2016-02-26 12:57||hgbot||Fixed in SCM revision||http://code.openbravo.com/erp/mods/org.openbravo.module.remittance/rev/16ea9df2dd32485b3ce9ff5788de4d700b9a5953 [^] => http://code.openbravo.com/erp/devel/pi/rev/975b020c2587eb2e46ef6078b0463e6e88fcf445 [^]|
|2016-02-26 13:00||dmiguelez||Review Assigned To||=> dmiguelez|
|2016-02-26 13:00||dmiguelez||Note Added: 0084565|
|2016-02-26 13:00||dmiguelez||Status||resolved => closed|
|2016-03-17 10:56||hudsonbot||Note Added: 0085171|
|Copyright © 2000 - 2009 MantisBT Group|