From fe82ff1f9ebe8e27b14476a0d3e08f0fb483dadd Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 8 Aug 2026 19:17:20 +1200 Subject: [PATCH] [refactor] Improve efficiency of check_missing_pricing (#12572) --- src/backend/InvenTree/part/tasks.py | 61 ++++++++++++---------- src/backend/InvenTree/part/test_pricing.py | 34 ++++++++++++ 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/backend/InvenTree/part/tasks.py b/src/backend/InvenTree/part/tasks.py index 296604422b..cec1279247 100644 --- a/src/backend/InvenTree/part/tasks.py +++ b/src/backend/InvenTree/part/tasks.py @@ -17,6 +17,7 @@ import InvenTree.helpers_model from common.settings import get_global_setting from InvenTree.tasks import ( ScheduledTask, + batch_offload_tasks, check_daily_holdoff, offload_task, record_task_success, @@ -265,45 +266,51 @@ def check_missing_pricing(limit=250): # Task does not run if the interval is zero return - # Find parts for which pricing information has never been updated - results = PartPricing.objects.filter(updated=None)[:limit] + # Scheduling each part's pricing update calls offload_task(), which (outside of a batch) + # checks the entire task queue for duplicates on every call. Batching collapses all of + # this run's scheduling into a single bulk write instead, avoiding that per-call scan. + with batch_offload_tasks(): + # Find parts for which pricing information has never been updated + results = PartPricing.objects.filter(updated=None)[:limit] - if results.count() > 0: - logger.info('Found %s parts with empty pricing', results.count()) + if results.count() > 0: + logger.info('Found %s parts with empty pricing', results.count()) - for pp in results: - pp.schedule_for_update() + for pp in results: + pp.schedule_for_update() - stale_date = datetime.now().date() - timedelta(days=days) + stale_date = datetime.now().date() - timedelta(days=days) - results = PartPricing.objects.filter(updated__lte=stale_date)[:limit] + results = PartPricing.objects.filter(updated__lte=stale_date)[:limit] - if results.count() > 0: - logger.info('Found %s stale pricing entries', results.count()) + if results.count() > 0: + logger.info('Found %s stale pricing entries', results.count()) - for pp in results: - pp.schedule_for_update() + for pp in results: + pp.schedule_for_update() - # Find any pricing data which is in the wrong currency - currency = common.currency.currency_code_default() - results = PartPricing.objects.exclude(currency=currency) + # Find any pricing data which is in the wrong currency + currency = common.currency.currency_code_default() + results = PartPricing.objects.exclude(currency=currency)[:limit] - if results.count() > 0: - logger.info('Found %s pricing entries in the wrong currency', results.count()) + if results.count() > 0: + logger.info( + 'Found %s pricing entries in the wrong currency', results.count() + ) - for pp in results: - pp.schedule_for_update() + for pp in results: + pp.schedule_for_update() - # Find any parts which do not have pricing information - results = Part.objects.filter(pricing_data=None)[:limit] + # Find any parts which do not have pricing information + results = Part.objects.filter(pricing_data=None)[:limit] - if results.count() > 0: - logger.info('Found %s parts without pricing', results.count()) + if results.count() > 0: + logger.info('Found %s parts without pricing', results.count()) - for p in results: - pricing = p.pricing - pricing.save() - pricing.schedule_for_update() + for p in results: + pricing = p.pricing + pricing.save() + pricing.schedule_for_update() @tracer.start_as_current_span('scheduled_stocktake_reports') diff --git a/src/backend/InvenTree/part/test_pricing.py b/src/backend/InvenTree/part/test_pricing.py index 468b221d49..2bfd92e967 100644 --- a/src/backend/InvenTree/part/test_pricing.py +++ b/src/backend/InvenTree/part/test_pricing.py @@ -1,5 +1,7 @@ """Unit tests for Part pricing calculations.""" +from unittest import mock + from django.core.exceptions import ObjectDoesNotExist from django.test.utils import override_settings @@ -431,6 +433,38 @@ class PartPricingTests(InvenTreeTestCase): # Check that PartPricing objects have been created self.assertEqual(part.models.PartPricing.objects.count(), 101) + def test_check_missing_pricing_batches_scheduling(self): + """check_missing_pricing() must batch its scheduling calls, not scan the task queue per part. + + Regression test: without batching, each PartPricing.schedule_for_update() call triggers + offload_task() -> check_existing_task(), which scans and unpickles every row in the task + queue to look for a duplicate. Calling this once per part inside check_missing_pricing's + loops made scheduling cost grow with the queue backlog, which is what caused a background + worker timeout in production (Sentry: "Task exceeded maximum timeout value (90 seconds)" + raised from check_existing_task). + """ + from part.tasks import check_missing_pricing + + # Create some parts (deliberately not using TESTING_PRICING here, so that + # schedule_for_update() actually offloads via the task queue rather than running inline) + for ii in range(20): + part.models.Part.objects.create( + name=f'Part_{ii}', description='A test part' + ) + + # Ensure there is no pricing data + part.models.PartPricing.objects.all().delete() + + with self.captureOnCommitCallbacks(execute=True): + with mock.patch('InvenTree.tasks.check_existing_task') as mock_check: + check_missing_pricing() + + # Scheduling was batched - the per-call duplicate-check scan never ran + mock_check.assert_not_called() + + # PartPricing objects were still created and scheduled for a background update + self.assertEqual(part.models.PartPricing.objects.count(), 21) + @override_settings(TESTING_PRICING=True) def test_delete_part_with_stock_items(self): """Test deleting a part instance with stock items.