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

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0045909
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] 09. Financial managementmajoralways2021-02-16 17:232021-02-24 11:13
ReportermalsasuaView Statuspublic 
Assigned Tovmromanos 
PrioritynormalResolutionfixedFixed in VersionPR21Q2
StatusclosedFix in branchFixed in SCM revision
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Merge Request Statusapproved
Review Assigned To
OBNetwork customerOBPS
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0045909: Performance problem with conversion rate document functionality

DescriptionWith massive data, the trigger c_conversion_rate_doc_trg2 is taking several seconds for each record.

Analyzing it, it is not efficient, as for each record, it is checking the whole table, not only the record inserted or updated
Steps To Reproducen/a
Proposed Solutionattached a possible fix
TagsNo tags attached.
Attached Filesdiff file icon patch.diff [^] (3,366 bytes) 2021-02-16 17:23 [Show Content]

- Relationships [ Relation Graph ] [ Dependency Graph ]

-  Notes
(0126128)
vmromanos (viewer)
2021-02-16 18:23

 WARNING: The proposed patch would create a mutating table issue as the trigger is querying its own table (that's why the trigger is originally defined at statement level, so the mutating table is avoided).

I tend to think an EventEntityObserver is a better option in this case.
(0126144)
vmromanos (viewer)
2021-02-17 17:08

 Test plan:

Go to Sales Invoice window and create a new header for España - Región Norte.
Go to Exchange Rates tab and create a new record:
  - To Currency: USD
  - Rate: 1.12
Create a new record:
  - To Currency: USD
  - Rate: 1.22
Verify an error is thrown and the second record can't be saved.


This exercise should be repeated in GL Journal, Payment and Financial Transaction, in both Oracle and PG.

Automatic tests should be developed to simplify
(0126149)
hgbot (developer)
2021-02-18 09:54

 Merge Request created: https://gitlab.com/openbravo/product/openbravo/-/merge_requests/316 [^]
(0126265)
hgbot (developer)
2021-02-22 13:52

 Repository: https://gitlab.com/openbravo/ci/backoffice-api [^]
Changeset: 9bc8d27a3ff9c1f3379b0efd9eada871552f59c9
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T13:38:10+01:00
URL: https://gitlab.com/openbravo/ci/backoffice-api/-/commit/9bc8d27a3ff9c1f3379b0efd9eada871552f59c9 [^]

Related to ISSUE-45909: False API change.

The trigger and the unique constraint has been replaced by the new unique index
that controls the same scenario but in an efficient way.

The change in the AD_Message's value is a riskless API change value. It has been
checked that no external module is pointing to this value.

---
M model/src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M model/src-db/database/sourcedata/AD_MESSAGE.xml
R model/src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
---
(0126266)
hgbot (developer)
2021-02-22 13:52

 Merge request merged: https://gitlab.com/openbravo/product/openbravo/-/merge_requests/316 [^]
(0126267)
hgbot (developer)
2021-02-22 13:52

 Directly closing issue as related merge request is already approved.

Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 9c2278bb6b1f1c60f5cc0952d0ca21d47ba903f2
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/9c2278bb6b1f1c60f5cc0952d0ca21d47ba903f2 [^]

Fixed BUG-45909: Remove trigger and improve unique constraint

The trigger has been replaced by an unique index that includes
the currencies and a coalesce between all the possibles document ids
that can be involved in a conversion rate document. Note that at least
one of them must be not-null.

This new unique index is the improved version of the existing unique constraint
(C_CONVERSIONRATEDOC_ONCE) that has been removed as it wasn't working properly.
With the new unique index there is no need to have the C_CONVERSION_RATE_DOC_TRG2
trigger as it was implementing the same functionality that the new unique index is
doing.

Changed existing error message key to match the unique constraint name.

Transform some indexes to partial index after trigger removal, as there is no real
advantage of having those indexes as full indexes anymore.

---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M src-db/database/sourcedata/AD_MESSAGE.xml
R src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
---
(0126268)
hgbot (developer)
2021-02-22 13:52

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: c36d47496705e62a52e53051159b79231554fb0b
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/c36d47496705e62a52e53051159b79231554fb0b [^]

Related to BUG-45909: Added JUnit tests

---
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
M src-test/src/org/openbravo/test/AllAntTaskTests.java
---
(0126269)
hgbot (developer)
2021-02-22 13:52

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: fe647fb22b726595abd4549e0cbf54eb42f9b54a
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/fe647fb22b726595abd4549e0cbf54eb42f9b54a [^]

Related to BUG-45909: JUnit tests stabilization

In Oracle the tests were working fine because the transaction was kept alive,
so the code was able to properly remove the changes in the database in @After.

However, PG has a different transaction management and the transaction was aborted
in the moment the exception was uncaught.

To fix this we make sure the last executed test is the one that must fail, and in
the @After we just rollback the transaction

---
M src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
---
(0126291)
hgbot (developer)
2021-02-23 09:29

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 35593ac8d4485d4d6766e789ead04a246fdeb287
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-23T09:27:05+01:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/35593ac8d4485d4d6766e789ead04a246fdeb287 [^]

Revert "Related to BUG-45909: JUnit tests stabilization"

This reverts commit fe647fb22b726595abd4549e0cbf54eb42f9b54a.

---
M src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
---
(0126292)
hgbot (developer)
2021-02-23 09:29

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 476d70b28656bda72696c345fdabd4ddd6afaeec
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-23T09:27:11+01:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/476d70b28656bda72696c345fdabd4ddd6afaeec [^]

Revert "Related to BUG-45909: Added JUnit tests"

This reverts commit c36d47496705e62a52e53051159b79231554fb0b.

---
M src-test/src/org/openbravo/test/AllAntTaskTests.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
---
(0126293)
hgbot (developer)
2021-02-23 09:29

 Directly closing issue as related merge request is already approved.

Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 63c9b0e40987d93a4f7822432901e6a89fb35aa8
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-23T09:27:13+01:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/63c9b0e40987d93a4f7822432901e6a89fb35aa8 [^]

Revert "Fixed BUG-45909: Remove trigger and improve unique constraint"

This reverts commit 9c2278bb6b1f1c60f5cc0952d0ca21d47ba903f2.

---
A src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M src-db/database/sourcedata/AD_MESSAGE.xml
---
(0126294)
hgbot (developer)
2021-02-23 09:29

 Repository: https://gitlab.com/openbravo/ci/backoffice-api [^]
Changeset: 7fa56c639c747a0b35d92c37a52f0f74eb709665
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-23T09:27:54+01:00
URL: https://gitlab.com/openbravo/ci/backoffice-api/-/commit/7fa56c639c747a0b35d92c37a52f0f74eb709665 [^]

Revert "Related to ISSUE-45909: False API change."

This reverts commit 9bc8d27a3ff9c1f3379b0efd9eada871552f59c9.

---
A model/src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
M model/src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M model/src-db/database/sourcedata/AD_MESSAGE.xml
---
(0126295)
vmromanos (viewer)
2021-02-23 09:32

 Reopened as the original solution breaks retail integration. Scenario where it's reproducible:

Create a ticket and pay in EUR (OK)
Create a similar ticket and pay in USD (paying more amount), and the change in EUR. This fails and it shouldn't.
(0126297)
hgbot (developer)
2021-02-23 11:45

 Merge Request created: https://gitlab.com/openbravo/product/openbravo/-/merge_requests/320 [^]
(0126318)
hgbot (developer)
2021-02-24 11:13

 Merge request merged: https://gitlab.com/openbravo/product/openbravo/-/merge_requests/320 [^]
(0126319)
hgbot (developer)
2021-02-24 11:13

 Directly closing issue as related merge request is already approved.

Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 748fb89b581b5cb36834d5847ba9cad5712396cd
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/748fb89b581b5cb36834d5847ba9cad5712396cd [^]

Fixed BUG-45909: Remove trigger and improve unique constraint

The trigger has been replaced by an unique index that includes
the currencies and a coalesce between all the possibles document ids
that can be involved in a conversion rate document. Note that at least
one of them must be not-null.

This new unique index is the improved version of the existing unique constraint
(C_CONVERSIONRATEDOC_ONCE) that has been removed as it wasn't working properly.
With the new unique index there is no need to have the C_CONVERSION_RATE_DOC_TRG2
trigger as it was implementing the same functionality that the new unique index is
doing.

Changed existing error message key to match the unique constraint name.

Transform some indexes to partial index after trigger removal, as there is no real
advantage of having those indexes as full indexes anymore.

---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M src-db/database/sourcedata/AD_MESSAGE.xml
R src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
---
(0126320)
hgbot (developer)
2021-02-24 11:13

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 7cc16094ae663de3fbf28043b9ab506abf973ea7
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/7cc16094ae663de3fbf28043b9ab506abf973ea7 [^]

Related to BUG-45909: Added JUnit tests

---
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
M src-test/src/org/openbravo/test/AllAntTaskTests.java
---
(0126321)
hgbot (developer)
2021-02-24 11:13

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 6e0bd8ecb0430c066ccf1f2e6478d0f0c0ba9dff
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/6e0bd8ecb0430c066ccf1f2e6478d0f0c0ba9dff [^]

Related to BUG-45909: JUnit tests stabilization

In Oracle the tests were working fine because the transaction was kept alive,
so the code was able to properly remove the changes in the database in @After.

However, PG has a different transaction management and the transaction was aborted
in the moment the exception was uncaught.

To fix this we make sure the last executed test is the one that must fail, and in
the @After we just rollback the transaction

---
M src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
---
(0126322)
hgbot (developer)
2021-02-24 11:13

 Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 088708b9c52257ecce0cbe64df7fd6258556106f
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/088708b9c52257ecce0cbe64df7fd6258556106f [^]

Related to ISSUE-45909: unique index is based on multiple coalesces

The unique index based on the coalesce was not working fine when the same UUID
was shared among different columns in different tuples. This was reproducible
in the case of FIN_PAYMENT_ID and APRM_FINACC_TRANSACTION_V_ID, where both share
the same UUID regardless there were two different tuples. Example:
CURRENCY, CURRENCYTO, FIN_PAYMENT_ID, APRM_FINACC_TRANSACTION_V_ID
100, 102, AAAAA, null
100, 102, null, AAAAA

The fix consists on adding a separate coalesce for each nullable column, that will
return the same dummy value if the column is null. This way we control the scenario
where the same UUID is found in different columns.

---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
---
(0126323)
hgbot (developer)
2021-02-24 11:13

 Repository: https://gitlab.com/openbravo/ci/backoffice-api [^]
Changeset: 53fdddccf5e284eaa8a4d8c64a53c1a15e485cec
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T11:11:26+01:00
URL: https://gitlab.com/openbravo/ci/backoffice-api/-/commit/53fdddccf5e284eaa8a4d8c64a53c1a15e485cec [^]

Related to ISSUE-45909: False API change.

The trigger and the unique constraint has been replaced by the new unique index
that controls the same scenario but in an efficient way.

The change in the AD_Message's value is a riskless API change value. It has been
checked that no external module is pointing to this value.

---
M model/src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M model/src-db/database/sourcedata/AD_MESSAGE.xml
---

- Issue History
Date Modified Username Field Change
2021-02-16 17:23 malsasua New Issue
2021-02-16 17:23 malsasua Assigned To => Triage Finance
2021-02-16 17:23 malsasua File Added: patch.diff
2021-02-16 17:23 malsasua OBNetwork customer => OBPS
2021-02-16 17:23 malsasua Modules => Core
2021-02-16 17:23 malsasua Resolution time => 1614812400
2021-02-16 17:23 malsasua Triggers an Emergency Pack => No
2021-02-16 18:23 vmromanos Note Added: 0126128
2021-02-16 18:23 vmromanos Status new => acknowledged
2021-02-17 17:00 vmromanos Status acknowledged => scheduled
2021-02-17 17:00 vmromanos Assigned To Triage Finance => vmromanos
2021-02-17 17:08 vmromanos Note Added: 0126144
2021-02-18 09:54 hgbot Merge Request Status => open
2021-02-18 09:54 hgbot Note Added: 0126149
2021-02-22 11:47 hgbot Merge Request Status open => approved
2021-02-22 13:52 hgbot Note Added: 0126265
2021-02-22 13:52 hgbot Resolution open => fixed
2021-02-22 13:52 hgbot Status scheduled => closed
2021-02-22 13:52 hgbot Note Added: 0126266
2021-02-22 13:52 hgbot Fixed in Version => PR21Q2
2021-02-22 13:52 hgbot Note Added: 0126267
2021-02-22 13:52 hgbot Note Added: 0126268
2021-02-22 13:52 hgbot Note Added: 0126269
2021-02-23 09:29 hgbot Note Added: 0126291
2021-02-23 09:29 hgbot Note Added: 0126292
2021-02-23 09:29 hgbot Note Added: 0126293
2021-02-23 09:29 hgbot Note Added: 0126294
2021-02-23 09:32 vmromanos Note Added: 0126295
2021-02-23 09:32 vmromanos Status closed => new
2021-02-23 09:32 vmromanos Resolution fixed => open
2021-02-23 09:32 vmromanos Fixed in Version PR21Q2 =>
2021-02-23 11:45 hgbot Note Added: 0126297
2021-02-23 11:48 vmromanos Status new => scheduled
2021-02-24 11:13 hgbot Resolution open => fixed
2021-02-24 11:13 hgbot Status scheduled => closed
2021-02-24 11:13 hgbot Note Added: 0126318
2021-02-24 11:13 hgbot Fixed in Version => PR21Q2
2021-02-24 11:13 hgbot Note Added: 0126319
2021-02-24 11:13 hgbot Note Added: 0126320
2021-02-24 11:13 hgbot Note Added: 0126321
2021-02-24 11:13 hgbot Note Added: 0126322
2021-02-24 11:13 hgbot Note Added: 0126323

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker