diff --git a/config/settings/base.py b/config/settings/base.py index 4cb96e70..f7c1f035 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -551,6 +551,10 @@ def required_host_list(var: str) -> list[str]: "task": "wafer_space.projects.tasks_checks.checks_cleanup_stale_pending_tasks", "schedule": 60.0, }, + "checks-cleanup-superseded": { + "task": "wafer_space.projects.tasks_checks.checks_cleanup_superseded", + "schedule": 60.0, + }, "checks-drc-update-requeue": { "task": "wafer_space.projects.tasks_checks.checks_drc_update_requeue", "schedule": 60.0, diff --git a/docs/celery_tasks_reference.md b/docs/celery_tasks_reference.md index 9116b380..53f8b6c2 100644 --- a/docs/celery_tasks_reference.md +++ b/docs/celery_tasks_reference.md @@ -49,8 +49,8 @@ These tasks run periodically via Celery Beat to poll for checks needing action. | `checks_retry` | `none:ro:checks-orch` | 60s | Create retry checks for ERROR state | | `checks_cleanup_stale_files` | `none:ro:checks-orch` | 60s | Cancel checks on inactive files | | `checks_cleanup_stale_pending_tasks` | `none:ro:checks-orch` | 60s | Remove orphaned task tracking records | +| `checks_cleanup_superseded` | `none:ro:checks-orch` | 60s | Cancel checks superseded by newer ones | | `checks_drc_update_requeue` | `none:ro:checks-orch` | 60s | Create DRC_UPDATE checks for outdated versions | -| `checks_cleanup` | `none:ro:checks-orch` | - | Combined cleanup operations | ### Work Tasks diff --git a/wafer_space/projects/models.py b/wafer_space/projects/models.py index da0eb0f8..2356c75a 100644 --- a/wafer_space/projects/models.py +++ b/wafer_space/projects/models.py @@ -2370,8 +2370,9 @@ def root_check(self) -> "ManufacturabilityCheck": def create_check_drc_update(self) -> "ManufacturabilityCheck": """Create a new pending check to re-run with latest precheck version. - If this check is still in progress, it will be automatically cancelled - by the existing superseded check cleanup logic. + If this check is still in progress, the newly created check supersedes + it; the older in-progress check is then cancelled by the scheduled + ``checks_cleanup_superseded`` task. Returns: The newly created ManufacturabilityCheck. diff --git a/wafer_space/projects/tasks_checks.py b/wafer_space/projects/tasks_checks.py index bd296f9c..23ba1b90 100644 --- a/wafer_space/projects/tasks_checks.py +++ b/wafer_space/projects/tasks_checks.py @@ -191,10 +191,10 @@ def wrapper(check_id: int, *args: Any, **kwargs: Any) -> T | dict[str, str]: __all__ = [ "checks_analyzing", "checks_cancelling", - "checks_cleanup", "checks_cleanup_orphaned_docker", "checks_cleanup_stale_files", "checks_cleanup_stale_pending_tasks", + "checks_cleanup_superseded", "checks_create", "checks_dispatching", "checks_drc_update_requeue", @@ -1948,11 +1948,21 @@ def checks_cleanup_stale_files() -> dict: return {"cancelled": cancelled} -def _cancel_superseded_checks() -> int: +@checks_task() +def checks_cleanup_superseded() -> dict: """Cancel in-progress checks that have been superseded by newer checks. + A check is "superseded" when a newer check exists for the same project + file while the older one is still in progress. This happens via the manual + DRC-update requeue view (``check_drc_update_requeue``), which can create a + fresh check for a file whose latest check is still running. The older, + now-redundant in-progress check is marked for cancellation here. + + The scheduled ``checks_drc_update_requeue`` beat task does not produce this + situation, because it only requeues FINISHED checks. + Returns: - Number of checks marked for cancellation. + Dict with 'cancelled' count of checks marked for cancellation. """ logger = logging.getLogger(__name__) @@ -1962,10 +1972,16 @@ def _cancel_superseded_checks() -> int: created_at__gt=OuterRef("created_at"), ) - # Find all superseded in-progress checks - superseded = ManufacturabilityCheck.objects.filter( - status__in=ManufacturabilityCheck.Status.in_progress(), - ).filter(Exists(newer_exists)) + # Find all superseded in-progress checks. Checks already in CANCELLING + # are excluded: CANCELLING -> CANCELLING is not a valid transition, and + # the checks_cancelling task completes them. + superseded = ( + ManufacturabilityCheck.objects.filter( + status__in=ManufacturabilityCheck.Status.in_progress(), + ) + .exclude(status=ManufacturabilityCheck.Status.CANCELLING) + .filter(Exists(newer_exists)) + ) cancelled = 0 for check in superseded: @@ -1976,40 +1992,10 @@ def _cancel_superseded_checks() -> int: check.id, ) cancelled += 1 - except Exception: + except InvalidStateTransitionError: logger.exception("Failed to cancel superseded check %s", check.id) - return cancelled - - -@checks_task() -def checks_cleanup() -> dict: - """Cleanup task that performs all periodic cleanup operations. - - This task combines multiple cleanup operations: - - Cancel checks superseded by newer checks - - Cancel checks on inactive project files - - Remove orphaned pending task records - - Returns: - Dict with counts of cleanup operations performed. - """ - # Cancel superseded checks - superseded_cancelled = _cancel_superseded_checks() - - # Cancel checks on stale files - stale_files_result = checks_cleanup_stale_files() - stale_files_cancelled = stale_files_result.get("cancelled", 0) - - # Clean up orphaned pending tasks - pending_tasks_result = checks_cleanup_stale_pending_tasks() - pending_tasks_deleted = pending_tasks_result.get("deleted", 0) - - return { - "superseded_cancelled": superseded_cancelled, - "stale_files_cancelled": stale_files_cancelled, - "pending_tasks_deleted": pending_tasks_deleted, - } + return {"cancelled": cancelled} @checks_task() diff --git a/wafer_space/projects/tests/test_tasks.py b/wafer_space/projects/tests/test_tasks.py index 4507aca5..c41f3149 100644 --- a/wafer_space/projects/tests/test_tasks.py +++ b/wafer_space/projects/tests/test_tasks.py @@ -56,8 +56,8 @@ from wafer_space.projects.tasks import do_starting from wafer_space.projects.tasks import download_project_file from wafer_space.projects.tasks_checks import _save_output_gds -from wafer_space.projects.tasks_checks import checks_cleanup from wafer_space.projects.tasks_checks import checks_cleanup_stale_pending_tasks +from wafer_space.projects.tasks_checks import checks_cleanup_superseded from wafer_space.projects.tasks_checks import checks_drc_update_requeue from wafer_space.projects.tasks_download import _apply_post_download_processing from wafer_space.projects.tasks_download import _initialize_hash_calculators @@ -2608,7 +2608,7 @@ def test_returns_zero_when_no_pending_tasks(self) -> None: class TestCancelSupersededChecks: - """Tests for cancel_superseded_checks functionality.""" + """Tests for the checks_cleanup_superseded task.""" @pytest.mark.django_db def test_cancels_older_in_progress_check_when_newer_exists(self) -> None: @@ -2625,7 +2625,7 @@ def test_cancels_older_in_progress_check_when_newer_exists(self) -> None: status=ManufacturabilityCheck.Status.PENDING, ) - checks_cleanup() + checks_cleanup_superseded() old_check.refresh_from_db() assert old_check.status == ManufacturabilityCheck.Status.CANCELLING @@ -2640,7 +2640,7 @@ def test_does_not_cancel_if_no_newer_check(self) -> None: status=ManufacturabilityCheck.Status.RUNNING, ) - checks_cleanup() + checks_cleanup_superseded() check.refresh_from_db() assert check.status == ManufacturabilityCheck.Status.RUNNING @@ -2660,11 +2660,36 @@ def test_does_not_cancel_finished_checks(self) -> None: status=ManufacturabilityCheck.Status.PENDING, ) - checks_cleanup() + checks_cleanup_superseded() old_check.refresh_from_db() assert old_check.status == ManufacturabilityCheck.Status.FINISHED + @pytest.mark.django_db + def test_skips_checks_already_cancelling_without_error_logs( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Checks already in CANCELLING are skipped, not re-cancelled with errors.""" + project_file = ProjectFileFactory() + old_check = ManufacturabilityCheckFactory( + project=project_file.project, + project_file=project_file, + status=ManufacturabilityCheck.Status.CANCELLING, + ) + ManufacturabilityCheckFactory( + project=project_file.project, + project_file=project_file, + status=ManufacturabilityCheck.Status.PENDING, + ) + + with caplog.at_level(logging.ERROR, logger="wafer_space.projects.tasks_checks"): + result = checks_cleanup_superseded() + + assert result == {"cancelled": 0} + assert not caplog.records + old_check.refresh_from_db() + assert old_check.status == ManufacturabilityCheck.Status.CANCELLING + @pytest.mark.django_db class TestChecksDrcUpdateRequeue: