Skip to content

Commit

Permalink
Fix cron for schedules that do not match any time (#4206)
Browse files Browse the repository at this point in the history
* Fix cron for schedules that do not match any time

* Use NoBackoff
  • Loading branch information
yiminc authored Apr 22, 2023
1 parent d6c298b commit ba09219
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 25 deletions.
16 changes: 13 additions & 3 deletions common/backoff/cron.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,14 @@ func ValidateSchedule(cronSchedule string) error {
if cronSchedule == "" {
return nil
}
if _, err := cron.ParseStandard(cronSchedule); err != nil {
return serviceerror.NewInvalidArgument("Invalid CronSchedule.")
schedule, err := cron.ParseStandard(cronSchedule)
if err != nil {
return serviceerror.NewInvalidArgument("invalid CronSchedule.")
}
nextTime := schedule.Next(time.Now().UTC())
if nextTime.IsZero() {
// no time can be found to satisfy the schedule
return serviceerror.NewInvalidArgument("invalid CronSchedule, no time can be found to satisfy the schedule")
}
return nil
}
Expand All @@ -68,10 +74,14 @@ func GetBackoffForNextSchedule(cronSchedule string, scheduledTime time.Time, now
} else {
nextScheduleTime = schedule.Next(scheduledUTCTime)
// Calculate the next schedule start time which is nearest to now (right after now).
for nextScheduleTime.Before(nowUTC) {
for !nextScheduleTime.IsZero() && nextScheduleTime.Before(nowUTC) {
nextScheduleTime = schedule.Next(nextScheduleTime)
}
}
if nextScheduleTime.IsZero() {
// no time can be found to satisfy the schedule
return NoBackoff
}

backoffInterval := nextScheduleTime.Sub(nowUTC)
roundedInterval := time.Second * time.Duration(convert.Int64Ceil(backoffInterval.Seconds()))
Expand Down
51 changes: 29 additions & 22 deletions common/backoff/cron_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,30 @@ var crontests = []struct {
scheduledTime string
now string
result time.Duration
err string
}{
{"0 10 * * *", "2018-12-17T08:00:00-08:00", "", time.Hour * 18},
{"0 10 * * *", "2018-12-18T02:00:00-08:00", "", time.Hour * 24},
{"0 * * * *", "2018-12-17T08:08:00+00:00", "", time.Minute * 52},
{"0 * * * *", "2018-12-17T09:00:00+00:00", "", time.Hour},
{"* * * * *", "2018-12-17T08:08:18+00:00", "", time.Second * 42},
{"0 * * * *", "2018-12-17T09:00:00+00:00", "", time.Minute * 60},
{"0 10 * * *", "2018-12-17T08:00:00+00:00", "2018-12-20T00:00:00+00:00", time.Hour * 10},
{"0 10 * * *", "2018-12-17T08:00:00+00:00", "2018-12-17T09:00:00+00:00", time.Hour},
{"*/10 * * * *", "2018-12-17T00:04:00+00:00", "2018-12-17T01:02:00+00:00", time.Minute * 8},
{"invalid-cron-spec", "2018-12-17T00:04:00+00:00", "2018-12-17T01:02:00+00:00", NoBackoff},
{"@every 5h", "2018-12-17T08:00:00+00:00", "2018-12-17T09:00:00+00:00", time.Hour * 4},
{"@every 5h", "2018-12-17T08:00:00+00:00", "2018-12-18T00:00:00+00:00", time.Hour * 4},
{"0 3 * * 0-6", "2018-12-17T08:00:00-08:00", "", time.Hour * 11},
{"@every 30s", "2020-07-17T09:00:02-01:00", "2020-07-17T09:00:02-01:00", time.Second * 30},
{"@every 30s", "2020-07-17T09:00:02-01:00", "2020-09-17T03:00:53-01:00", time.Second * 9},
{"@every 30m", "2020-07-17T09:00:00-01:00", "2020-07-17T08:45:00-01:00", time.Minute * 15},
{"0 0 31 2 *", "2018-12-17T08:00:00-08:00", "", 0, "invalid CronSchedule, no time can be found to satisfy the schedule"},
{"invalid-cron-spec", "2018-12-17T00:04:00+00:00", "2018-12-17T01:02:00+00:00", NoBackoff, "invalid CronSchedule"},

{"0 10 * * *", "2018-12-17T08:00:00-08:00", "", time.Hour * 18, ""},
{"0 10 * * *", "2018-12-18T02:00:00-08:00", "", time.Hour * 24, ""},
{"0 * * * *", "2018-12-17T08:08:00+00:00", "", time.Minute * 52, ""},
{"0 * * * *", "2018-12-17T09:00:00+00:00", "", time.Hour, ""},
{"* * * * *", "2018-12-17T08:08:18+00:00", "", time.Second * 42, ""},
{"0 * * * *", "2018-12-17T09:00:00+00:00", "", time.Minute * 60, ""},
{"0 10 * * *", "2018-12-17T08:00:00+00:00", "2018-12-20T00:00:00+00:00", time.Hour * 10, ""},
{"0 10 * * *", "2018-12-17T08:00:00+00:00", "2018-12-17T09:00:00+00:00", time.Hour, ""},
{"*/10 * * * *", "2018-12-17T00:04:00+00:00", "2018-12-17T01:02:00+00:00", time.Minute * 8, ""},
{"@every 5h", "2018-12-17T08:00:00+00:00", "2018-12-17T09:00:00+00:00", time.Hour * 4, ""},
{"@every 5h", "2018-12-17T08:00:00+00:00", "2018-12-18T00:00:00+00:00", time.Hour * 4, ""},
{"0 3 * * 0-6", "2018-12-17T08:00:00-08:00", "", time.Hour * 11, ""},
{"@every 30s", "2020-07-17T09:00:02-01:00", "2020-07-17T09:00:02-01:00", time.Second * 30, ""},
{"@every 30s", "2020-07-17T09:00:02-01:00", "2020-09-17T03:00:53-01:00", time.Second * 9, ""},
{"@every 30m", "2020-07-17T09:00:00-01:00", "2020-07-17T08:45:00-01:00", time.Minute * 15, ""},
// At 16:05 East Coast (Day light saving on)
{"CRON_TZ=America/New_York 5 16 * * *", "2021-03-14T00:00:00-04:00", "2021-03-14T15:05:00-04:00", time.Hour * 1},
{"CRON_TZ=America/New_York 5 16 * * *", "2021-03-14T00:00:00-04:00", "2021-03-14T15:05:00-04:00", time.Hour * 1, ""},
// At 04:05 East Coast (Day light saving off)
{"CRON_TZ=America/New_York 5 4 * * *", "2021-11-25T00:00:00-05:00", "2021-11-25T03:05:00-05:00", time.Hour * 1},
{"CRON_TZ=America/New_York 5 4 * * *", "2021-11-25T00:00:00-05:00", "2021-11-25T03:05:00-05:00", time.Hour * 1, ""},
}

func TestCron(t *testing.T) {
Expand All @@ -71,11 +74,15 @@ func TestCron(t *testing.T) {
assert.NoError(t, err, "Parse end time: %v", tt.now)
}
err = ValidateSchedule(tt.cron)
if tt.result != NoBackoff {
assert.NoError(t, err)
if len(tt.err) > 0 {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.err)
} else {
assert.NoError(t, err, "Failed on cron schedule: %v", tt.cron)

backoff := GetBackoffForNextSchedule(tt.cron, start, end)
assert.Equal(t, tt.result, backoff, "The cron spec is %s and the expected result is %s", tt.cron, tt.result)
}
backoff := GetBackoffForNextSchedule(tt.cron, start, end)
assert.Equal(t, tt.result, backoff, "The cron spec is %s and the expected result is %s", tt.cron, tt.result)
})
}
}

0 comments on commit ba09219

Please sign in to comment.