Skip to content

Commit d852c24

Browse files
committed
support continuous schedule
1 parent be2c0c7 commit d852c24

File tree

3 files changed

+84
-56
lines changed

3 files changed

+84
-56
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ require (
2929
github.com/go-audio/audio v1.0.0
3030
github.com/go-audio/transforms v0.0.0-20180121090939-51830ccc35a5
3131
github.com/go-audio/wav v1.1.0
32-
github.com/go-co-op/gocron/v2 v2.16.2
32+
github.com/go-co-op/gocron/v2 v2.18.0
3333
github.com/go-git/go-git/v5 v5.16.2
3434
github.com/go-gl/mathgl v1.0.0
3535
github.com/go-nlopt/nlopt v0.0.0-20230219125344-443d3362dcb5
@@ -397,7 +397,7 @@ require (
397397
github.com/stbenjam/no-sprintf-host-port v0.1.1 // indirect
398398
github.com/stoewer/go-strcase v1.3.0 // indirect
399399
github.com/stretchr/objx v0.5.2 // indirect
400-
github.com/stretchr/testify v1.10.0 // indirect
400+
github.com/stretchr/testify v1.11.1 // indirect
401401
github.com/subosito/gotenv v1.4.1 // indirect
402402
github.com/tdakkota/asciicheck v0.2.0 // indirect
403403
github.com/tetafro/godot v1.4.17 // indirect

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,8 +464,8 @@ github.com/go-audio/wav v1.1.0 h1:jQgLtbqBzY7G+BM8fXF7AHUk1uHUviWS4X39d5rsL2g=
464464
github.com/go-audio/wav v1.1.0/go.mod h1:mpe9qfwbScEbkd8uybLuIpTgHyrISw/OTuvjUW2iGtE=
465465
github.com/go-chi/chi/v5 v5.2.2 h1:CMwsvRVTbXVytCk1Wd72Zy1LAsAh9GxMmSNWLHCG618=
466466
github.com/go-chi/chi/v5 v5.2.2/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops=
467-
github.com/go-co-op/gocron/v2 v2.16.2 h1:r08P663ikXiulLT9XaabkLypL/W9MoCIbqgQoAutyX4=
468-
github.com/go-co-op/gocron/v2 v2.16.2/go.mod h1:4YTLGCCAH75A5RlQ6q+h+VacO7CgjkgP0EJ+BEOXRSI=
467+
github.com/go-co-op/gocron/v2 v2.18.0 h1:DS3Uhru66q1jy/5f9V0itmi3cLXcn2b7N+duGfgT7gU=
468+
github.com/go-co-op/gocron/v2 v2.18.0/go.mod h1:Zii6he+Zfgy5W9B+JKk/KwejFOW0kZTFvHtwIpR4aBI=
469469
github.com/go-critic/go-critic v0.5.4/go.mod h1:cjB4YGw+n/+X8gREApej7150Uyy1Tg8If6F2XOAUXNE=
470470
github.com/go-critic/go-critic v0.11.4 h1:O7kGOCx0NDIni4czrkRIXTnit0mkyKOCePh3My6OyEU=
471471
github.com/go-critic/go-critic v0.11.4/go.mod h1:2QAdo4iuLik5S9YG0rT4wcZ8QxwHYkrr6/2MWAiv/vc=
@@ -1392,8 +1392,8 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o
13921392
github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
13931393
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
13941394
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
1395-
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
1396-
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
1395+
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
1396+
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
13971397
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
13981398
github.com/subosito/gotenv v1.4.1 h1:jyEFiXpy21Wm81FBN71l9VoMMV8H8jG+qIK3GCpY6Qs=
13991399
github.com/subosito/gotenv v1.4.1/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0=

robot/jobmanager/jobmanager.go

Lines changed: 78 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,14 @@ func (jm *JobManager) createDescriptorSourceAndgRPCMethod(
206206
}
207207

208208
// createJobFunction returns a function that the job scheduler puts on its queue.
209-
func (jm *JobManager) createJobFunction(jc config.JobConfig) func(ctx context.Context) error {
209+
func (jm *JobManager) createJobFunction(jc config.JobConfig, continuous bool) func(ctx context.Context) error {
210210
jobLogger := jm.logger.Sublogger(jc.Name)
211211
// To support logging for quick jobs (~ on the seconds schedule), we disable log
212212
// deduplication for job loggers.
213213
jobLogger.NeverDeduplicate()
214-
return func(ctx context.Context) error {
214+
215+
// using jm.ctx so we interrupt only if JM is shutting down. When changing schedule, let existing jobs complete instead of interrupting.
216+
jobFunc := func(_ context.Context) error {
215217
res, err := jm.getResource(jc.Resource)
216218
if err != nil {
217219
jobLogger.CWarnw(jm.ctx, "Could not get resource", "error", err.Error())
@@ -272,6 +274,7 @@ func (jm *JobManager) createJobFunction(jc config.JobConfig) func(ctx context.Co
272274
jobLogger.CWarnw(jm.ctx, "Job failed", "name", jc.Name, "error", err.Error())
273275
return err
274276
} else if h.Status != nil && h.Status.Err() != nil {
277+
// if job panics, it seems to be captured here.
275278
jobLogger.CWarnw(jm.ctx, "Job failed", "name", jc.Name, "error", h.Status.Err())
276279
return h.Status.Err()
277280
}
@@ -286,6 +289,34 @@ func (jm *JobManager) createJobFunction(jc config.JobConfig) func(ctx context.Co
286289
//jm.logger.Infof("done ctx=%v, jmctx=%v", ctx.Err(), jm.ctx.Err())
287290
return nil
288291
}
292+
293+
return func(ctx context.Context) error {
294+
var err error
295+
for {
296+
select {
297+
case <-ctx.Done():
298+
// Job cancelled (e.g. from schedule modification)
299+
return err
300+
case <-jm.ctx.Done():
301+
// JM shutting down
302+
return err
303+
default:
304+
}
305+
err = jobFunc(ctx)
306+
now := timestamppb.Now()
307+
if jh, ok := jm.JobHistories.Load(jc.Name); ok {
308+
if err != nil {
309+
// this includes captured panics (from InvokeRPC).
310+
jh.AddFailure(now)
311+
} else {
312+
jh.AddSuccess(now)
313+
}
314+
}
315+
if !continuous {
316+
return err
317+
}
318+
}
319+
}
289320
}
290321

291322
// removeJob removes the job from the scheduler and clears the internal map entry.
@@ -309,32 +340,21 @@ func (jm *JobManager) scheduleJob(jc config.JobConfig, verbose bool) {
309340
return
310341
}
311342

312-
var jobType gocron.JobDefinition
313-
var jobLimitMode gocron.LimitMode
314-
t, err := time.ParseDuration(jc.Schedule)
315-
if err != nil {
316-
withSeconds := len(strings.Split(jc.Schedule, " ")) >= 6
317-
jobType = gocron.CronJob(jc.Schedule, withSeconds)
318-
jobLimitMode = gocron.LimitModeReschedule
343+
var continuous bool
344+
var jobDefinition gocron.JobDefinition
345+
var jobOptions []gocron.JobOption
346+
if strings.ToLower(jc.Schedule) == "continuous" {
347+
continuous = true
348+
// used with WithIntervalFromCompletion: if job unexpectedly exits, try to restart later.
349+
// since we capture panics, this is largely unused, but helps reduce scheduler overhead.
350+
jobDefinition = gocron.DurationJob(time.Second * 5)
351+
jobOptions = append(jobOptions,
352+
// don't queue up if running
353+
gocron.WithSingletonMode(gocron.LimitModeReschedule),
354+
gocron.WithIntervalFromCompletion())
319355
} else {
320-
jobType = gocron.DurationJob(t)
321-
jobLimitMode = gocron.LimitModeWait
322-
}
323-
324-
if _, ok := jm.JobHistories.Load(jc.Name); !ok {
325-
jm.JobHistories.Store(jc.Name, &JobHistory{
326-
successTimes: ring.New(historyLength),
327-
failureTimes: ring.New(historyLength),
328-
})
329-
jm.NumJobHistories.Add(1)
330-
}
331-
332-
jobLogger := jm.logger.Sublogger(jc.Name)
333-
334-
jobFunc := jm.createJobFunction(jc)
335-
j, err := jm.scheduler.NewJob(
336-
jobType,
337-
gocron.NewTask(jobFunc),
356+
// Regular gocron-supported modes:
357+
//
338358
// WithSingletonMode option allows us to perform jobs on the same schedule
339359
// sequentially. This will guarantee that there is only one instance of a particular
340360
// job running at the same time. If a job reaches its schedule while the previous
@@ -363,30 +383,38 @@ func (jm *JobManager) scheduleJob(jc config.JobConfig, verbose bool) {
363383

364384
// It is also important to note that DURATION jobs start relative to when they were
365385
// queued on the job scheduler, while CRON jobs are tied to the physical clock.
366-
gocron.WithSingletonMode(jobLimitMode),
386+
t, err := time.ParseDuration(jc.Schedule)
387+
if err != nil {
388+
// TODO: exit if cron job is also invalid. Currently it's stored as an invalid string and validated at NewJob call.
389+
withSeconds := len(strings.Split(jc.Schedule, " ")) >= 6
390+
jobDefinition = gocron.CronJob(jc.Schedule, withSeconds)
391+
jobOptions = append(jobOptions, gocron.WithSingletonMode(gocron.LimitModeReschedule))
392+
393+
} else {
394+
jobDefinition = gocron.DurationJob(t)
395+
jobOptions = append(jobOptions, gocron.WithSingletonMode(gocron.LimitModeWait))
396+
}
397+
}
398+
399+
jobOptions = append(jobOptions,
367400
gocron.WithName(jc.Name),
368-
gocron.WithContext(jm.ctx),
369-
gocron.WithEventListeners(
370-
// May be slightly more accurate to use j.LastRun(), but we don't have direct reference to it here, and we don't want the job to
371-
// complete before we can store the returned Job.
372-
gocron.AfterJobRuns(func(jobID uuid.UUID, jobName string) {
373-
now := timestamppb.Now()
374-
if jh, ok := jm.JobHistories.Load(jobName); ok {
375-
jh.AddSuccess(now)
376-
}
377-
}),
378-
gocron.AfterJobRunsWithError(func(jobID uuid.UUID, jobName string, err error) {
379-
now := timestamppb.Now()
380-
if jh, ok := jm.JobHistories.Load(jobName); ok {
381-
jh.AddFailure(now)
382-
}
383-
}),
384-
gocron.AfterJobRunsWithPanic(func(jobID uuid.UUID, jobName string, recoverData any) {
385-
now := timestamppb.Now()
386-
if jh, ok := jm.JobHistories.Load(jobName); ok {
387-
jh.AddFailure(now)
388-
}
389-
})),
401+
gocron.WithContext(jm.ctx))
402+
403+
jobLogger := jm.logger.Sublogger(jc.Name)
404+
405+
if _, ok := jm.JobHistories.Load(jc.Name); !ok {
406+
jm.JobHistories.Store(jc.Name, &JobHistory{
407+
successTimes: ring.New(historyLength),
408+
failureTimes: ring.New(historyLength),
409+
})
410+
jm.NumJobHistories.Add(1)
411+
}
412+
413+
jobFunc := jm.createJobFunction(jc, continuous)
414+
j, err := jm.scheduler.NewJob(
415+
jobDefinition,
416+
gocron.NewTask(jobFunc),
417+
jobOptions...,
390418
)
391419
if err != nil {
392420
jobLogger.CErrorw(jm.ctx, "Failed to create a new job", "name", jc.Name, "error", err.Error())

0 commit comments

Comments
 (0)