Add --no-db-upgrade option to setup:upgrade

[AntonEvers]

Description (*)

Maintenance mode during deployment on cloud, as well as on-premise, can be significantly shortened, by skipping schema and data install/update steps.
"Zero-Downtime Deployment" can still requires up to 5 minutes of downtime with big databases. With databases of 100+ GiB, the recurring operations in setup:upgrade can take 5 minutes, even if there is no module to update. This leads to timeouts on the frontend system, which is waiting for Adobe Commerce to respond, often no longer than 30 seconds before timing out (if the customer has not left by then).

Reducing the maintenance mode from 5 minutes to 15 seconds, can be achieved by skipping lengthy database operations, if all modules are up-to-date. But we cannot simply skip setup:upgrade entirely when setup:db:status tells us that modules are up to date. The setup:upgrade command contains more than just upgrade code, that can not be found in any other command. For instance the \Magento\Setup\Model\Installer::updateModulesSequence method and \Magento\Setup\Model\Installer::removeUnusedTriggers, but also \Magento\Setup\Model\SearchConfig::validateSearchEngine which is only found on the search config save action but not in any other deployment command.

Because of this, I would like to add the option to skip schema and data upgrades when executing setup:upgrade. Deployments should execute setup:db:status before going into maintenance mode and use the outcome of that command to determine whether to skip the schema and data upgrades.

An alternative was to include the setup:db:status within setup:upgrade and let it decide for itself whether to run DB upgrades or not, which I have created a PR for in ed11437, but this results in executing setup:db:status inside maintenance mode, adding 10 seconds to maintenance mode, for a command that does not need maintenance mode to be enabled.

Below diagram shows the current deployment, with maintenance mode enabled, and setup:upgrade taking 5 minutes of that time.

The second diagram shows what it could look like, when skipping $installer->installSchema($request) and $installer->installDataFixtures($request, true) in the setup:upgrade command, based on the outcome of setup:db:status.

A 15 second delay in response time during deployment is completely acceptable. Whereas a 1 - 5 minute downtime is often not.

I have added the --no-db-upgrade flag to the setup:upgrade command in https://github.com/magento/magento2/pull/35116/files

ECE tools should run setup:db:status before maintenance mode, to determine whether the --no-db-upgrade flag should be set when using the setup:upgrade command. If all modules are up to date, the flag should be added to skip unnecessary code execution in maintenance mode. The request for this ECE tools update is logged in MDVA-43621.

Related Pull Requests

Fixed Issues (if relevant)

1. MDVA-43621

Manual testing scenarios (*)

1. bin/magento setup:upgrade --no-db-upgrade --keep-generated should NOT RUN upgrades
2. bin/magento setup:upgrade --keep-generated should RUN upgrades

Questions or comments

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)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @AntonEvers. Thank you for your contribution
Here are 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.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[AntonEvers]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[AntonEvers]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[AntonEvers]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[fooman]

Could the same end result be achieved like so

if (setup:db:status) {
    setup:upgrade
}

?

Without the need for an extra flag?

I believe that is what Deployer Plus has been doing for a few years now.

[AntonEvers]

Hi @fooman,

It is different from what you describe.

if (!setup:db:status) {
    setup:upgrade
} else {
    app:config:import
}

We will call that "deployer-plus".
We will call this PR, "no-db-upgrade"

Take a look at Magento\Setup\Console\Command\UpgradeCommand:: execute().

Method	deployer-plus	no-db-upgrade
updateModulesSequence	No	Yes
validateSearchEngine	No	Yes
removeUnusedTriggers	No	Yes
app:config:import	Yes	Yes

Especially updateModulesSequence is important here, I think you can live without validateSearchEngine and removeUnusedTriggers.

