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

[RFC] Poor Man's Cron #22310

Closed
wants to merge 26 commits into from
Closed

[RFC] Poor Man's Cron #22310

wants to merge 26 commits into from

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Sep 21, 2018

porting of https://github.com/joomla-projects/privacy-framework/pull/133
Pull Request for Issue #20839

Summary of Changes

a basic job scheduler system to be able to schedule and execute tasks that could be run as:

  • a cron job on CLI/terminal
  • a webcron from webcron services
  • a normal onAfterRespond event

plugins.

  • a new system plugin scheduler

it runs at scheduled time
it trigger the onExecuteScheduledTask event from the new job plugin group
can be triggered from a webcron service

  • a new job plugin logrotation

when triggered (even from cli/cron) it runs at scheduled time

the CLI script

  • a new cli scheduler script

it trigger the onExecuteScheduledTask event from the new job plugin group

Testing Instructions

apply PR and discover
install the system scheduler plugin
install the job logrotation plugin
disable the system log rotation plugin
enable the debug

additionally you can install these 2 sample job plugin
plg_jobone.zip
plg_jobtwo.zip

wich do quite nothing, just sleep for some seconds, useful to help testing concurrency

Test settings

the scheduler system plugin

screenshot from 2018-09-29 10-51-03

the job plugin

screenshot from 2018-09-29 11-00-58

i can suggest to test with the ApacheBench tool

if you want to test the webcron way
ab -n 1000 -c 50 http://yoursite/?webcronkey=mysecretkey
or without a webcron
ab -n 1000 -c 50 http://yoursite/

for those lucky enough to use a cron job simply schedule/run from CLI
php cli/scheduler.php

the execution can be checked in the log path
and should look like something like this , depending on your plugin settings
screenshot from 2018-09-29 11-05-31

Expected result

even without a cron facilities you are able to schedule job's

Actual result

n.a

Additional comments

it can be easly used for tasks like:

  • deleteExpiredConsents
  • remindExpiringConsents
  • logRotation
  • updateNotifications
  • sessionGC

etc...

todo

if approved
convert the old plugins that use directly onAfterRespond event
the sql install/update script

// Configure error reporting to maximum for CLI output.
error_reporting(E_ALL);
ini_set('display_errors', 1);
use Joomla\Registry\Registry;
Copy link
Contributor

@Quy Quy Sep 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add blank line above the use statement.

@alikon
Copy link
Contributor Author

alikon commented Sep 21, 2018

@Quy i will be happy to fix all CS related issues,
just need some kind of green light before 🙄

@roland-d
Copy link
Contributor

@alikon Personally I am not so in favor for the poor man's cron but I am not against it though. Let me explain. First of all, if a host doesn't offer cron job facilities, it's time to look for another host. Second, it doesn't offer the same options as a real cron job. You cannot run the Joomla CLI files as we check if you are on the CLI. You cannot be sure the job is executed at the set time, you are depending on the visitor. Third, the user who happens to visit the site and trigger the job has to wait until the job is finished. Fourth, you are limited to the max_execution_time of your host. If your host doesn't offer real cron jobs, this is most likely not exceeding the 30 second mark.

There is also a service for this like https://www.easycron.com/ which can trigger cronjobs if your host doesn't support it.

Users usually don't understand these limitations and if this goes in we should be very clear about this. As an extension developer that relies a lot on cronjobs I have seen the problems that these poor man crons give. In my case it is users importing huge amounts of data and wondering why it crashes or takes so long. I can see the questions coming that the cron doesn't work.

Just my 3 cents :)

@brianteeman
Copy link
Contributor

@roland-d I agree with all your points. No question about it - a real cron will always be better. However this is much more beginner friendly and for those people who regularly move hosts it has the advantage of being portable. As long as there is a notice pointing out that a real cron is better etc then I don't see an issue with this.

@brianteeman
Copy link
Contributor

@roland-d should be noticed that drupal has a "poor mans cron" see https://www.drupal.org/docs/8/cron-automated-tasks/cron-automated-tasks-overview

