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

preventOverrun maybe not working for CronJob #195

Closed
boristian opened this issue Jul 14, 2023 · 7 comments · May be fixed by #202
Closed

preventOverrun maybe not working for CronJob #195

boristian opened this issue Jul 14, 2023 · 7 comments · May be fixed by #202

Comments

@boristian
Copy link

Hi,
first of alle thanks for the great work!
Here is an example where a CronJob an a SimpleIntervalJob are created both with an Interval of 10 seconds and the same task which takes > 17 seconds to execute and both with the option preventOverrun.
While the SimpleIntervalJob works and is only started every second time (20 seconds) the Cronjob is called every 10 seconds.
It seems like the option is not working for CronJobs or am i'm doing something wrong?

const {
  AsyncTask,
  CronJob,
  SimpleIntervalJob,
  ToadScheduler
} = require('toad-scheduler')

const delay = (timeout = 2000) => new Promise(resolve => {
  setTimeout(() => resolve(), timeout)
})

const cronTaskFn = () => {
  const now = new Date().toISOString()
  // const now = Date.now()

  console.error(new Date().toISOString(), 'CronTask Start', now)

  return delay(17000)
    .then(() => console.error(new Date().toISOString(), 'CronTask End', now))
}

const simpleTaskFn = () => {
  const now = new Date().toISOString()
  // const now = Date.now()

  console.error(new Date().toISOString(), ': SimpleTask Start', now)

  return delay(17000)
    .then(() => console.error(new Date().toISOString(), 'SimpleTask End', now))
}

const run = () => {
  try {
    const scheduler = new ToadScheduler()

    // Scheduler-Setup
    const cronSchedule = { cronExpression: '*/10 * * * * *' }
    const cronTask = new AsyncTask('cron-task', () => cronTaskFn())
    const cronOpts = { id: 'CRON_TASK_ID', preventOverrun: true }

    console.debug('Scheduler schedule and opts', cronSchedule, cronOpts)

    const cronJob = new CronJob(cronSchedule, cronTask, cronOpts)

    scheduler.addCronJob(cronJob)

    const simpleTask = new AsyncTask('simple-task', () => simpleTaskFn())
    const simpleOpts = { id: 'SIMPLE_TASK_ID', preventOverrun: true }

    console.debug('Scheduler opts', simpleOpts)

    const simpleJob = new SimpleIntervalJob({ seconds: 10 }, simpleTask, simpleOpts)

    scheduler.addSimpleIntervalJob(simpleJob)
  } catch (err) {
    console.error(err)

    process.exit(1)
  }

  process.on('SIGINT', () => process.exit(0))
}

run()

I am using version 3.0.0.

Thanks in advance

@kibertoad
Copy link
Owner

@Hexagon

@Hexagon
Copy link

Hexagon commented Jul 14, 2023

👋

I can not find any code in toad-scheduler that translate option preventOverrun to croner option protect (https://github.com/Hexagon/croner#options). Could that be the cause?

@kibertoad
Copy link
Owner

@Hexagon is it turned off by default?

@Hexagon
Copy link

Hexagon commented Jul 14, 2023

@kibertoad Yes it is, i see now that default is "undefined" according to the docs, will change that to false to clarify

@kibertoad
Copy link
Owner

thanks, I'll fix then!

@ahamid
Copy link

ahamid commented Aug 11, 2023

it looks like it's explicitly turned off regardless of the option supplied:

this.preventOverrun = false

constructor(schedule: CronSchedule, task: Task | AsyncTask, options: JobOptions = {}) {
  super(options.id)
  this.preventOverrun = false // xxx
  this.schedule = schedule
  this.task = task
}

@kibertoad
Copy link
Owner

Fixed in 3.0.1, sorry for taking so long, writing a working test for this was much trickier than expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants