Repository: go-co-op/gocron
Pull request #159 (2021-04-13 14:56:53)

handle occasional occurence of double run on daily job
### What does this do?

we see an occasional instance of a daily job running twice within a second. this seems to be due to the job running too fast and starting a fraction of a second early such that it runs during the 1 second window before it should. this fix handles that case for daily jobs as those only run once at a given even hour/min/sec time.

### Which issue(s) does this PR fix/relate to?
<!--- Put `Resolves #XXX` here to auto-close the issue that your PR fixes (if such) --->

https://github.com/go-co-op/gocron/issues/52#issuecomment-816502678

### List any changes that modify/break current functionality


### Have you included tests for your changes?


### Did you document any new/modified functionality?

- [ ] Updated `example_test.go`
- [ ] Updated `README.md`

### Notes



Patch:
diff --git a/scheduler.go b/scheduler.go
index d345b49..b436a13 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -214,31 +214,31 @@ func (s *Scheduler) calculateMonths(job *Job, lastRun time.Time) time.Duration {
 		daysDifference := int(math.Abs(lastRun.Sub(jobDay).Hours()) / 24)
 		nextRun := s.roundToMidnight(lastRun).Add(job.getAtTime())
 		if jobDay.Before(lastRun) { // shouldn't run this month; schedule for next interval minus day difference
-			nextRun = nextRun.AddDate(0, int(job.interval), -daysDifference)
+			nextRun = nextRun.AddDate(0, job.interval, -daysDifference)
 		} else {
 			if job.interval == 1 { // every month counts current month
-				nextRun = nextRun.AddDate(0, int(job.interval)-1, daysDifference)
+				nextRun = nextRun.AddDate(0, job.interval-1, daysDifference)
 			} else { // should run next month interval
-				nextRun = nextRun.AddDate(0, int(job.interval), daysDifference)
+				nextRun = nextRun.AddDate(0, job.interval, daysDifference)
 			}
 		}
-		return s.until(lastRun, nextRun)
+		return until(lastRun, nextRun)
 	}
-	nextRun := lastRunRoundedMidnight.Add(job.getAtTime()).AddDate(0, int(job.interval), 0)
-	return s.until(lastRunRoundedMidnight, nextRun)
+	nextRun := lastRunRoundedMidnight.Add(job.getAtTime()).AddDate(0, job.interval, 0)
+	return until(lastRunRoundedMidnight, nextRun)
 }
 
 func (s *Scheduler) calculateWeekday(job *Job, lastRun time.Time) time.Duration {
 	daysToWeekday := remainingDaysToWeekday(lastRun.Weekday(), *job.scheduledWeekday)
 	totalDaysDifference := s.calculateTotalDaysDifference(lastRun, daysToWeekday, job)
 	nextRun := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, totalDaysDifference)
-	return s.until(lastRun, nextRun)
+	return until(lastRun, nextRun)
 }
 
 func (s *Scheduler) calculateWeeks(job *Job, lastRun time.Time) time.Duration {
 	totalDaysDifference := int(job.interval) * 7
 	nextRun := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, totalDaysDifference)
-	return s.until(lastRun, nextRun)
+	return until(lastRun, nextRun)
 }
 
 func (s *Scheduler) calculateTotalDaysDifference(lastRun time.Time, daysToWeekday int, job *Job) int {
@@ -258,18 +258,26 @@ func (s *Scheduler) calculateTotalDaysDifference(lastRun time.Time, daysToWeekda
 }
 
 func (s *Scheduler) calculateDays(job *Job, lastRun time.Time) time.Duration {
+
 	if job.interval == 1 {
 		lastRunDayPlusJobAtTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.Location()).Add(job.getAtTime())
+
+		// handle occasional occurrence of job running to quickly / too early such that last run was within a second of now
+		lastRunUnix, nowUnix := job.LastRun().Unix(), s.now().Unix()
+		if lastRunUnix == nowUnix || lastRunUnix == nowUnix-1 || lastRunUnix == nowUnix+1 {
+			lastRun = lastRunDayPlusJobAtTime
+		}
+
 		if shouldRunToday(lastRun, lastRunDayPlusJobAtTime) {
-			return s.until(lastRun, s.roundToMidnight(lastRun).Add(job.getAtTime()))
+			return until(lastRun, s.roundToMidnight(lastRun).Add(job.getAtTime()))
 		}
 	}
 
-	nextRunAtTime := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, int(job.interval)).In(s.Location())
-	return s.until(lastRun, nextRunAtTime)
+	nextRunAtTime := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, job.interval).In(s.Location())
+	return until(lastRun, nextRunAtTime)
 }
 
