diff --git a/api/caching/tasks.py b/api/caching/tasks.py index 9a3a0264cc2..2ed1cefd4bd 100644 --- a/api/caching/tasks.py +++ b/api/caching/tasks.py @@ -1,6 +1,7 @@ import logging from urllib.parse import urlparse +from billiard.exceptions import SoftTimeLimitExceeded from django.apps import apps from django.db import connection from django.db.models import Sum @@ -113,14 +114,23 @@ def ban_url(instance): ) -@app.task(max_retries=5, default_retry_delay=10) +@app.task( + max_retries=5, + default_retry_delay=10, + soft_time_limit=270, + time_limit=300, +) def update_storage_usage_cache(target_id, target_guid, per_page=_DEFAULT_FILEVERSION_PAGE_SIZE): if not settings.ENABLE_STORAGE_USAGE_CACHE: return from osf.models import Guid - storage_usage_total = compute_storage_usage_total(Guid.load(target_guid).referent, per_page=per_page) - key = cache_settings.STORAGE_USAGE_KEY.format(target_id=target_guid) - storage_usage_cache.set(key, storage_usage_total, settings.STORAGE_USAGE_CACHE_TIMEOUT) + try: + storage_usage_total = compute_storage_usage_total(Guid.load(target_guid).referent, per_page=per_page) + key = cache_settings.STORAGE_USAGE_KEY.format(target_id=target_guid) + storage_usage_cache.set(key, storage_usage_total, settings.STORAGE_USAGE_CACHE_TIMEOUT) + except SoftTimeLimitExceeded: + logger.exception('Storage usage cache update timed out for target %s', target_guid) + raise def compute_storage_usage_total(target_obj, per_page=_DEFAULT_FILEVERSION_PAGE_SIZE): diff --git a/api/caching/test_tasks.py b/api/caching/test_tasks.py new file mode 100644 index 00000000000..3c55d22903a --- /dev/null +++ b/api/caching/test_tasks.py @@ -0,0 +1,51 @@ +import ast +from pathlib import Path + + +def _storage_usage_task(): + module = ast.parse(Path(__file__).with_name('tasks.py').read_text()) + return module, next( + node + for node in module.body + if isinstance(node, ast.FunctionDef) and node.name == 'update_storage_usage_cache' + ) + + +def test_update_storage_usage_cache_has_time_limits(): + _, task = _storage_usage_task() + decorator = next( + node + for node in task.decorator_list + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) and node.func.attr == 'task' + ) + options = {keyword.arg: ast.literal_eval(keyword.value) for keyword in decorator.keywords} + + assert options['soft_time_limit'] == 270 + assert options['time_limit'] == 300 + + +def test_update_storage_usage_cache_logs_guid_and_reraises_soft_timeout(): + module, task = _storage_usage_task() + assert any( + isinstance(node, ast.ImportFrom) + and node.module == 'billiard.exceptions' + and any(alias.name == 'SoftTimeLimitExceeded' for alias in node.names) + for node in module.body + ) + handler = next( + node + for node in ast.walk(task) + if isinstance(node, ast.ExceptHandler) + and isinstance(node.type, ast.Name) + and node.type.id == 'SoftTimeLimitExceeded' + ) + log_call = next( + node + for node in ast.walk(handler) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr == 'exception' + ) + + assert any(isinstance(arg, ast.Name) and arg.id == 'target_guid' for arg in log_call.args) + assert any(isinstance(node, ast.Raise) for node in handler.body)