Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0033581Openbravo ERP07. Sales managementpublic2016-08-01 15:402017-03-16 16:34
Reporteralostale 
Assigned Tovmromanos 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target Version3.0PR17Q2Fixed in Version3.0PR17Q2 
Merge Request Status
Review Assigned Toaferraz
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0033581: slow defaults retrieval in add payment
DescriptionIn an environment with ~6M records in fin_payment it takes long to open Add Payment popup.

The problem is in this part:

AdvPaymentMngtDao.getCustomerPaymentsWithCredit(Organization, BusinessPartner, boolean, Currency) line: 1990
AdvPaymentMngtDao.getCustomerCredit(BusinessPartner, boolean, Organization, Currency) line: 1933
AdvPaymentMngtDao.getCustomerCredit(BusinessPartner, boolean, Organization) line: 1927
SalesInvoiceAddPaymentDefaultValues(AddPaymentDefaultValuesHandler).getDefaultCustomerCredit(Map<String,String>) line: 233

Problems:
- It seems fin_payment table is sequentially scanned
- This code is invoked twice
- (?) is it possible many rows to be returned, if so .list() shouldn't be used
Steps To Reproduce-In an environment with ~6M records in fin_payment
-Create and Invoice and complete it
-Open Add Payemnt
  -Check it takes long to retrieve default values (in prod environment took ~16s)
Proposed Solution
Additional Information
TagsPerformance
Relationships
depends on defect 0033647 closed platform New function in index doesn't work (Oracle only) 
depends on defect 0033659 closed inigosanchez DB consistency error with function index (Oracle only) 
depends on defect 0034488 closed caristu Unsolvable dbconsistency problems in index functions 
depends on defect 0035458 closed platform com.openbravo.test.integration.erp.modules.ci.Upgrade_FS fails with function index 
related to defect 0033629 closed alostale slow order/invoice grid population in Add Payment 
blocks feature request 0035455 new Triage Omni OMS Adapt every query that uses credit to take advantage of FIN_PAYMENT_BPARTNER_CREDIT index 
Attached Filesdiff 33581_17q2.diff (10,181) 2017-03-09 08:37
https://issues.openbravo.com/file_download.php?file_id=10544&type=bug
Issue History
Date ModifiedUsernameFieldChange
2016-08-01 15:40alostaleNew Issue
2016-08-01 15:40alostaleAssigned To => Triage Finance
2016-08-01 15:40alostaleModules => Core
2016-08-01 15:40alostaleTriggers an Emergency Pack => No
2016-08-01 15:46alostaleTag Attached: Performance
2016-08-05 09:32alostaleRelationship addedrelated to 0033629
2016-08-05 11:40vmromanosNote Added: 0088841
2016-08-05 11:40vmromanosStatusnew => scheduled
2016-08-05 11:40vmromanosAssigned ToTriage Finance => vmromanos
2016-08-05 15:16vmromanosFile Added: 33581.diff
2016-08-05 15:17vmromanosNote Added: 0088850
2016-08-05 15:19vmromanosFile Deleted: 33581.diff
2016-08-05 15:19vmromanosFile Added: 33581.diff
2016-08-05 15:20vmromanosFile Deleted: 33581.diff
2016-08-05 15:20vmromanosFile Added: 33581.diff
2016-08-08 09:21vmromanosFile Deleted: 33581.diff
2016-08-08 09:21vmromanosFile Added: 33581.diff
2016-08-08 09:23vmromanosNote Added: 0088873
2016-08-08 10:02vmromanosRelationship addeddepends on 0033647
2016-08-08 12:29vmromanosFile Deleted: 33581.diff
2016-08-08 12:30vmromanosFile Added: 33581.diff
2016-08-08 12:30vmromanosNote Deleted: 0088873
2016-08-08 14:22vmromanosFile Deleted: 33581.diff
2016-08-08 14:22vmromanosFile Added: 33581_final.diff
2016-08-09 08:29vmromanosRelationship addeddepends on 0033659
2016-10-20 09:25vmromanosNote Edited: 0088841bug_revision_view_page.php?bugnote_id=0088841#r13419
2016-11-11 08:42vmromanosFile Deleted: 33581_final.diff
2016-11-11 08:42vmromanosFile Added: 33581_final.diff
2016-11-14 12:21vmromanosRelationship addeddepends on 0034488
2017-03-08 17:54vmromanosRelationship addedblocks 0035455
2017-03-09 08:34vmromanosRelationship addeddepends on 0035458
2017-03-09 08:36vmromanosFile Deleted: 33581_final.diff
2017-03-09 08:37vmromanosFile Added: 33581_17q2.diff
2017-03-13 12:06vmromanosTarget Version => 3.0PR17Q2
2017-03-15 14:46hgbotCheckin
2017-03-15 14:46hgbotNote Added: 0095048
2017-03-15 14:46hgbotStatusscheduled => resolved
2017-03-15 14:46hgbotResolutionopen => fixed
2017-03-15 14:46hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/c29ffbf0c8edaa055eb2311f47824e2ca5125eb4 [^]
2017-03-15 14:47aferrazReview Assigned To => aferraz
2017-03-15 14:47aferrazNote Added: 0095049
2017-03-15 14:47aferrazStatusresolved => closed
2017-03-15 14:47aferrazFixed in Version => 3.0PR17Q2
2017-03-16 16:34hudsonbotCheckin
2017-03-16 16:34hudsonbotNote Added: 0095333