@mbabker
Copy link
Contributor

mbabker commented Sep 22, 2018

There is also a similar capability in WordPress - https://developer.wordpress.org/plugins/cron/

@mbabker
Copy link
Contributor

mbabker commented Sep 22, 2018

As an extension developer that relies a lot on cronjobs I have seen the problems that these poor man crons give.

Similar to the session GC capabilities added in 3.8, when complete this should be able to run in a way where the actual task runners can execute in either a web context or through a CLI script (then someone who can and does set up a proper cron at the server level can turn off the web based executor, probably a plugin, and rely on the server's cron to run things).

@alikon
Copy link
Contributor Author

alikon commented Sep 25, 2018

sorry folks for my late reply.... even if i'm lazy.... i'm still working on this task (with my times, sorry ) ... for me this it is a very important task
@roland-d all your concerns are mine's too... and the same i think for the other's who cares about this matter

for sure we cannot equiparate a cron with a web-cron we all agree on this...

and this must be crystal clear when and if this [wip] pr will go in to the core

...my simulations with tools like ApacheBench are smoothly going quite well, my first goal is to manage concurrency for the web side... for the cron side if we a have a normal sysadmin ... maybe we should'nt need to manage that issue....

i'll come back with some still "new and dirty code" quite soon...

@alikon alikon requested a review from brianteeman as a code owner September 29, 2018 05:47
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Sep 29, 2018
@alikon alikon changed the title [WIP][Feature] Poor Man's Cron [RFC][Feature] Poor Man's Cron Sep 29, 2018
@joomla-cms-bot joomla-cms-bot added the RFC Request for Comment label Sep 29, 2018
@alikon
Copy link
Contributor Author

alikon commented Sep 29, 2018

updated the pr description&test info accordingly to the latest changes

try
{
// Lock the tables to prevent multiple plugin executions causing a race condition
$db->lockTable('#__extensions');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You execute one UPDATE query, which is atomic. IMO you do not need to call $db->lockTable(). Checking the result of $db->getAffectedRows() should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, maybe i was too scared about concurrency issues.... 🤕
we can changed it for sure, if the whole thing gain some kind of green light

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's go with the "atomic"

PLG_JOB_LOGROTATION_CACHETIMEOUT_LABEL="Frequency (in unit)"
PLG_JOB_LOGROTATION_CACHETIMEOUT_DESC="How often should the job run."
PLG_JOB_LOGROTATION_UNIT_LABEL="Unit of time"
PLG_JOB_LOGROTATION_UNIT_DESC="How often should the job run."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isnt a correct description for the selection of the unit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@tecpromotion
Copy link
Contributor

@roland-d should be noticed that drupal has a "poor mans cron" see https://www.drupal.org/docs/8/cron-automated-tasks/cron-automated-tasks-overview

Shopware has one, TYPO3 of course.
In both examples, it is more about setting up the cronjobs, ie the tasks and how often they should run.
It must then continue to run a real cron job on the host, which calls the scheduler. For example. every 5 minutes, the scheduler is called up and then looks if one of the jobs has to be processed and leaves it running. In Shopware, the answer will be displayed partly in the settings of the respective task with values, or a true false. It can even be set to be notified by false via email or even the task is set to inactive.

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@ghost ghost changed the title [RFC][Feature] Poor Man's Cron [RFC] Poor Man's Cron Apr 19, 2019
@ghost
Copy link

ghost commented Apr 26, 2019

As new Feature this should be rebased on J4.

@HLeithner
Copy link
Member

@alikon I would be very happy to see this feature in J4. But wouldn't be merged in J3.

I close this here, please reference if you have a PR for J4.

@brianteeman
Copy link
Contributor

@alikon it would be really great if you were able to port this to J4

@alikon alikon mentioned this pull request May 27, 2019
@alikon alikon deleted the patch-111 branch July 27, 2019 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators RFC Request for Comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants