xfra35 / f3-cron

Job scheduling for the PHP Fat-Free Framework
GNU General Public License v3.0
72 stars 22 forks source link

[Hint] `max_execution_time` and Cronjobs #7

Closed exodus4d closed 4 years ago

exodus4d commented 6 years ago

I run into a problem where multiple PHP processes for the same cronJob were active. If you have a job that is executed very frequently (e.g. every minute), the next trigger starts a new PHP process in case the previous job is not finished already :)

This can happen for long running tasks, or a DB connection that could not established and no "timeout" was set.....

I figured out, that PHP CLI scripts are not affected by max_execution_time set in your php.ini config file.

"The default setting is 30. When running PHP from the command line the default setting is 0." source: http://php.net/manual/en/info.configuration.php#ini.max-execution-time

So even if a max_execution_time is set, it is ignored. In case there is no timeout set for e.g. a DB connection, the PHP script runs forever... 41500026-b7aec26e-718a-11e8-93bb-ebbeb907389c

So adding something like this, can help:

ini_set('max_execution_time', 300)
ikkez commented 5 years ago

Good addition for the docs, but I think it should not go into the plugin itself (or be configurable) as the use cases might differ here and there.. i.e. I have cases where the cron job run for 20min or longer.

@xfra35 would you mind tagging a new release with the latest commit? ..I'm currently using "xfra35/f3-cron": "dev-master#eba0a5574a8c35623307dd27135a67d76e4da12e as 1.2", in composer, which works but i dont like it 😅

xfra35 commented 5 years ago

Hi guys, sorry for the delay.

@exodus4d thanks for the report. I'm wondering how the plugin should handle this.

I'm thinking about 4 solutions:

  1. mention that issue in the docs and let developpers handle it
  2. set automatically max_execution_time inside the execute() method (although guessing the right value out of a crontab schedule seems quite tricky)
  3. add a new optional parameter to the set() method, allowing to define a max execution time for each job. E.g: $cron->set('Job1', 'App->job1', '* * * * *', 59)
  4. add a new optional parameter to the set() method, allowing to prevent parallel calls. E.g: $cron->set('Job1', 'App->job1', '* * * * *', FALSE) (which would internally trigger a call to the framework mutex() method)

What's your opinion about it?

@ikkez v1.2.1 has been released

xfra35 commented 5 years ago

Actually, I had already updated the docs long time ago, but forgot to push 😅

See https://github.com/xfra35/f3-cron#overlapping-jobs