Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
0x01F4 opened this issue Apr 30, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@0x01F4
Copy link
Contributor

0x01F4 commented Apr 30, 2024

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 0x01F4 added the bug Something isn't working label Apr 30, 2024
@0x01F4 0x01F4 changed the title [BUG] - Scheduler with LimitModeWait andJob Singleton mode with LimitModeReschedule - Job Queues up if Scheduler limit reached [BUG] - Scheduler with LimitModeWait and Job Singleton mode with LimitModeReschedule - Job Queues up if Scheduler limit reached Apr 30, 2024
@bearrito
Copy link

bearrito commented May 1, 2024

I hit this as well. Think it is the same as - #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

@0x01F4
Copy link
Contributor Author

0x01F4 commented May 1, 2024

@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

@bearrito
Copy link

bearrito commented May 1, 2024

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

@JohnRoesler
Copy link
Contributor

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.

@0x01F4
Copy link
Contributor Author

0x01F4 commented May 2, 2024

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.

@JohnRoesler
Copy link
Contributor

adding a comment in d808cd9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants