Bosma / Scheduler

Modern C++ Scheduling Library
MIT License
272 stars 74 forks source link

cron runs at random time #7

Closed nishantd01 closed 6 years ago

nishantd01 commented 6 years ago

s.cron(15 8 *)[]() , this shoud run daily at 8:15am in the morning however it triggers the cron at random time including 8:15 am also

Bosma commented 6 years ago

I'm not sure how this could happen. Could you provide some way to reproduce this?

Let me explain how the cron functionality works to help us fix this (this is mostly to ensure my own understanding, and document the correctness of the code):

Calling s.cron("15 8 * * *") will create a CronTask that stores your function in f and a Cron object cron. cron is initialized with your expression "15 8 * * *".

cron then calculates the next time f will run and adds the task to the task pool. The task pool is a multimap keyed by a time_point, so it is always sorted by time (ascending). Then it will interrupt the watcher thread.

Meanwhile, the watcher thread will sleep until the next task time. Let's assume that it is our cron job. Once the time has passed manage_tasks() is run. manage_tasks() only grabs the tasks in the task pool that have a time_point that come before the current time (multimap::upper_bound() returns an iterator to the first element greater than the given key). A tasks f is only run (after being added to the thread pool) inside the end_of_tasks_to_run != tasks.begin() check. Meaning the current time is greater than the tasks time. So, your cron task will be added to the thread pool only when the current time is greater than your tasks time.

I can imagine some situations causing a task to be fired improperly, but I think those cases are covered by the code (unless I'm mistaken):

  1. If the sleep was halted prematurely (by another task being added, or a spurious wakeup [if possible?]). manage_tasks() won't fire off our task, because that time hasn't happened yet.
  2. If somehow the task is in the task pool, and the sleep was halted (significantly) after the task time. This would cause manage_tasks() to fire off our task much later then expected. However, I don't think this is possible, as the tasks map is sorted by time in ascending order. So the sleep_until will always wait for the task that happens the soonest. Also, this implies the task wasn't fired off at 8:15 am (which you say it was).
  3. A race condition (that could cause 1 or 2). add_task() and manage_tasks() both read/modify the tasks, except they both lock lock, so there's no issue there. There is a race condition with interspersed add_task() and the watcher thread, but the interrupt call fixes that.
  4. The task is added twice, or with the wrong time. The time is always calculated with cron_to_next(), which as far as I know works correctly (see below). Tasks are added only in manage_tasks() and add_task(). manage_tasks() only adds a recurring task back to tasks after it was erased. An interval task (which isn't the case here) is only added back once (it is not added to recurring tasks). add_task() is only called once by s.cron("15 8 * * *)

Here's a quick little test program to demonstrate the accurate workings of the cron_to_next() function:

#include` <iostream>

#include "Scheduler.h"

void print_tp(const Bosma::Clock::time_point &tp) {
  std::time_t t =  std::chrono::system_clock::to_time_t(tp);
  std::cout << std::ctime(&t) << std::endl;
}

int main() {
  std::string TEST_EXP = "15 8 * * *";

  // manually verify the printed times are accurate
  Bosma::Cron test(TEST_EXP);
  auto next = test.cron_to_next();
  print_tp(next);
  for (int i = 0; i < 2*365; i++) {
    next = test.cron_to_next(next);
    print_tp(next);
  }

  // create the task manually (as Scheduler does when you run Scheduler::cron)
  Bosma::CronTask task(TEST_EXP, []() { std::cout << "8:15am" << std::endl; });
  print_tp(task.get_new_time()); // is expected
}

Let me know what you find.

nishantd01 commented 6 years ago

I apologize for late reply, using same scheduler still and works very well,the issue was with the containerized application , and the timezone was different than the local linux system, so i had to sync the timezone of container with host machine. that was causing the issue .

Bosma commented 6 years ago

No problem, good to know. It was useful to document the proper workings at any rate.