gocron icon indicating copy to clipboard operation
gocron copied to clipboard

[BUG] - Scheduler with LimitModeWait and Job Singleton mode with LimitModeReschedule - Job Queues up if Scheduler limit reached

Open 0x01F4 opened this issue 1 year ago • 5 comments

Describe the bug

Scheduler with LimitModeWait and Job Singleton mode with LimitModeReschedule - Job Queues up if Scheduler limit reached

To Reproduce


import (
	"github.com/go-co-op/gocron/v2"
	"log"
	"time"
	)
func main() {

	scheduler, err := gocron.NewScheduler(
		gocron.WithLimitConcurrentJobs(1, gocron.LimitModeWait),
	)
	if err != nil {
		log.Println(err)
		return
	}
	_, err = scheduler.NewJob(
		gocron.DurationJob(time.Second),
		gocron.NewTask(
			func() {
				time.Sleep(10 * time.Second)
				log.Println("RAN")
			},
		),
		gocron.WithSingletonMode(gocron.LimitModeReschedule),
	)

	if err != nil {
		log.Println(err)
		return
	}
	scheduler.Start()

	select {
	default:
		for {
			time.Sleep(1 * time.Second)
			log.Printf("In Queue: %d\n", scheduler.JobsWaitingInQueue())
		}
	}
}

Output:

2024/05/01 01:18:33 In Queue: 0
2024/05/01 01:18:34 In Queue: 1
2024/05/01 01:18:35 In Queue: 2
2024/05/01 01:18:36 In Queue: 3
2024/05/01 01:18:37 In Queue: 4
2024/05/01 01:18:38 In Queue: 5
2024/05/01 01:18:39 In Queue: 6
2024/05/01 01:18:40 In Queue: 7
2024/05/01 01:18:41 In Queue: 8
2024/05/01 01:18:42 In Queue: 9
2024/05/01 01:18:43 RAN
2024/05/01 01:18:43 In Queue: 9
2024/05/01 01:18:44 In Queue: 10
2024/05/01 01:18:45 In Queue: 11

Version

Latest

Expected behaviour

If Scheduler with LimitModeWait limit is reached and Job is singleton with LimitModeReschedule then it should reschedule immediately

Additional context

If one job is slow it will hog the queue

0x01F4 avatar Apr 30 '24 19:04 0x01F4

I hit this as well. Think it is the same as - https://github.com/go-co-op/gocron/issues/706

What is happening is that https://github.com/go-co-op/gocron/blob/v2/executor.go#L306 sends out for reschedule but https://github.com/go-co-op/gocron/blob/v2/executor.go#L171 also does.

The job doesn't have to be slow, you can return inside the duration and jobs still accumulate

bearrito avatar May 01 '24 04:05 bearrito

@bearrito I think it is two different issue.If you use WithLimitConcurrentJobs with scheduler then job is not run by singletonModeRunner, but issue maybe related

0x01F4 avatar May 01 '24 17:05 0x01F4

I think your right, I missed that. In this case

https://github.com/go-co-op/gocron/blob/v2/executor.go#L99

and https://github.com/go-co-op/gocron/blob/v2/executor.go#L135

are both rescheduling

bearrito avatar May 01 '24 21:05 bearrito

I've got #723 up to solve the memory issue with the duplicate rescheduling that was going on.

As to this particular issue, what's happening makes sense with the current design:

  • limit mode is queueing up the jobs because it's set to wait, and the check for whether the job itself is singleton with reschedule isn't being reached until the limit mode runner is free to process the next job in the queue. At this point, it will never pass check that the job itself is singleton and reschedule it because the limit mode limit takes precedence and only 1 job may ever be running

I had originally designed it not thinking that the two modes would be used concurrently, but I see now that there are valid reasons to do so. As such, this will need some refactoring.

Essentially, the logic in https://github.com/go-co-op/gocron/blob/v2.3.0/executor.go#L241-L267 will need to be moved up to https://github.com/go-co-op/gocron/blob/v2.3.0/executor.go#L129-L135 and that will need to be tested / investigated for any unintended consequences.

JohnRoesler avatar May 01 '24 22:05 JohnRoesler

This issue happening is rare.Since I don't see a use case were both will be used concurrently(maybe it may change later or others might have this use case). Also this can happen only when one job is stuck for long time.I found it only during test. So maybe just documenting this behaviour is enough for now.

0x01F4 avatar May 02 '24 16:05 0x01F4

adding a comment in d808cd9

JohnRoesler avatar May 06 '24 15:05 JohnRoesler