perf: increase job concurrency, add channel routing and identity keys#151
perf: increase job concurrency, add channel routing and identity keys#151kneckinator wants to merge 4 commits into19.0from
Conversation
Replace per-record ORM creates and Command.create() tuples with raw SQL INSERT ... ON CONFLICT (unique_cols) DO NOTHING for bulk membership creation. Duplicates are silently skipped and the inserted count is returned via cursor.rowcount. Updates _import_registrants and _add_beneficiaries to use the new skip_duplicates path, with ORM cache invalidation after raw SQL inserts.
Set enrollment_date to current timestamp when state is 'enrolled' in the program membership SQL insert (computed field not triggered by raw SQL). Hoist fields.Date.today() outside the loop in cycle membership to avoid repeated calls per record.
Add context flags (skip_registrant_statistics, skip_program_statistics) that allow bulk operation callers to suppress expensive computed field recomputation. Add refresh_beneficiary_counts() on spp.program and refresh_statistics() on spp.cycle to recompute once at completion. Also replace bool(rec.program_membership_ids) with SQL EXISTS in _compute_has_members to avoid loading the full membership recordset.
Increase parallel-safe channel limits from 1 to 4 (cycle, eligibility_manager, program_manager) now that INSERT ON CONFLICT makes these operations safe for concurrent execution. Add two serial channels: - entitlement_approval (limit=1): fund balance tracking must be serial - statistics_refresh (limit=1): avoid concurrent refresh storms Route entitlement approval/validation jobs to entitlement_approval channel. Route all completion handlers (mark_*_as_done) to statistics_refresh channel. Add identity_key to all async dispatch methods to prevent duplicate job submission when users double-click action buttons.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements bulk membership creation for programs and cycles using raw SQL to improve performance and handle duplicates. It introduces context flags to skip expensive statistics recomputations during bulk operations, adds explicit refresh methods, refactors asynchronous job handling with specific channels and identity keys, and updates queue configurations. Feedback focuses on ensuring UTC timestamps in raw SQL queries and correcting a missing return value to align with method documentation.
| values = [] | ||
| params = [] | ||
| for v in batch: | ||
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") |
There was a problem hiding this comment.
When using raw SQL in Odoo, it is recommended to explicitly use (now() at time zone 'utc') for create_date and write_date. This ensures that the timestamps are stored in UTC regardless of the database session's timezone configuration, adhering to Odoo's standard data storage practices.
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") | |
| values.append("(%s, %s, %s, %s, %s, %s, now() at time zone 'utc', now() at time zone 'utc')") |
| for v in batch: | ||
| state = v.get("state", "draft") | ||
| enrollment_date = now if state == "enrolled" else None | ||
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") |
There was a problem hiding this comment.
When using raw SQL in Odoo, it is recommended to explicitly use (now() at time zone 'utc') for create_date and write_date. This ensures that the timestamps are stored in UTC regardless of the database session's timezone configuration, adhering to Odoo's standard data storage practices.
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") | |
| values.append("(%s, %s, %s, %s, %s, %s, now() at time zone 'utc', now() at time zone 'utc')") |
| } | ||
| for partner_id in beneficiaries | ||
| ] | ||
| self.env["spp.cycle.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) |
There was a problem hiding this comment.
The method _add_beneficiaries is missing a return statement. According to its docstring, it should return an integer representing the count of inserted members. Returning the result of bulk_create_memberships will satisfy this requirement.
| self.env["spp.cycle.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) | |
| return self.env["spp.cycle.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #151 +/- ##
==========================================
+ Coverage 71.06% 71.12% +0.05%
==========================================
Files 925 925
Lines 54704 54797 +93
==========================================
+ Hits 38876 38973 +97
+ Misses 15828 15824 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
1 participant
Summary
cycle,eligibility_manager, andprogram_manager— now safe with INSERT ON CONFLICTentitlement_approvalchannel (limit=1) for fund balance tracking that must remain serialstatistics_refreshchannel (limit=1) to avoid concurrent refresh stormsentitlement_approvalchannelmark_*_as_done) to the serialstatistics_refreshchannelidentity_keyto all async dispatch methods to prevent duplicate job submission on double-clickNote: This PR is based on Phase 8 (#150) which is based on Phase 7 (#149). Will need rebasing after those merge.
Changes
queue_data.xml: Increase limits to 4 for parallel channels, add 2 new serial channelsentitlement_manager_base.py: Route_set_pending_validation,_validate,_canceltoentitlement_approvalentitlement_manager_cash.py: Route_validate_entitlementstoentitlement_approvalentitlement_manager_inkind.py: Route_set_pending_validation,_validatetoentitlement_approvalprogram_manager.py: Addidentity_key, route completion tostatistics_refreshcycle_manager_base.py: Addidentity_key, route completions tostatistics_refresheligibility_manager.py: Addidentity_key, route completion tostatistics_refreshTest plan
./scripts/test_single_module.sh spp_programs— 600 tests, 0 failurespre-commit run --files <changed_files>— all checks pass