[BUG] - CPU usage 100% in WaitMode

[aibotsoft]

With this test code:

package main

import (
	"github.com/go-co-op/gocron"
	"time"
)

func job() {
	time.Sleep(time.Second * 3)
}

func main() {
	s := gocron.NewScheduler(time.UTC)
	s.SetMaxConcurrentJobs(1, gocron.WaitMode)
	s.Every(time.Second * 5).Do(job)
	s.Every(time.Second * 5).Do(job)
	s.StartBlocking()
}

I get 100% CPU utilization (at least one core) which is equivalent to an infinite loop:

package main

func main() {
	for {
		
	}
}

Version
v1.13.0

Expected behavior
While waiting for a free executor, the processor should not be loaded at 100%

[AlexanderSutul]

@JohnRoesler Is that a good approach to add a delay in 1 * nanosecond? It's just a fastest solution I can suggest for now 😊

[AlexanderSutul]

@aibotsoft Hey! Are you sure it's a problem? I just checked that 100% here doesn't effect any I/O operations. So, yeah it's 100% but it's like fake 100% of usage. So, it's just flooded OS scheduler.
Correct me, if I'm wrong, please. (links for articles about that are appreciated)

[aibotsoft]

Maybe you are right, but I am sure that the system should not be overloaded with such a simple task as cron, not with the jobs themselves, but with the cron scheduler.

I think there is an inaccuracy here, in the executor.go file:

if !e.maxRunningJobs.TryAcquire(1) {
  switch e.limitMode {
  case RescheduleMode:
	  return
  case WaitMode:
	  for {
		  select {
		  case <-stopCtx.Done():
			  return
		  case <-f.ctx.Done():
			  return
		  default:
		  }
  
		  if e.maxRunningJobs.TryAcquire(1) {
			  break
		  }
	  }
  }
}

Scheduler trying to get a free executor, if there is no free one, in RescheduleMode we just leave, but in WaitMode we enter an endless loop that constantly tries to get the executor using the TryAcquire method, which simply returns bool without blocking.

Perhaps instead of the TryAcquire method, we should use the Acquire method, which acquires the semaphore blocking until resources are available or ctx is done. In this case, you don't need an infinite loop.

[AlexanderSutul]

@aibotsoft Thanks for your thoughts. Let's see what I can do. I'll try to fix it till the end of next week.

[JohnRoesler]

@aibotsoft this is interesting! The reason it is done the way currently is to allow checking the state of both the scheduler and individual job context states. The Acquire method allows waiting for a single context to be done, but I notice even in that case, it can still run the job even after the context done occurs:

// Acquired the semaphore after we were canceled. Rather than trying to
// fix up the queue, just pretend we didn't notice the cancelation.

Now maybe that's a cost worth paying to lower CPU. I haven't tried it yet with this method, but it looks to be doing something similar to the executor logic under the hood.