Disable cron:run when maintenance mode is enabled #21181

kassner · 2019-02-13T09:33:40Z

Description

I believe cron jobs should be disabled while setup:upgrade is being run, to avoid concurrent accesses, which can lead to table locks that can make setup upgrade fail, plus we'll be using stale/wrong data for the cron jobs.

Since the maintenance mode makes sure the frontend is inaccessible, avoiding any writes to the database through it, I believe we should disable crons as well, as they have a heavy impact on the database.

Manual testing scenarios

Run php bin/magento maintenance:enable;
Run php bin/magento cron:run;
Cron jobs should not run;

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 on Travis CI are green)

magento-engcom-team · 2019-02-13T09:33:44Z

Hi @kassner. 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-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

miguelbalparda · 2019-02-13T11:17:26Z

Since the maintenance mode makes sure the frontend is inaccessible, avoiding any writes to the database through it, I believe we should disable crons as well, as they have a heavy impact on the database.

This is true only for some cases, I'm not sure if this should be included in the core. Maybe create a config instead of assuming this applies to everybody?

ihor-sviziev · 2019-02-13T11:21:13Z

Hi @miguelbalparda,
Is there any use case when cron jobs should be running during enabled maintenance mode?

miguelbalparda · 2019-02-13T11:24:16Z

It depends on how people is using maintenance mode. I've seen admins putting the site in that mode just to do changes on prices or CMS pages or even deploying new code changes while on maintenance, which sometimes require crons to be run.

ihor-sviziev · 2019-02-13T13:15:09Z

So in this case would be better to add some parameter to env.php file, for instance “disable_cron_running_in_maintenance_mode”. If this parameter exists and it is set to 1 - than cron jobs should not be running. What do you think?

…

On Wed, Feb 13, 2019 at 13:24 Miguel Balparda ***@***.***> wrote: It depends on how people is using maintenance mode. I've seen admins putting the site in that mode just to do changes on prices or CMS pages or even deploying new code changes while on maintenance, which sometimes require crons to be run. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#21181 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AByXUTH7JT77JCGWS4yeJrx-sTsel0sqks5vM_X-gaJpZM4a4-da> .

miguelbalparda · 2019-02-13T14:08:43Z

I think that’s no different than disabling Magento_Cron using the bin/magento tool and should have the same effect without adding anything to the core for an use case that’s not super intuitive.

kassner · 2019-02-13T14:10:54Z

Disabling Magento_Cron with module:disable is not the same, as it will prevent future db upgrades while running setup:upgrade.

miguelbalparda · 2019-02-13T14:14:36Z

Which is consistent with the description of this PR regarding no DB writes while in maintenance, achieving virtually the same objective.

ihor-sviziev · 2019-02-13T14:19:49Z

With disabling cron module we’ll have few issues: 1. We’ll have errors that command cron:run doesn’t exists, these messages will be sent to email. As for me - it’s not expected 2. Any amount of modules might have dependence on cron module, so we have to disable all together, it means - not really useful

…

On Wed, Feb 13, 2019 at 16:15 Miguel Balparda ***@***.***> wrote: Which is consistent with the description of this PR regarding no DB writes while in maintenance, achieving virtually the same objective. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#21181 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AByXUe02G8XiaUuiFkTsXc2KzW0V72lBks5vNB3wgaJpZM4a4-da> .

kassner · 2019-02-13T14:21:40Z

@miguelbalparda no, as I am enabling maintenance mode just because I want to run setup:upgrade without anything else concurrently. I still want to run Magento_Cron's UpgradeData/UpgradeSchema.

miguelbalparda · 2019-02-13T17:11:29Z

Maintenance windows are very short usually, which means --force can be used to disable Magento_Cron just for the duration of the window without causing much issues. I still don't see why an edge case like this should be considered in the core, the reasoning behind an inclusion should be a broader benefit and I don't see that in this issue. In either case, this should be consulted with an architect IMO.

sivaschenko · 2019-02-13T18:27:43Z

I think it is a good idea to disable cron by default during maintenance mode as it appears to be the safes approach. I'd add a command option to maintenance:enable to keep cron jobs running for those who are confident that cron jobs are not going to be affected during maintenance operations.

ihor-sviziev · 2019-02-13T20:15:28Z

Hi @kandy @buskamuza,
What do you think about such proposal?

NadiyaS · 2019-02-13T22:53:46Z

app/code/Magento/Cron/Console/Command/CronCommand.php

+        if ($this->maintenanceMode->isOn()) {
+            $output->writeln('<info>' . 'Cron is disabled because the maintenance mode is enabled.' . '</info>');
+            return;
+        }
        if (!$this->deploymentConfig->get('cron/enabled', 1)) {


Please pay attention to this configuration. You already may disable cron. Add cron/enabled as 0 to configuration file. There is no special CLI command for this setting for now.

NadiyaS · 2019-02-13T22:56:16Z

Hi,
First, there may be some possible cases when user enables maintenance mode and wants to have cron running.
Also, there may be cases when you want just to disable cron without maintenance mode.
And we already have this possibility. I added comment to the review.
You may add cron/enabled config as 0 to configuration file (e.g. app/etc/env.php) and cron won't be run.
We did not implement CLI command to manage this configuration yet.

ihor-sviziev · 2019-02-14T08:19:30Z

@NadiyaS good catch!
In this case I think would be better to add cli commands cron:enable and cron:disable. @kassner @buskamuza what do you think?

ihor-sviziev

Hi,

As we discussed with Architects in #appdesign - it's better to introduce cron:enable and cron:disable CLI commands, that will fit all needs and will not add backward incompatible changes.

@kassner could you update your PR with these chagnes?

sidolov · 2019-03-28T14:37:28Z

@kassner , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

ghost · 2019-03-28T14:37:31Z

Hi @kassner, 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.

Rafael Kassner added 2 commits February 13, 2019 10:26

Disable cron:run when maintenance mode is enabled

25a803b

Disable cron:run when maintenance mode is enabled

ebdaf98

magento-engcom-team added Component: Cron Release Line: 2.3 Partner: Vaimo Pull Request is created by partner Vaimo partners-contribution Pull Request is created by Magento Partner labels Feb 13, 2019

Rename unit test method

92b0fc2

ihor-sviziev self-assigned this Feb 13, 2019

NadiyaS reviewed Feb 13, 2019

View reviewed changes

ihor-sviziev requested changes Mar 14, 2019

View reviewed changes

ghost added the Progress: needs update label Mar 14, 2019

sidolov closed this Mar 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable cron:run when maintenance mode is enabled #21181

Disable cron:run when maintenance mode is enabled #21181

kassner commented Feb 13, 2019

magento-engcom-team commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019 •

edited

ihor-sviziev commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019 via email

miguelbalparda commented Feb 13, 2019

kassner commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019 via email

kassner commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

sivaschenko commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019

NadiyaS Feb 13, 2019

NadiyaS commented Feb 13, 2019 •

edited

ihor-sviziev commented Feb 14, 2019 •

edited

ihor-sviziev left a comment

sidolov commented Mar 28, 2019

ghost commented Mar 28, 2019

Disable cron:run when maintenance mode is enabled #21181

Disable cron:run when maintenance mode is enabled #21181

Conversation

kassner commented Feb 13, 2019

Description

Manual testing scenarios

Contribution checklist

magento-engcom-team commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019 • edited

ihor-sviziev commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019 via email

miguelbalparda commented Feb 13, 2019

kassner commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019 via email

kassner commented Feb 13, 2019

miguelbalparda commented Feb 13, 2019

sivaschenko commented Feb 13, 2019

ihor-sviziev commented Feb 13, 2019

NadiyaS Feb 13, 2019

Choose a reason for hiding this comment

NadiyaS commented Feb 13, 2019 • edited

ihor-sviziev commented Feb 14, 2019 • edited

ihor-sviziev left a comment

Choose a reason for hiding this comment

sidolov commented Mar 28, 2019

ghost commented Mar 28, 2019

miguelbalparda commented Feb 13, 2019 •

edited

NadiyaS commented Feb 13, 2019 •

edited

ihor-sviziev commented Feb 14, 2019 •

edited