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

[FW][PERF] stock_account: batch _run_fifo_vacuum #165948

Open
wants to merge 1 commit into
base: saas-16.3
Choose a base branch
from

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented May 17, 2024

Currently _run_fifo_vacuum can be a performance bottleneck when confirming a large IN transfer. This is especially true for anglo_saxon accounting companies.

To fix that, this commit introduces batch versions of _run_fifo_vacuum and _create_fifo_vacuum_anglo_saxon_expense_entry. The idea of these batch versions is to batch records creation, moves posting, search, etc. Stuff that cannot be batched/don't need to be batched are left as is.

Speedup

15.0 customer database with 476 000 account.moves, 1M account.move.lines,
8400 products, 700 000 stock.moves, 650 000 stock.move.lines, 500 000 svls
All categories have Inventory Valuation set to real_time.

Benchmark validation of IN transfers, changing the number of products + total number of svls

Number of products Total number of svls Before PR After PR
1 112 3.88s 3.78s
2 607 548ms 524ms
5 2561 2.27s 1.10s
3 8956 15min 2min
32 43310 7min 50s

Some pickings are not directly impacted by the PR, most probably because these pickings don't have candidates svls/svls to
vacuum to begin with. Still for the fourth and fith picking, the batch version performs way better than the iterative
one.

Validating an IN transfer with stock_account and real-time inventory valuation is a complex process so it's a bit difficult to
pinpoint tables cardinalities that correlate with the validation time. Here it's the Before vs After time that is relevant, more than
the validation time growth.


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

Forward-Port-Of: #164295
Forward-Port-Of: #157558

@robodoo
Copy link
Contributor

robodoo commented May 17, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented May 17, 2024

@Aurelienvd @Whenrow cherrypicking of pull request #157558 failed.

stdout:

Auto-merging addons/sale_stock/tests/test_anglo_saxon_valuation_reconciliation.py
Auto-merging addons/stock_account/models/product.py
CONFLICT (content): Merge conflict in addons/stock_account/models/product.py
Auto-merging addons/stock_account/models/stock_move.py
Auto-merging addons/stock_account/tests/test_stockvaluation.py

stderr:

12:38:21.777407 git.c:463               trace: built-in: git cherry-pick f9b9320eac0ae2be6cec82c1abc0c5b4fd60dc1d
error: could not apply f9b9320eac0a... [PERF] stock_account: batch _run_fifo_vacuum
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels May 17, 2024
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label May 17, 2024
@Aurelienvd Aurelienvd force-pushed the saas-16.3-15.0-opw-3727953-avd-wAsC-fw branch from 1041979 to 550de50 Compare May 22, 2024 09:23
@johnw-bluemark
Copy link
Contributor

I hate to barge in here, but this change has been causing an issue in Odoo 16.0. The attached test causes an exception in Odoo 16.0

test_inventory_user_permission.zip

I believe this patch should be applied before forward porting any further

Currently _run_fifo_vacuum can be a performance bottleneck when
confirming a large IN transfer. This is especially true for
anglo_saxon accounting companies.

To fix that, this commit introduces batch versions of _run_fifo_vacuum
and _create_fifo_vacuum_anglo_saxon_expense_entry. The idea of these
batch versions is to batch records creation, moves posting, search, etc.
Stuff that cannot be batched/don't need to be batched are left as is.

opw-3727953

X-original-commit: 22e7bfb
Co-authored-by: Whenrow <whe@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants