Skip to content

Commit

Permalink
jobs: don't reset next_run when resuming active schedules
Browse files Browse the repository at this point in the history
Consider an active schedule that was created with a specific first_run
time. The first_run would populate the next_run_time on the schedule.
The resumption of this schedule before it executed would re-evaluate the
next runtime based off the schedule's recurrence.

This commit changes the scheduling system to only recompute the next run
time on paused schedules.

Release justification: bug fix
Release note (bug fix): Fix a bug where resuming an active schedule
would always reset its next run time. This was sometimes undesirable
with schedules that had a first_run option specified.
  • Loading branch information
pbardea committed Aug 30, 2021
1 parent d7d56e8 commit 13b09e7
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
16 changes: 16 additions & 0 deletions pkg/jobs/schedule_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -75,6 +76,21 @@ func TestScheduleControl(t *testing.T) {
require.True(t, th.loadSchedule(t, scheduleID).IsPaused())
})

t.Run("pause-active-schedule", func(t *testing.T) {
schedule := th.newScheduledJob(t, "test schedule", "select 42")
require.NoError(t, schedule.SetSchedule("@weekly"))
firstRunTime := timeutil.Now().Add(10 * time.Second)
schedule.SetNextRun(firstRunTime)
require.NoError(t, schedule.Create(ctx, th.cfg.InternalExecutor, nil))
scheduleID := schedule.ScheduleID()
require.Equal(t, schedule.NextRun(), firstRunTime)
th.sqlDB.Exec(t, "RESUME SCHEDULE $1", scheduleID)

afterSchedule := th.loadSchedule(t, scheduleID)
require.False(t, afterSchedule.IsPaused())
require.Equal(t, afterSchedule.NextRun(), firstRunTime)
})

t.Run("cannot-resume-one-off-schedule", func(t *testing.T) {
schedule := th.newScheduledJob(t, "test schedule", "select 42")
require.NoError(t, schedule.Create(ctx, th.cfg.InternalExecutor, nil))
Expand Down
10 changes: 7 additions & 3 deletions pkg/sql/control_schedules.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,13 @@ func (n *controlSchedulesNode) startExec(params runParams) error {
schedule.Pause()
err = updateSchedule(params, schedule)
case tree.ResumeSchedule:
err = schedule.ScheduleNextRun()
if err == nil {
err = updateSchedule(params, schedule)
// Only schedule the next run time on PAUSED schedules, since ACTIVE
// schedules may have a custom next run time set by first_run.
if schedule.IsPaused() {
err = schedule.ScheduleNextRun()
if err == nil {
err = updateSchedule(params, schedule)
}
}
case tree.DropSchedule:
var ex jobs.ScheduledJobExecutor
Expand Down

0 comments on commit 13b09e7

Please sign in to comment.