Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] account: Concurrency errors increased considerably in account.move for Odoo>=v14.0 #91873

Closed
moylop260 opened this issue May 19, 2022 · 10 comments

Comments

@moylop260
Copy link
Contributor

moylop260 commented May 19, 2022

Impacted versions:

14.0, 15.0 and current master futures? (After the following commit: dfd01b8)

Steps to reproduce:

Invoice means:

  • Customer Invoice
  • Customer Invoice Refund
  • Bill Invoice
  • Bill Invoice Refund

Payment means:

  • Customer Payment
  • Customer Payment Refund
  • Supplier Payment
  • Supplier Payment Refund

Scenario 1 - Draft Invoices

  1. Create a new invoice and post it (in order to get a new number and one record to be locked for new records)
  2. Create 2 draft invoices concurrently with 2 different users (no posting only creating in draft)

Scenario 2 - Edit any field of the last posted invoice

  1. Create a new invoice and post it (in order to get a new number and one record to be locked for new records)
  2. Create a new invoice with user A
  3. Edit any field of the last posted invoice at the same time with user B

Scenario 3 - Edit any field of the last posted payment

Same than scenario 2 but using payment

Scenario 4 - Reconcile the last invoice

  1. Reconcile the last invoice with user A
  2. Create a new invoice with user B at the same time

Scenario 5 - Reconcile the last payment

Same than 4 but using payment

Scenario 4 - Create 2 payments

  1. Create payment with user A
  2. Create payment with user B at the same time

Scenario 5 - Create 2 miscellaneous journal entries

  1. Create Journal Entry with user A
  2. Create Journal Entry with user B at the same time

Scenario 6 - Create payment then invoice and invoice then payment

  1. Create a process to make payment after invoice (like payment transaction works in e-commerce)
  2. Create a process to make invoice after invoice payment (like subscriptions case of use)
  3. Run both processes at the same time

Current behavior:

All these cases have concurrent issues
Even the last one has a deadlock one

I have reproduced almost all these cases in the following unittests:

Odoo is not prepared for these kinds of errors since that:

  • Ecommerce raises errors at final users and the sale order is not processed so new payment is requested again and again, double charges since that odoo rollback the whole transaction
  • Subscription set tag Payment Exception and a new manual process is needed
  • if you have many users/process creating invoices, payments or accounting moves the system wait for release last record so consuming more time the workers supporting less users than before showing “loading page” frequently or during more time or even the system down because of there are not more workers available for users
  • If your country could relax the gaps for miscellaneous journal entries or payment you can not configure it. it forces to use no gap for all moves for all companies in the world
  • Reconciling a big payment with many invoices related could be a heavy process so you will not be able to use the system since the last payment could be locked in the reconciliation process and any other process will not be able to lock the last record to get the new numbering

Expected behavior:

We have too many solution to discuss here for future versions (not stable ones)

Possible solution 1

Use a different table to lock the next numbering and don’t allow to request a new number if it is locked from another process to avoid gaps
and only locks if you are posting the move not draft ones

It will fix a lot of concurrent issues even if you are using no-gap like <=v13.0 using ir.sequence

Even It could allow to the user relax the no gaps for a few stressed journals since that the wizard to resequence could allow you to fill the gaps for all the companies or countries where it is strictly needed

Possible solution 2

Prepare odoo for all these “expected” locks, raises, rollbacks
It could be done by creating the crons to assign one by one journal entry numbering
It could be done by creating the journal entry numbering asynchronous
The problem with this is related to cases where you need to make an invoice to make a payment to reconcile together in the same process
So you will need to make the invoice and then wait for numbering finishes
then make payment then wait for numbering finishes
then reconcile

And it could require change many current process of Odoo

Possible solution 3

Analyze what are doing the other big system as Quickbox or SAP

I do not think they are doing the same than Odoo v14.0 since that it could be not usable for big companies with many users

I quickly looked for on the internet the keywords "gaps number invoice {SYSTEM}"

SAP:

https://userapps.support.sap.com/sap/support/knowledge/en/2766068

If your company is based in Italy according to the organizational structure, you should create an incident if you are facing this gap issue.
If your company is not based in Italy, the sequential document number is not applicable.

I think it means, it has an option to configure sequences based in Italy in order to have no-gap
Notice that the exception are no-gap instead but even there is an option to disable it (similar to Odoo v13.0)

Quickbox

Click Filter, then check the Transactions box.
Select Deleted/Voided Transactions for Show.

Gaps are allowed since there is a report to show Deleted/Voided number invoices

Maybe it could be re-created the moves with zero amounts, resequencing or only a report of gaps is enough? IDK

_____ (Another big system)?

... (look for how it is done in other systems)

@yajo
Copy link
Contributor

yajo commented May 23, 2022

Notice that the exception are no-gap instead but even there is an option to disable it (similar to Odoo v13.0)

FWIW Spain is such exception. Translating from RD 1619/2012, article 6.1:

All invoices and copies thereof shall contain the following data or requirements, without prejudice to those that may be mandatory for other purposes and the possibility of including any other information:
(a) Number and, if applicable, series. The numbering of the invoices within each series will be correlative.

So, whatever the solution is, we need to cover both cases. Of course, the no-gap case seems more difficult.

