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

16.0 - [REF] account: Adding unittests for concurrency issues in account_move sequences #104647

Closed

Conversation

moylop260
Copy link
Contributor

@moylop260 moylop260 commented Nov 1, 2022

Related to #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:

Foward-Porting:

Applying the commits from:

@robodoo
Copy link
Contributor

robodoo commented Nov 1, 2022

Pull request status dashboard

@C3POdoo C3POdoo requested review from a team and csnauwaert and removed request for a team November 1, 2022 03:16
@moylop260 moylop260 force-pushed the 16.0-test-concurrency-sequence-moy branch 3 times, most recently from 7b75aca to 44cbf0d Compare November 1, 2022 03:59
@moylop260 moylop260 force-pushed the 16.0-test-concurrency-sequence-moy branch from 44cbf0d to dd4c516 Compare November 3, 2022 17:15
@C3POdoo C3POdoo requested a review from a team November 3, 2022 17:17
moylop260 and others added 2 commits November 17, 2022 13:48
…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
Change the way the uniqueness of the sequence numbers is ensured.
Instead of using a `SELECT FOR UPDATE` approach inspired from the
`ir.sequence`, we can use a constraint approach.

SELECT FOR UPDATE
=================

By doing a FOR UPDATE NO WAIT, we are throwing an exception when another
transaction is creating a move using the same sequence as this one, by
locking the row that holds the current greatest sequence.
Since the row doesn't change, the lock is released and the following
UPDATE is allowed. Because of this, a "useless" UPDATE was always done
on the previous row to ensure a SerializationFailureError if two
concurrent transactions were trying to assign the same number.
This means that in a database with a lot of concurrent transactions
(typically with an online shop), many transactions were
aborted/rollbacked.
This also approach also has the drawback of having the constraint
implemented python side, which is less robust.

UNIQUE CONSTRAINT
=================

Using a constraint on the database level means that the database knows
when some transactions will try to do something similar. It will
therefore make the concurrent transactions wait instead of aborting the
transaction.
However, this approach can't lock only one sequence/journal, it locks
the entire table, which can lead to a different kind of congestion. It
should however always resolve and lead to a result without any
transactions rollbacked.
It is also harder to have the constraint customized by different
modules/localization because it is now done directly in the database and
not in python. Changing the constraint means deleting/recreating the
index. For now, this only happens for Latin America l10n, so a simple
hardcoded approach is implemented.
In order to achieve this, the business code is trying to use the
sequence incrementally until it gets a number it can use for when
multiple transactions are concurrent.
@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

Check the following thread to decide the future of the account_move_name_sequence module since that Odoo v16.0 doesn't allow to configure journals as Gaps-Allowed:

@moylop260 moylop260 closed this Dec 1, 2022
@moylop260 moylop260 deleted the 16.0-test-concurrency-sequence-moy branch December 1, 2022 19:18
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

Successfully merging this pull request may close these issues.

None yet

3 participants