fix: match compound failed statuses in jobs cleanup and retention#2861
Merged
Conversation
STATUS_VARIANTS mapped 'failed' to an IN ('failed') list, but real
failure statuses are always compound (failed - packet_failure,
failed: policy not satisfied...) and embed arbitrary error text that
can never be enumerated. resolve_status_match() preferred the IN list
for known keys, so count_old_jobs()/delete_old_jobs() matched nothing
for 'failed' — making
a permanent no-op and breaking RetentionFailedJobsTask (same path).
Drop 'failed' from STATUS_VARIANTS so it falls through to the prefix
LIKE 'failed%' branch. LIKE is the honest match for failures: it still
uses idx_status_created (prefix pattern), matches bare 'failed' and
every compound form, and aligns with the LIKE semantics already used
by get_jobs_count(), get_jobs_for_list_table(), and delete_jobs().
The completed IN-optimization is preserved (its variants are genuinely
enumerable).
Fixes #2858
Contributor
Homeboy Results —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wp datamachine jobs cleanup --status=failedwas a permanent no-op becauseSTATUS_VARIANTSmapped'failed'to anIN ('failed')list, but real failure statuses are always compound (failed - packet_failure,failed: Daily memory completion policy was not satisfied...) and embed arbitrary error text that can never be enumerated.resolve_status_match()preferred the exactIN (...)list for known keys, so neithercount_old_jobs()nordelete_old_jobs()matched any compound failure — contradicting the documented contract ondelete_old_jobs()("Uses LIKE matching to handle compound statuses (e.g., 'failed - timeout')") and the LIKE semantics used everywhere else in the file.Fixes #2858.
Root cause
inc/Core/Database/Jobs/Jobs.php:resolve_status_match()returns theIN (...)variant list when the prefix is a known key, only falling back toLIKE 'prefix%'for unknown prefixes. Since'failed'is a known key, every compound failure is invisible. Production evidence from the issue: 194 failed rows, zero with the exact statusfailed.This affected both consumers of
resolve_status_match():JobsCommand::cleanup→count_old_jobs()/delete_old_jobs('failed')(the reported bug).RetentionFailedJobsTask→RetentionCleanup::cleanupFailedJobs()→delete_old_jobs('failed')— shares the exact same bug, so automated retention never purged failed rows either (failed counts grew unboundedly on health dashboards).The fix
Drop
'failed'fromSTATUS_VARIANTSsoresolve_status_match('failed')falls through to theLIKE 'failed%'branch.LIKE is the honest match for failures because failure statuses embed arbitrary error text and can never be enumerated. This approach was chosen over the alternatives for these reasons:
LIKE 'failed%'is a prefix pattern, so MySQL can still use the compositeidx_status_created (status(50), created_at)index for a range scan — the optimization the constant was introduced to preserve.failedtoo. A prefixLIKE 'failed%'matches the bare stringfailed(zero trailing chars) as well as every compound form, so a hypothetical bare-failedrow is still counted/deleted.get_jobs_count(),get_jobs_for_list_table(),build_jobs_summary_where(), anddelete_jobs()already usestatus LIKE 'failed%'for thefailedfilter. TheIN ('failed')list was the outlier that broke the documented contract.completedvariants (completed,completed_no_items,agent_skipped) are genuinely enumerable, so theIN (...)optimization stays for that key.The
completedretention/count paths (RetentionCleanup::cleanupCompletedJobs/countCompletedJobs) are unchanged and verified by tests.Retention-task finding
RetentionFailedJobsTask→RetentionCleanup::cleanupFailedJobs()(inc/Engine/AI/System/Tasks/Retention/RetentionCleanup.php:434) calls(new Jobs())->delete_old_jobs('failed', $max_age_days )— the same broken path. No separate fix is needed: fixingSTATUS_VARIANTSfixes both the CLI and the automated retention task in one place.countFailedJobs()(line 430) is fixed by the same change.Tests
New
tests/jobs-cleanup-status-variants-smoke.phpstubs$wpdbwith an in-memory jobs table and exercisescount_old_jobs()/delete_old_jobs()against representative rows mirroring the production evidence (compound failures, barefailed, completed variants, a recent failed row, a non-terminalprocessingrow). 16 assertions cover:failed - x,failed: y) are counted and deletedfailedstill matchesLIKE 'failed%'(notIN ('failed')) for the failed prefixprocessing) andcompleted-variant rows are untouched by a failed cleanup passcompletedpath keeps theIN ('completed','completed_no_items','agent_skipped')optimization and matches its 3 variantscompleteddelete leaves failed rows untouchedVerification
php tests/jobs-cleanup-status-variants-smoke.php).homeboy review data-machine lint --changed-since origin/main: PASS (phpcs + phpstan clean).jobs-list-efficiency,jobs-get-children,jobs-command-undo-fanout,jobs-trim-runtime-queuesall pass).Constraints honored
CHANGELOG.mdedit, no version bump (homeboy owns both).fix:).