\Magento\Setup\Model\Installer::updateModulesSequence can be necessary to change the order of modules in app/etc/config.php after you add a dependency sequence in a module's module.xml. That might be needed after a code change in a module, when you introduce a new dependency, but no DB changes.
You will need to change the order in which modules depend on each other. Without running setup:upgrade you will not run \Magento\Setup\Model\Installer::updateModulesSequence and you will not change the order of modules in config.php.
You do need to run \Magento\Setup\Model\Installer::updateModulesSequence but you do not need to run DB schema and data install/update commands in that case. And there we have no solution at the moment. An alternative would be to create a new CLI command which exclusively runs \Magento\Setup\Model\Installer::updateModulesSequence and run that together with app:config:import if you don't need module upgrades. I found this PR less complex.

[fredden]

Updating the module sequence is a development task, not something that should be done during deployment. The deployment process should compile and deploy a known-state, not make configuration changes (ie, write to app/etc/config.php).

Deployer-plus actually doesn't run setup:upgrade at all (nor does Capistrano), instead it runs setup:db-schema:upgrade then setup:db-data:upgrade, but only if setup:db:status says that there are database changes required.

https://github.com/jalogut/magento2-deployer-plus/blob/a8c4432a68a4ac442541f250577e6049abaed52b/recipe/magento_2_2/database.php#L36-L41

https://github.com/davidalger/capistrano-magento2/blob/3d32c7d654bd9221f67e20c27fc4e62503bdd73d/lib/capistrano/tasks/deploy.rake#L86-L87

Perhaps the change that's needed here is to convert ece-tools to use setup:db-schema:upgrade and setup:db-data:upgrade instead of setup:upgrade.

https://github.com/magento/ece-tools/blob/b4ea5e111aeadb6d1d06e4a9983288eb73bd91ca/scenario/deploy.xml#L105
https://github.com/magento/ece-tools/blob/b4ea5e111aeadb6d1d06e4a9983288eb73bd91ca/src/Step/Deploy/InstallUpdate/Update/Setup.php#L55
https://github.com/magento/ece-tools/blob/b4ea5e111aeadb6d1d06e4a9983288eb73bd91ca/src/Util/UpgradeProcess.php#L88

Do you have any information about what tasks are taking a long time when you run setup:upgrade locally? Perhaps those tasks can / should be optimised / fixed.

[fooman]

@AntonEvers thanks for getting back to me. I believe a committed app/etc/config.php should help with the module order. If you run bin/magento module:enable Fooman_Example the sequence gets written to file. If for some reason the module sequence changed running bin/magento module:enable Fooman_Example && bin/magento module:enable Fooman_Example will do the same.

However maybe this points to a need for a separate command to check module sequence?

[AntonEvers]

@fooman absolutely, a committed config.php with the right sequence will also remove the need for running updateModulesSequence. My experience is that we cannot consistently rely on a manually updated config.php file, so an automated check will be useful. I am thinking about including the sequence check in the setup:db:status command.
After all setup:db:status should determine if you should run setup:upgrade. It would fit within that definition. Only the semantics are in the way because of the :db: part in setup:db:status.
And it might be loosely backward incompatible to start checking config.php during that command as it might change deployment behavior. So indeed better to create a new command for it, or add it as an optional part of setup:db:status by use of a flag.

[BarnyShergold]

@AntonEvers Based on discussions on the internal ticket MCLOUD-8511 and that skipping setup:upgrade could cause other issues (such as not running recurring scripts) and that the solution would actually slow down deployments, can this PR be closed?

[hostep]

@AntonEvers:

My experience is that we cannot consistently rely on a manually updated config.php file, so an automated check will be useful. I am thinking about including the sequence check in the setup:db:status command.