-func (s *Scheduler) until(from time.Time, until time.Time) time.Duration {
+func until(from time.Time, until time.Time) time.Duration {
 	return until.Sub(from)
 }
 
@@ -477,7 +485,7 @@ func (s *Scheduler) RemoveByTag(tag string) error {
 }
 
 func (s *Scheduler) findJobsByTag(tag string) ([]*Job, error) {
-	jobs := []*Job{}
+	var jobs []*Job
 	for _, job := range s.Jobs() {
 		if strings.Contains(strings.Join(job.Tags(), " "), tag) {
 			jobs = append(jobs, job)
@@ -658,12 +666,12 @@ func (s *Scheduler) setUnit(unit schedulingUnit) {
 	job.setUnit(unit)
 }
 
-// Second sets the unit with seconds
+// Millisecond sets the unit with seconds
 func (s *Scheduler) Millisecond() *Scheduler {
 	return s.Milliseconds()
 }
 
-// Seconds sets the unit with seconds
+// Milliseconds sets the unit with seconds
 func (s *Scheduler) Milliseconds() *Scheduler {
 	s.setUnit(milliseconds)
 	return s


Test patch:
diff --git a/scheduler_test.go b/scheduler_test.go
index 11d4a79..24d4b15 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -228,6 +228,8 @@ func TestAt(t *testing.T) {
 
 		select {
 		case <-time.After(1 * time.Second):
+			log.Println(now.Add(time.Minute))
+			log.Println(dayJob.nextRun)
 			assert.Equal(t, now.Add(1*time.Minute), dayJob.nextRun)
 		case <-semaphore:
 			t.Fatal("job ran even though scheduled in future")
@@ -618,6 +620,10 @@ func TestScheduler_StartAt(t *testing.T) {
 }
 
 func TestScheduler_CalculateNextRun(t *testing.T) {
+	ft := fakeTime{onNow: func(l *time.Location) time.Time {
+		return time.Date(1970, 1, 1, 12, 0, 0, 0, l)
+	}}
+
 	day := time.Hour * 24
 	januaryFirst2020At := func(hour, minute, second int) time.Time {
 		return time.Date(2020, time.January, 1, hour, minute, second, 0, time.UTC)
@@ -649,6 +655,7 @@ func TestScheduler_CalculateNextRun(t *testing.T) {
 		{name: "daily job just ran at 5:30AM and should be scheduled for today at 8:30AM", job: &Job{interval: 1, unit: days, atTime: 8*time.Hour + 30*time.Minute, lastRun: januaryFirst2020At(5, 30, 0)}, wantTimeUntilNextRun: 3 * time.Hour},
 		{name: "job runs every 2 days, just ran at 5:30AM and should be scheduled for 2 days at 8:30AM", job: &Job{interval: 2, unit: days, atTime: 8*time.Hour + 30*time.Minute, lastRun: januaryFirst2020At(5, 30, 0)}, wantTimeUntilNextRun: (2 * day) + 3*time.Hour},
 		{name: "job runs every 2 days, just ran at 8:30AM and should be scheduled for 2 days at 8:30AM", job: &Job{interval: 2, unit: days, atTime: 8*time.Hour + 30*time.Minute, lastRun: januaryFirst2020At(8, 30, 0)}, wantTimeUntilNextRun: 2 * day},
+		{name: "daily, last run was 1 second ago", job: &Job{interval: 1, unit: days, atTime: 12 * time.Hour, lastRun: ft.Now(time.UTC).Add(-time.Second)}, wantTimeUntilNextRun: 1 * day},
 		//// WEEKS
 		{name: "every week should run in 7 days", job: &Job{interval: 1, unit: weeks, lastRun: januaryFirst2020At(0, 0, 0)}, wantTimeUntilNextRun: 7 * day},
 		{name: "every week with .At time rule should run respect .At time rule", job: &Job{interval: 1, atTime: _getHours(9) + _getMinutes(30), unit: weeks, lastRun: januaryFirst2020At(9, 30, 0)}, wantTimeUntilNextRun: 7 * day},
@@ -679,6 +686,7 @@ func TestScheduler_CalculateNextRun(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			s := NewScheduler(time.UTC)
+			s.time = ft
 			got := s.durationToNextRun(tc.job.LastRun(), tc.job)
 			assert.Equalf(t, tc.wantTimeUntilNextRun, got, fmt.Sprintf("expected %s / got %s", tc.wantTimeUntilNextRun.String(), got.String()))
 		})