Conversation
bgentry
commented
Oct 4, 2026
| } | ||
|
|
||
| func insertParamsFromConfigArgsAndOptions(archetype *baseservice.Archetype, config *Config, args JobArgs, insertOpts *InsertOpts) (*rivertype.JobInsertParams, error) { | ||
| func insertParamsFromConfigArgsAndOptions(archetype *baseservice.Archetype, config *Config, args JobArgs, insertOpts *InsertOpts, defaultScheduledAt *time.Time) (*rivertype.JobInsertParams, error) { |
Contributor
Author
There was a problem hiding this comment.
I don't like this change, going to iterate on this more when I get some time (or feel free to do so yourself).
Periodic occurrences can be inserted just before their scheduled time. With `UniqueOpts.ByPeriod`, computing the key before applying that schedule can place the occurrence in the previous period. This can skip a valid occurrence or insert it twice across a leader handoff. Pass the occurrence time into the internal periodic constructor and apply it to copied insertion options before generating the unique key. Keep explicit constructor and job-argument schedules ahead of the occurrence default, and preserve available and pending states for periodic defaults. Cover early inserts caused by another periodic schedule's timer wake, explicit schedule precedence, reused constructor options, and durable occurrence restores on either side of a period boundary.
bgentry
force-pushed
the
bg/periodic-job-period-uniqueness
branch
from
October 4, 2026 19:15
d41036e to
2f43a0d
Compare
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.
An hourly periodic job with
UniqueOpts{ByPeriod: time.Hour}can be skipped at noon even when the 11:00 job has already finished. River uses one timer for all periodic jobs, and whenever that timer wakes it also inserts jobs due within the next 100 milliseconds. For example, a separatePeriodicInterval(time.Minute)job scheduled when a leader starts at 11:58:59.950 wakes the enqueuer at 11:59:59.950. The hourly cron occurrence due at 12:00:00 is then eligible to be inserted 50 milliseconds early.Before this fix, the periodic constructor builds the insert parameters and unique key before the enqueuer supplies the occurrence's scheduled time. With no explicit
ScheduledAt, it rounds the insertion time of 11:59:59.950 down to the 11:00 period. If the 11:00 job is still in the database, the noon insert is skipped as a duplicate: completed jobs are included in the default uniqueness states. The noon occurrence is advanced to the next hour, so it is lost. This is why the bug can occur during an ordinary timer wake, without any clock skew or process failure.Early insertion can also undermine deduplication during an OSS leader handoff. If the first leader inserts the noon occurrence before noon and the next leader starts just before noon, both independently schedule that same occurrence. The new leader's timer can fire at or after noon, producing the 12:00 key instead of the first leader's 11:00 key. If no existing 11:00 key prevented the first insert, both copies can be inserted. Durable periodic jobs commit the job insert and next-run update together; the durable regression deliberately replays an occurrence through a pilot mock to check key stability across the boundary.
Pass the occurrence time through the internal periodic constructor and use it as a fallback in a copy of the constructor options before computing the unique key. Keep the shared insert-param helper and ordinary insertion callers unchanged. Explicit constructor and job-argument schedules retain precedence, and the periodic adapter preserves available or pending state when it supplies the occurrence default. The regression test seeds the previous hour's job, adds another periodic schedule to wake the shared timer early, and freezes the insertion clock just before noon. It checks that the noon occurrence gets its own period key, and that explicit schedules still win.