fix(runners,tasks): critical runner requeue and finalization bugs#4004
Closed
cursor[bot] wants to merge 2 commits into
Closed
fix(runners,tasks): critical runner requeue and finalization bugs#4004cursor[bot] wants to merge 2 commits into
cursor[bot] wants to merge 2 commits into
Conversation
When every runner is at capacity, ErrAllRunnersBusy requeued tasks by enqueueing them while they still sat in the running set, then sending EventTypeRequeued from a defer. A periodic queue tick could ClaimAndDequeue the task in that window and start a second dispatch on the same TaskRunner. Release running/active bookkeeping before enqueueing and notify the pool synchronously, matching the reconciler requeue paths. Co-authored-by: Denis Gukov <fiftin@outlook.com>
…finalization Runner client: when applyTerminatedJobs emergency-stops a job and Run() unwinds with an error, the error path overwrote stopped with failed. Add finalizeAfterRun that respects an already-finished status. HA reconciler: when failTaskRunnerLost wins the finalize lock but the DB already has a terminal status, complete finalization instead of bailing so pool/Redis state and autorun children are not leaked. Harden offline requeue with a pre-mutation DB re-check and rollback on persist failure. Co-authored-by: Denis Gukov <fiftin@outlook.com>
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
2 participants
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
Daily critical-bug inspection found three high-severity correctness bugs still present on
develop. This PR applies minimal, previously validated fixes.Bugs and impact
1. Duplicate dispatch on
ErrAllRunnersBusyrequeueImpact: When all runners are at capacity, a task can be dispatched twice (duplicate Ansible/Terraform execution).
Trigger:
TaskRunner.runenqueued the task while it still sat in the running set, then sentEventTypeRequeuedfrom a defer. A concurrent queue tick couldClaimAndDequeuethe task in that window.Fix: Release running/active bookkeeping via
onTaskStopbefore enqueueing; notify the pool synchronously (matching reconciler requeue paths).2. Stopped → failed on runner after
terminated_jobsImpact: Server emergency-stop (
terminated_jobs) is reported to users as failed instead of stopped.Trigger:
applyTerminatedJobssets status to stopped and kills the job; whenRun()unwinds with an error, the error path overwrote the terminal status.Fix: Add
finalizeAfterRunthat respects an already-finished status.3. HA pool state leak in
failTaskRunnerLostImpact: Leaked running/active Redis state; skipped finish webhook and autorun children.
Trigger: Reconciler won the finalize lock but observed the DB already terminal, then returned early without
finalizeRemoteTaskLocked.Fix: Complete finalization when DB is already terminal; harden offline requeue with DB re-check and rollback on persist failure.
Validation
go test ./services/tasks/... ./services/runners/...— all pass (includes new regression tests)Scope inspected (no new critical issues)