Repository navigation
fix(executor): the failure commit is retried like the success commit (#641) - #645
Merged
Merged
Conversation
…t, red (#641) Red (unit, JobExecutorStorageErrorTests): FailureCommitFailsOnce_IsRetried_AndDeadLetterRunsOnce, FailureCommitAlwaysFails_RetriesWithinBudget_NoThrow_NoDeadLetter, ForeignJobCommitFailsOnce_IsRetried_WithRefund and FailureCommit_WithNoRetryDelays_IsASingleCall_AndDoesNotThrow fail with the storage exception escaping ExecuteJobAsync. Red (Postgres, CommitFailureScenarios): failures 3 and 4 never reach the dead-letter handler within 40 s. Guard already green: failures 0.
…atcher's dead-letter (#641) The fault-injecting decorator hides IOrphanedJobReporter from the orphan watcher, so the watcher's dead-letter cannot be observed here (OrphanScenarios covers it). Red without the fix: Postgres failures 3 and 4 find plan.Injected 1.
…641) CommitFailureAsync was a single call: a storage error at the moment a job failed skipped the dead-letter dispatch and reached the dispatcher as an unhandled error. Both commits now share TryCommitAsync (same delays). When the retries are exhausted the job stays Processing, the dead-letter dispatch is skipped and the orphan watcher decides.
|
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
1 participant
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
The commit of a failed (or deferred) job was a single call. If it threw, the dead-letter handler was skipped and the dispatcher logged
Unhandled error while executing job. Both commits now share one helper (TryCommitAsync, same three retries, about 2.6 s). When the retries run out the job staysProcessing, the executor does not call the dead-letter handler, and the orphan watcher decides, as after a crash. All four branches that useCommitFailureAsyncbenefit: job failure, foreign job, throttle deferral and shutdown requeue.Type of change
Red-first proof
28709e4, run against the unfixed code: the four newJobExecutorStorageErrorTestsfailed with the storage exception escapingExecuteJobAsync(Did not expect any exception ... but found System.InvalidOperationException: commit down;blipin the retry cases).eb800a8reworked the shared reliability scenario. Without the fix on Postgres:Expected plan.Injected to be 3 ... but found 1andto be 4 ... but found 1. Guard, already green: 0 failures.CommitFailureTests24 passed across PostgreSQL, SQL Server, MongoDB and Redis.Verification Checklist
dotnet format --verify-no-changespasseddotnet build -c Releasepassed, 0 warningsNexJob.Tests1206 passed on net8.0 and net10.0; core integration subset 154 passed on bothCommitRetryDelaysline of theBuildhelper, which gained an optional parametersrc/NexJob/Internal/JobExecutor.cs) by the core owner--strict: 0 drift; delivery-guarantees has a row for the failure commitCHANGELOG.mdupdated under[Unreleased]/ FixedNotes
IOrphanedJobReporterfrom the watcher. That path is covered byOrphanScenarios.EnableBatchAcknowledgment) is unchanged and its flush failure policy was not part of this issue.Closes #641