This already exists, by the command bin/magento module:config:status (introduced in Magento 2.3.3 by #22733 😉 ), just FYI

[AntonEvers]

@hostep, @fooman and @BarnyShergold with the current code base I would say that it is not beneficial indeed to skip setup:upgrade after all. Because of @BarnyShergold 's response I evaluated all recurring scripts to see what we are dealing with. Here is my outcome:

File	Responsibility	Expected Duration	Safe to Skip
\Dotdigitalgroup\Email\Setup\Recurring	DotDigital notifier, and install-only script for Foreign keys outside of the module. Adds foreign keys if they are missing between catalog_product_entity.entity_id and email_catalog.product_id. This should happen only once, during installation. Creates the email_abandoned_cart if it does not exist. If connector_configuration / tracking / integration_insights is enabled, tells DotDigital which urls are using it using an API call to DotDigital.	short	yes
\Magento\ComposerRootUpdatePlugin\Setup\RecurringData	Will not run if cloud installation detected Passthrough Magento setup command to check the plugin installation in the var directory Can also be done with composer magento-update-plugin install However this is installing WebSetupWizardPluginInstaller and most customers don't use and should not use the web setup wizard.	short	yes
\Magento\GoogleShoppingAds\Setup\Recurring	Disabled from version 4.0.1 Sets scconnector_google_feed and scconnector_google_remove index to realtime if they were set to scheduled.	short	yes
\Magento\Amqp\Setup\Recurring	Installs AMQP Exchanges and Queues. Will run anyway on first install and on each DB change. But should only execute once, or if queue solution changes.	short	yes
\Magento\Bundle\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\CatalogInventory\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\CatalogUrlRewrite\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\Catalog\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\ConfigurableProduct\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\ConfigurableSampleData\Setup\RecurringData	Reindex All. But this module should never be installed in production env	long	yes
\Magento\Cron\Setup\Recurring	Updates the cron_schedule table and sets all running jobs to ERROR with the message: "The job is terminated due to system upgrade". Side note: these jobs can have successfully run and finished. This script only updated the cron_schedule table and does not do anything with cron jobs themselves. This can be skipped on non-DB updates if a separate command is created for this. Besides, this does not need maintenance mode to be enabled.	short	no
\Magento\Customer\Setup\RecurringData	Reindexes customer grid table if it needs reindexing. But this is already picked up by the indexer_reindex_all_invalid cron job and does not need to be done in maintenance mode.	long	yes
\Magento\Indexer\Setup\Recurring	If indexer config has changed, invalidates indexes involved If indexes are scheduled, removes and re-adds triggers Saves md5 hash of index config in indexer_state table This should be part of the setup:db:status checks. If it is, then this can be safely skipped on non-DB deployment.	short	no
\Magento\Integration\Setup\Recurring	Only necessary for customers that implement integration.xml files in order to maintain integration accounts. This also does not need maintenance mode to be enabled. Also it could be duplicated in a bin/magento command. If that were done, this can be safely skipped on non-DB deployment.	short	no
\Magento\MysqlMq\Setup\Recurring	Adds MySQL Queue tables if new ones have been added in Queue config. Could be duplicated in a bin/magento command. If that were done, this can be safely skipped on non-DB deployment.	short	no
\Magento\ProductAlert\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\Reports\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\SalesArchive\Setup\Recurring	Syncs Magento sales archive table structure. Because of the fact that the source tables need to have changed, this will only run on a deployment where DB tables are altered. And so it can safely be skipped on a non-DB deployment.	long	yes
\Magento\SalesSequence\Setup\Recurring	Creates sales sequences. Can be skipped once your stores are set up and running. Needs to be run on new store creation and it does already, as it is also running on the store_add event.	short	yes
\Magento\SalesSequence\Setup\RecurringData	Creates sales sequences. Can be skipped once your stores are set up and running. Needs to be run on new store creation and it does already, as it is also running on the store_add event.	short	yes
\Magento\Theme\Setup\RecurringData	Reindexes the design_config_grid index. Registers themes in the database. Unless you are regularly adding / removing themes, you should be able to skip this.	medium	no
\Magento\Weee\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes
\Magento\Wishlist\Setup\Recurring	Install-only script for Foreign keys outside of the module.	short	yes

This might be valuable info for the maintainers of Deployer Plus as well.

My personal view is that the steps that cannot be skipped should become part of setup:db:status as they will trigger a DB change based on other things then what is currently measured in setup:db:status.

For now I will look into my clients custom recurring scripts and recurring data scripts, maybe even add some logging so that I can see which script takes so long during the deployment. Because as far as I can see deployment should not take long at all if the above scripts are executed, and no sample data is installed.

Customer grid reindex is the most probable suspect for customers without recurring scripts of their own I think. Personally I would patch this so that it is not executed during deployment.