Skip to content

magento/magento2#23054 #24789

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

Closed

Conversation

mamsincl
Copy link
Contributor

@mamsincl mamsincl commented Sep 30, 2019

Description (*)

Extending Magento/Cron module with functionality of being able to close the orphan/stale running cron tasks (schedules) which been started before, but somehow stopped running

Fixed Issues (if relevant)

  1. Fixes Cron job not running after crashed once #23054 : Cron job not running after crashed once

Manual testing scenarios (*)

preconditions:

  1. ps command availability on server
  2. php bin/magento setup:upgrade to apply modifications in database
  3. run php bin/magento cron:run multiple time (or be sure that crontab does the same) to get the necessary data for process schedules

(scenario a)

  1. change "success" to "running" on couple of job on cron_schedule table
  2. run php bin/magento cron:run
  3. check the modified rows

(scenario b)

  1. run php bin/magento cron:run on CLI
  2. shut down/restart the server from a different console before getting back the prompt
    https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-hostname.html

Questions or comments

If you using multiple instances or in cloud environment, be sure that you have set up the hostname(s) of your instance(s) (eg. https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-hostname.html) in prior to avoid false negative match if you checking the hostnames as well

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

main business logic developed: saving host and pid when process start running, checking running jobs for host and PID
@todo: check starting time match if PID available on HOST
also some bits'n'bobs for complaint coding
also some modification in database and function scopes
and some minor bits
Make sure that the additotion will not break the cron system also some minor bit
due misinterpret original output
and some minor change to be M2 compliant
reconsider current code structure using other way to decide if process still running
@m2-assistant
Copy link

m2-assistant bot commented Sep 30, 2019

Hi @mamsincl. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 30, 2019

CLA assistant check
All committers have signed the CLA.

Conflicts:
	app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php
@mamsincl
Copy link
Contributor Author

mamsincl commented Oct 11, 2019

Hello @magento-engcom-team, @buskamuza and @tariqjawed83 ,

I do not know If you ever checking tests results if any is failing, but - since I not even touch it - Unit Tests build errors came from original branch.
Also unable to do anything with the suggested version increase in Semantic Version Checker build, since I had to implement those modifications into (even new) files to cover everything (setting with default value, language file, etc)

The failing of Static Tests build is coming from my additions, the reasons are:

  • Magento\Test\Integrity\ObserverImplementationTest is failing since my main function - which is responsible for clearing the database - is public to give the chance to the integrators to implement their own solution (there are multiple workarounds already available to solve the issue)
  • Magento\Test\Php\LiveCodeTest is failing mainly of using shell_exec command: the reason of using it, since Magento Shell class implements exec php command to execute console command - but often happening that the output (of exec) just coming back truncated which happened during development process ..... that is the reason of using forbidden and discouraged php functions
    I am not too keen, but can re-implement Magento\Framework\Shell if necessary even with possible issue of the command output - or even extending Shell class with a new public function to implement shell_exec function (but this also will failing static test with the same issue)

@mamsincl
Copy link
Contributor Author

Hi @magento-engcom-team @magento-cicd2,

Can you review my comments and react as well, please.

Thanks,
Robert

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:19
@brucemead
Copy link
Contributor

Any update on this one please?

@sidolov sidolov added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S1 Affects critical data or functionality and forces users to employ a workaround. labels Aug 12, 2020
@gabrieldagama gabrieldagama self-assigned this Aug 26, 2020
Copy link
Contributor

@gabrieldagama gabrieldagama left a comment

Choose a reason for hiding this comment

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

Hi @mamsincl, thanks for your contribution and sorry for the delay in processing it.

I'm not sure if cleaning of orphan/stale jobs from the cronjob schedule table is a good idea, we are basically masking the problem, and by removing that probably the developer won't even be aware that there could be a problem. Usually, this kind of issue happens when something needs to be fixed, so I guess it would be better something that improves developer visibility on such problems.

I'm not sure if you have seen this but we have this proposal https://github.com/magento/architecture/pull/171/files, which has really nice ideas on how to make cronjob more reliable, if you are interested in this we could start some discussion on how to start such implementation.

Please let me know your thoughts.

Thanks.

@gabrieldagama
Copy link
Contributor

Closing this PR due to contributor inactivity, if you still want to work on this please feel free to reopen it. Thank you!

@m2-assistant
Copy link

m2-assistant bot commented Sep 22, 2020

Hi @mamsincl, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Cron Partner: AYKO Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: needs update Release Line: 2.4 Severity: S1 Affects critical data or functionality and forces users to employ a workaround.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cron job not running after crashed once
6 participants