Fix PreviousFireTimeUtc reset to null on restart with OverWriteExistingData (#1834)#2957
Conversation
…ngData=true (#1834) When OverWriteExistingData is true (the default), restarting the application would overwrite PREV_FIRE_TIME in the database with null from the freshly constructed trigger object. This caused context.PreviousFireTimeUtc to always be null on the first execution after a restart. The fix preserves PreviousFireTimeUtc from the existing database trigger when the replacement trigger's PreviousFireTimeUtc is null, which is the common case during application restart. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
5f28af7 to
4ad0d97
Compare
|
There was a problem hiding this comment.
Pull request overview
Fixes an ADO JobStore persistence issue where PreviousFireTimeUtc could be reset to null in the database when triggers are re-stored with replaceExisting=true (commonly on restart with OverWriteExistingData=true), which then surfaces as context.PreviousFireTimeUtc == null.
Changes:
- Preserve
PreviousFireTimeUtcwhen updating an existing trigger if the incoming trigger hasPreviousFireTimeUtc == null. - Add unit tests covering preservation behavior and ensuring explicitly-set values aren’t overridden.
- Add a test helper to invoke the protected
StoreTriggerpath directly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Quartz/Impl/AdoJobStore/JobStoreSupport.cs |
Preserves PreviousFireTimeUtc during trigger replacement by reading the existing trigger before updating. |
src/Quartz.Tests.Unit/Impl/AdoJobStore/JobStoreSupportTest.cs |
Adds tests for the new preservation behavior and a helper to call StoreTrigger. |
| // Preserve PreviousFireTimeUtc from the existing trigger when replacing, | ||
| // so that context.PreviousFireTimeUtc is not lost on application restart (#1834) | ||
| if (newTrigger.GetPreviousFireTimeUtc() is null) | ||
| { | ||
| IOperableTrigger? existingTrig = await Delegate.SelectTrigger(conn, newTrigger.Key, cancellationToken).ConfigureAwait(false); | ||
| var prevFireTime = existingTrig?.GetPreviousFireTimeUtc(); |
There was a problem hiding this comment.
This adds an extra SelectTrigger call when replacing an existing trigger whose PreviousFireTimeUtc is null. SelectTrigger is relatively heavy (loads JobDataMap, schedule details, and may read BLOB triggers), so OverWriteExistingData=true restarts can incur an additional full trigger read per trigger. Consider preserving PREV_FIRE_TIME at the SQL update level instead (e.g., update PREV_FIRE_TIME = COALESCE(@triggerPreviousFireTime, PREV_FIRE_TIME) in SqlUpdateTrigger/SqlUpdateTriggerSkipData) so the existing value is kept without loading the entire trigger from the DB.
…ngData (#1834) When StoreTrigger replaces an existing trigger (e.g., on restart with OverWriteExistingData=true), the new trigger's PreviousFireTimeUtc is null. Now preserves the value from the existing DB trigger via Delegate.SelectTrigger before updating. Port of 3.x PR #2957. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…ngData (#1834) (#2959) When StoreTrigger replaces an existing trigger (e.g., on restart with OverWriteExistingData=true), the new trigger's PreviousFireTimeUtc is null. Now preserves the value from the existing DB trigger via Delegate.SelectTrigger before updating. Port of 3.x PR #2957. Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Summary
PreviousFireTimeUtcbeing reset to null in the database when a trigger is re-stored withreplaceExisting=true(e.g., on application restart withOverWriteExistingData=true, the default)StoreTriggerupdates an existing trigger and the new trigger'sPreviousFireTimeUtcis null, preserve the value from the existing database triggerPreviousFireTimeUtcsetFixes #1834
Test plan
StoreTrigger_PreservesPreviousFireTimeUtc_WhenReplacingExistingTrigger-- verifies the existing trigger'sPreviousFireTimeUtcis carried forwardStoreTrigger_DoesNotOverridePreviousFireTimeUtc_WhenNewTriggerAlreadyHasIt-- verifies that an explicitly set value on the new trigger is not overwritten🤖 Generated with Claude Code