Port: Handle transient database exceptions with automatic retry (3.x)#2883
Conversation
Port of #2863: adds automatic retry for transient database exceptions (e.g. SQL Server deadlocks) in ExecuteInNonManagedTXLock with separate TransientRetryInterval (default 1s) and MaxTransientRetries (default 3). Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
There was a problem hiding this comment.
Pull request overview
Ports the transient database exception retry behavior for ADO.NET JobStore from mainline into the 3.x branch, aiming to reduce startup failures (e.g., SQL Server deadlocks) by retrying ExecuteInNonManagedTXLock operations.
Changes:
- Added
MaxTransientRetriesandTransientRetryIntervalconfiguration knobs (defaults: 3 retries, 1s delay) toJobStoreSupport. - Updated
ExecuteInNonManagedTXLockto retry on exceptions detected as transient viaIsTransient(). - Exposed the new knobs through
SchedulerBuilder.PersistentStoreOptionsand added unit tests covering the retry behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Quartz/SchedulerBuilder.cs | Exposes persistent store configuration for transient retry count and delay. |
| src/Quartz/Impl/AdoJobStore/JobStoreSupport.cs | Implements transient retry loop and adds new configurable properties with defaults. |
| src/Quartz.Tests.Unit/Impl/AdoJobStore/JobStoreSupportTest.cs | Adds unit tests verifying transient retry success/failure/disable scenarios. |
| } | ||
|
|
||
| // Delay before the next attempt | ||
| await Task.Delay(TransientRetryInterval, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Task.Delay(TransientRetryInterval, ...) will throw ArgumentOutOfRangeException if TransientRetryInterval is configured negative. Since TransientRetryInterval is publicly settable via config and SchedulerBuilder, it’s worth validating it (>= TimeSpan.Zero) before use (e.g., in Initialize) and/or guarding the delay (only delay when > TimeSpan.Zero).
| await Task.Delay(TransientRetryInterval, cancellationToken).ConfigureAwait(false); | |
| if (TransientRetryInterval > TimeSpan.Zero) | |
| { | |
| await Task.Delay(TransientRetryInterval, cancellationToken).ConfigureAwait(false); | |
| } |
| ConnectionAndTransactionHolder? conn = null; | ||
| try | ||
| int maxRetries = MaxTransientRetries; | ||
| int totalAttempts = maxRetries + 1; |
There was a problem hiding this comment.
MaxTransientRetries is used directly to compute totalAttempts = maxRetries + 1 without any validation. If configured to a negative value (or very large value causing overflow), the retry loop can be skipped and the method will throw InvalidOperationException("...loop exited unexpectedly"), which is confusing and unrelated to the original DB error. Consider validating MaxTransientRetries >= 0 during initialization (fail fast with a config exception) and/or clamping to 0, and compute totalAttempts in a checked context to avoid overflow.
| int totalAttempts = maxRetries + 1; | |
| if (maxRetries < 0) | |
| { | |
| maxRetries = 0; | |
| } | |
| int totalAttempts; | |
| try | |
| { | |
| totalAttempts = checked(maxRetries + 1); | |
| } | |
| catch (OverflowException) | |
| { | |
| totalAttempts = int.MaxValue; | |
| } |
Summary
3.xbranchExecuteInNonManagedTXLockMaxTransientRetries(default 3) andTransientRetryInterval(default 1s) as separate configuration fromDbRetryIntervalSchedulerBuilder.PersistentStoreOptionsTest plan
JobStoreSupportTeststill passes🤖 Generated with Claude Code