Skip to content

[12.0] base_exception: fix concurrency issue - #1639

Closed
gurneyalex wants to merge 2 commits into
OCA:12.0from
gurneyalex:12.0-base_exception_fix_concurrency_error
Closed

[12.0] base_exception: fix concurrency issue#1639
gurneyalex wants to merge 2 commits into
OCA:12.0from
gurneyalex:12.0-base_exception_fix_concurrency_error

Conversation

@gurneyalex

@gurneyalex gurneyalex commented Aug 7, 2019

Copy link
Copy Markdown
Member

When several connections are creating sale orders, the exceptions are checked parallel transactions.

Without this fix:

  • the write() can be a noop but this will nevertheless trigger a UPDATE of the write_uid and write_date automatic fields, which in turn will cause a Rollback of the transaction because of concurrent access. When using the web client, there is a retry, but this causes a slow down. When scripting the creation of sale orders from an external system (e.g. Magento) this causes failed queue.jobs
  • if the write() actually writes something, then we need to serialize the database updates / insertions, otherwise we run into the same problem as above. We do this by locking the rows of the transactions in SQL with SELECT ... FOR UPDATE.

hparfr and others added 2 commits August 7, 2019 13:04
@hparfr @OCA-git-bot
Remove the feature when self is empty.
6b99723 
This recently added feature is counter intuitive, error prone and is
already causing bugs in sale_workflow.
@gurneyalex
[12.0] base_exception: fix concurrency error
ebfa187 
the write will cause updates of the write date even if nothing is written
-> don't call if not necessary

also to be sure, lock the rules while checking, because e.g. with queue jobs you can
get several creations of sale.orders simultaneously and this has been creating rollbacks
because of concurrent updates to exception_rule
@gurneyalex

gurneyalex commented Aug 7, 2019

Copy link
Copy Markdown
Member Author

forward port of #1638

@gurneyalex gurneyalex added this to the 12.0 milestone Aug 7, 2019
guewen
guewen requested changes Aug 14, 2019
View reviewed changes

@guewen guewen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1648 is more complete

@yvaucher

yvaucher commented Sep 5, 2019

Copy link
Copy Markdown
Member

@guewen let's close this one in favor of #1648

@yvaucher yvaucher closed this Sep 5, 2019
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
@bt-admin
Merge pull request OCA#1639 from OCA/16.0
bbc098a 
Syncing from upstream OCA/server-tools (16.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guewen guewen guewen requested changes
@yvaucher yvaucher yvaucher approved these changes
@rousseldenis rousseldenis Awaiting requested review from rousseldenis
@florian-dacosta florian-dacosta Awaiting requested review from florian-dacosta
@hparfr hparfr Awaiting requested review from hparfr

Assignees

No one assigned

Labels

needs review

Projects

None yet

Milestone

12.0

Development

Successfully merging this pull request may close these issues.

4 participants

@gurneyalex @yvaucher @guewen @hparfr