Notes
(0088841)
vmromanos   
2016-08-05 11:40   
(edited on: 2016-10-20 09:25)
Problems:
- It seems fin_payment table is sequentially scanned:
An index in FIN_Payment.C_BPARTNER_ID must be added. However this might not solved the problem if the same business partner has a great percentage of records in this table.

Another function index must be added [c_bpartner_id, haspendingcredit(generated_credit,used_credit)].
The haspendingcredit will return Y when generated_credit - used_credit <> 0, else N.

The query should be modified to take advantage of this index.
Before:
AND generated_credit <> used_credit

After:
AND haspendingcredit(generated_credit, used_credit) = 'Y'


- This code is invoked twice
Reason: one for the default value and another for the display logic.
I would change that behavior only if not enough with DB improvements.


- (?) is it possible many rows to be returned, if so .list() shouldn't be used:
It's very strange to have many credit payments in "real life". Not an issue.
(0088850)
vmromanos   
2016-08-05 15:17   
Attached patch improvements:

BEFORE:
Sort (cost=839247.80..839247.82 rows=9 width=989) (actual time=2751.879..2751.880 rows=2 loops=1)
 
AFTER:
Sort (cost=66314.70..66314.90 rows=77 width=989) (actual time=2.037..2.037 rows=2 loops=1)
(0095048)
hgbot   
2017-03-15 14:46   
Repository: erp/devel/pi
Changeset: c29ffbf0c8edaa055eb2311f47824e2ca5125eb4
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Wed Mar 08 12:50:21 2017 +0100
URL: http://code.openbravo.com/erp/devel/pi/rev/c29ffbf0c8edaa055eb2311f47824e2ca5125eb4 [^]

Fixed bug 33581: slow defaults retrieval in add payment

The FIN_Payment table didn't have an index on the c_bpartner_id column, thus forcing a sequencial scan in any query run over this table.
Added an index on the c_bpartner_id column to fix that problem.

This index should fix most of the cases, however when for example a concrete business partner has an important percentage of the records in that table, the previous index would be useless, and the db might decide to run a seq. scan.
To control this scenario we have extended the previous index with a function to distinguish credit payments (generated credit != used credit). This way we could filter by the credit records for the business partner directly using the index.
Note that the function obequals is declared in the prescript because it must be defined as IMMUTABLE/DETERMINISTIC and this syntaxis is not supported yet by the DBSM.

The AdvPaymentMngtDao.getCustomerPaymentsWithCredit() has been rewritten to take advantage of the new index.

---
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/AdvPaymentMngtDao.java
M src-db/database/model/excludeFilter.xml
M src-db/database/model/prescript-Oracle.sql
M src-db/database/model/prescript-PostgreSql.sql
M src-db/database/model/tables/FIN_PAYMENT.xml
---
(0095049)
aferraz   
2017-03-15 14:47   
Code review + Testing OK
(0095333)
hudsonbot   
2017-03-16 16:34   
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/41ab6f71c85b [^]
Maturity status: Test