Scheduled Jobs - Define and track "last_run_end"

[mattwire]

Overview

Currently if a job throws an exception it's caught and logged in JobLog. But if it fails for another reason (eg. 500 error, Out of Memory) it silently crashes and stops all jobs from running because it tries to run again and crashes every time you run cron.

Before

No easy was to identify that a job started but did not finish. You can analyse the civicrm_job_log table but as you won't find a "job completed" log entry it requires some interesting queries to identify this situation (because you have to try to match the lack of a record with an existing "job start" record of which there will be many...

After

If a job completes (whether successful or by exception) it will record a last_run_end date in the job table.

This could be used to improve system status messages but also the queuing and prioritising of scheduled jobs. For example a simple sort on last_run_date is not empty would prevent a single bad job from stopping execution of all other jobs.

Technical Details

Add a new field to the table. Add job to scheduled job runner to clear and populate the field.

Comments

We don't implement any logic in this PR to use the field for analysis or to prioritise the job queue. This could be done in a future PR.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[totten]

This does seem like a pretty good idea, and the patch includes some good/non-obvious bits.

(I haven't run tho.)

[totten]

@mattwire Error:

Add last_run_end column to Job table (CRM_Upgrade_Incremental_php_FiveSeventyTwo::addColumn(civicrm_job,last_run_end,last_run_end timestamp NULL DEFAULT NULL COMMENT "When did this cron entry last finish running"))
Error executing task "Add last_run_end column to Job table"

In Error.php line 971:
                                          
  [Civi\Core\Exception\DBQueryException]  
  DB Error: syntax error                  
                                          

[totten]

For the rest of the patch, I did some r-run and provoked various hacks (e.g. using sleep(5) to slow down job; interjecting exceptions and PHP fatals). It seemed to behave as expected.

Should allow a brief pause in case others have comments. Flagging merge ready.

[highfalutin]

Reading through the code, this looks good and I agree it will be useful.

[seamuslee001]

I think people have had some time now to get their heads around this merging