mirror of
https://github.com/inventree/InvenTree.git
synced 2026-08-11 16:06:19 +00:00
[refactor] Improve efficiency of check_missing_pricing (#12572)
This commit is contained in:
@@ -17,6 +17,7 @@ import InvenTree.helpers_model
|
|||||||
from common.settings import get_global_setting
|
from common.settings import get_global_setting
|
||||||
from InvenTree.tasks import (
|
from InvenTree.tasks import (
|
||||||
ScheduledTask,
|
ScheduledTask,
|
||||||
|
batch_offload_tasks,
|
||||||
check_daily_holdoff,
|
check_daily_holdoff,
|
||||||
offload_task,
|
offload_task,
|
||||||
record_task_success,
|
record_task_success,
|
||||||
@@ -265,45 +266,51 @@ def check_missing_pricing(limit=250):
|
|||||||
# Task does not run if the interval is zero
|
# Task does not run if the interval is zero
|
||||||
return
|
return
|
||||||
|
|
||||||
# Find parts for which pricing information has never been updated
|
# Scheduling each part's pricing update calls offload_task(), which (outside of a batch)
|
||||||
results = PartPricing.objects.filter(updated=None)[:limit]
|
# 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:
|
if results.count() > 0:
|
||||||
logger.info('Found %s parts with empty pricing', results.count())
|
logger.info('Found %s parts with empty pricing', results.count())
|
||||||
|
|
||||||
for pp in results:
|
for pp in results:
|
||||||
pp.schedule_for_update()
|
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:
|
if results.count() > 0:
|
||||||
logger.info('Found %s stale pricing entries', results.count())
|
logger.info('Found %s stale pricing entries', results.count())
|
||||||
|
|
||||||
for pp in results:
|
for pp in results:
|
||||||
pp.schedule_for_update()
|
pp.schedule_for_update()
|
||||||
|
|
||||||
# Find any pricing data which is in the wrong currency
|
# Find any pricing data which is in the wrong currency
|
||||||
currency = common.currency.currency_code_default()
|
currency = common.currency.currency_code_default()
|
||||||
results = PartPricing.objects.exclude(currency=currency)
|
results = PartPricing.objects.exclude(currency=currency)[:limit]
|
||||||
|
|
||||||
if results.count() > 0:
|
if results.count() > 0:
|
||||||
logger.info('Found %s pricing entries in the wrong currency', results.count())
|
logger.info(
|
||||||
|
'Found %s pricing entries in the wrong currency', results.count()
|
||||||
|
)
|
||||||
|
|
||||||
for pp in results:
|
for pp in results:
|
||||||
pp.schedule_for_update()
|
pp.schedule_for_update()
|
||||||
|
|
||||||
# Find any parts which do not have pricing information
|
# Find any parts which do not have pricing information
|
||||||
results = Part.objects.filter(pricing_data=None)[:limit]
|
results = Part.objects.filter(pricing_data=None)[:limit]
|
||||||
|
|
||||||
if results.count() > 0:
|
if results.count() > 0:
|
||||||
logger.info('Found %s parts without pricing', results.count())
|
logger.info('Found %s parts without pricing', results.count())
|
||||||
|
|
||||||
for p in results:
|
for p in results:
|
||||||
pricing = p.pricing
|
pricing = p.pricing
|
||||||
pricing.save()
|
pricing.save()
|
||||||
pricing.schedule_for_update()
|
pricing.schedule_for_update()
|
||||||
|
|
||||||
|
|
||||||
@tracer.start_as_current_span('scheduled_stocktake_reports')
|
@tracer.start_as_current_span('scheduled_stocktake_reports')
|
||||||
|
|||||||
@@ -1,5 +1,7 @@
|
|||||||
"""Unit tests for Part pricing calculations."""
|
"""Unit tests for Part pricing calculations."""
|
||||||
|
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
from django.core.exceptions import ObjectDoesNotExist
|
from django.core.exceptions import ObjectDoesNotExist
|
||||||
from django.test.utils import override_settings
|
from django.test.utils import override_settings
|
||||||
|
|
||||||
@@ -431,6 +433,38 @@ class PartPricingTests(InvenTreeTestCase):
|
|||||||
# Check that PartPricing objects have been created
|
# Check that PartPricing objects have been created
|
||||||
self.assertEqual(part.models.PartPricing.objects.count(), 101)
|
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)
|
@override_settings(TESTING_PRICING=True)
|
||||||
def test_delete_part_with_stock_items(self):
|
def test_delete_part_with_stock_items(self):
|
||||||
"""Test deleting a part instance with stock items.
|
"""Test deleting a part instance with stock items.
|
||||||
|
|||||||
Reference in New Issue
Block a user