From ba09219d41401a85bf25564109d254efc0425a29 Mon Sep 17 00:00:00 2001 From: Yimin Chen Date: Fri, 21 Apr 2023 18:22:37 -0700 Subject: [PATCH] Fix cron for schedules that do not match any time (#4206) * Fix cron for schedules that do not match any time * Use NoBackoff --- common/backoff/cron.go | 16 +++++++++--- common/backoff/cron_test.go | 51 +++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/common/backoff/cron.go b/common/backoff/cron.go index 11d2ee52e90..c02f0b544f5 100644 --- a/common/backoff/cron.go +++ b/common/backoff/cron.go @@ -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 } @@ -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())) diff --git a/common/backoff/cron_test.go b/common/backoff/cron_test.go index d4ab5c67fb0..c8a7291882c 100644 --- a/common/backoff/cron_test.go +++ b/common/backoff/cron_test.go @@ -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) { @@ -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) }) } }