Has anybody explained why did this change? Why the v13's ir.sequence-based was not enough? It worked fine after all... 🤷🏼‍♂️

@moylop260
Copy link
Contributor Author

@yajo
Why the v13's ir.sequence-based was not enough? It worked fine after all... 🤷🏼‍♂️

I think the same

The ir.sequence was working well for +15y even for no-gap features and
it is very versatile without concurrency increased

The author of the new account numbering looks like:

  • image

Actually, see the following new feature for future v16.0:

  • Foo

It means, after all costs, resequence wizard is even needed?

@yajo
Copy link
Contributor

yajo commented May 26, 2022

It means, after all costs, resequence wizard is even needed?

For example in Spain, as quoted in #91873 (comment), resequencing is illegal (in invoice numbers). I really can't understand why the change in v14 if all was working fine before.

If it was just for the UX of being able to let anybody write the invoice number they like (which is BTW very dangerous)... Wouldn't it have been better to just add some computed+inverse fields that mangle with the ir.sequence underneath? That would provide the desired UX without having to reinvent the wheel.

@moylop260 would you be so kind to open an OPW so we know somebody in Odoo reads this thread? Thanks for your work on the matter BTW.

@moylop260
Copy link
Contributor Author

FYI OPW#2879097

@aliencrash
Copy link

@yajo
Why the v13's ir.sequence-based was not enough? It worked fine after all... 🤷🏼‍♂️

I think the same

The ir.sequence was working well for +15y even for no-gap features and it is very versatile without concurrency increased

The author of the new account numbering looks like:

* ![image](https://user-images.githubusercontent.com/6644187/170300919-d0671b4a-f145-44d6-bd9d-35559e4b7fa4.png)

Actually, see the following new feature for future v16.0:

* [![Foo](https://user-images.githubusercontent.com/6644187/170304149-f0f19c09-1c01-4147-b490-dafedd6ad59d.png)](https://twitter.com/ray_odoo/status/1526976153939955712?s=20&t=l1BE_rkgiVsTgvWkEi9chA)

It means, after all costs, resequence wizard is even needed?

This is one of the things Odoo accounting is very naive. Invoices in most of the country is a document that is defined by the government. Specially country with SAFT it's not allowed to change the number of you invoice. Actually make no sense to provide you customer with invoice number XSF/xxxx and then after the customer already reported to the government you have the same invoice with another number. This kind of feature are not features but just gives lot's of problems. See in Portugal that use SAF-T. you are not allowed to change documents number.

@moylop260
Copy link
Contributor Author

moylop260 commented Jun 22, 2022

For record,

I have co-created (original author by @alexis-via) the following module to mitigate these concurrency errors using the v13.0 old way:

It looks more people is affecting this issue, check the following threads:

@sebalix
Copy link
Contributor

sebalix commented Jun 22, 2022

@moylop260 thank you for this module we started to use it. A first fix we did locally was to remove this highest_name field from the account.move form view, because even opening an invoice was triggering an UPDATE query, generating a lot of concurrency issues. This field seems useful only if the user wants to update manually the invoice number... but as @yajo said it's very dangerous to do this and generally not what we want.

@tmotyl
Copy link
Contributor

tmotyl commented Jun 22, 2022

regarding gaps -> in Poland invoice numers should not have gaps, however if the gap occurs, accountant needs to create a statement/document saying why the gap occurred. So there is a workaround for gaps.

@moylop260
Copy link
Contributor Author

For record,

PR related #104345

Thanks a lot!

moylop260 added a commit to vauxoo-dev/odoo that referenced this issue Nov 17, 2022
…e sequences

Related to odoo#90465

Reproducing all the new cases of concurrency for Odoo v14.0 account.move records in the following cases:
- Creating draft invoices
- Creating payments
- Deadlock payment vs invoice
- Editing last invoice and creating new one
- Editing last payment and creating new one
- Reconciling last invoice and creating new one
- Reconciling last payment and creating new one
- No be able to configure to standard sequences anymore

All these kind of errors are raising other kind of errors related to subscription in payment exception, sale order in draft but with transaction, pos order errors, ecommerce users raising errors after pay then request the pay again and again and so on

All workers are used waiting for releases in the queue

Issue describing this:
 - odoo#91873

Foward-Porting:
  - odoo#91525
@moylop260
Copy link
Contributor Author

Fixed for >=v16.0 from #104606
Fixed for >=14.0 and <16.0 from OCA module account_move_name_sequence

moylop260 added a commit to vauxoo-dev/odoo that referenced this issue Apr 22, 2024
…e sequences

Related to odoo#90465

Reproducing all the new cases of concurrency for Odoo v14.0 account.move records in the following cases:
- Creating draft invoices
- Creating payments
- Deadlock payment vs invoice
- Editing last invoice and creating new one
- Editing last payment and creating new one
- Reconciling last invoice and creating new one
- Reconciling last payment and creating new one
- No be able to configure to standard sequences anymore

All these kind of errors are raising other kind of errors related to subscription in payment exception, sale order in draft but with transaction, pos order errors, ecommerce users raising errors after pay then request the pay again and again and so on

All workers are used waiting for releases in the queue

Issue describing this:
 - odoo#91873

Foward-Porting:
  - odoo#91525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants