Skip to content

Commit

Permalink
Fix accidental use of non-tx in job scheduler causing intermittency i…
Browse files Browse the repository at this point in the history
…ssues (#332)

Fixes the intermittent testing problem in #321. While tempting to blame
a bug in `SharedTx`, `SharedTx` was actually revealing a problem that
wasn't noticed anywhere else in that the job scheduler was opening a
transaction, then proceeding to not use it in favor of executing on the
raw `s.exec`.

This would've failed every time with a success condition exercised with
`SharedTx`, but because `SharedTx` is only in use with stress test
cases, most of the time the job scheduler would never get far enough to
try and run a `JobSchedule` (usually it fails with a context cancel well
before getting that far). The job scheduler's test suite does use a test
transaction, but `pgx.Tx` apparently continues to allow invocations on a
parent `pgx.Tx` even while a subtransaction exists, I think probably
because the child is really a savepoint rather than a full transaction
of its own.

Fixes #321.
  • Loading branch information
brandur authored May 3, 2024
1 parent 377eac0 commit 9af3be8
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion internal/maintenance/job_scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (s *JobScheduler) runOnce(ctx context.Context) (*schedulerRunOnceResult, er
now := s.TimeNowUTC()
nowWithLookAhead := now.Add(s.config.Interval)

scheduledJobs, err := s.exec.JobSchedule(ctx, &riverdriver.JobScheduleParams{
scheduledJobs, err := tx.JobSchedule(ctx, &riverdriver.JobScheduleParams{
Max: s.config.Limit,
Now: nowWithLookAhead,
})
Expand Down

0 comments on commit 9af3be8

Please sign in to comment.