Enable indexer application locking by default

[fredden]

Description

Since Magento version 2.4.3, there has been a feature of the indexer process where Magento can automatically recover from crashed indexer processes. The feature was off by default. This pull request enables the feature by default. Individual applications can explicitly enable/disable this feature as described in the documentation.

https://developer.adobe.com/commerce/php/development/components/indexing/#using-application-lock-mode-for-reindex-processes

Manual testing scenarios

1. Install Magento with no configuration
2. Witness if application locking is used for indexer process

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 @fredden. Thank you for your contribution!
Here are some useful tips on 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 Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[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.

[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.

[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.

[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.

[ihor-sviziev]

Please fix failing static 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.

[ihor-sviziev]

✔ Approved.

Failing tests look not related to changes from this PR.

[engcom-Hotel]

We have discussed this PR with PO internally and as per him we never enabled this feature by default because there is a potential risk involved in using it. There might be a situation when we can have duplicate reindexing processes running at the same time because of this setting depending on the lock provider:

https://experienceleague.adobe.com/docs/commerce-operations/installation-guide/tutorials/lock-provider.html

So we should not proceed with this PR. Hence closing it for now.

Thanks

[fredden]

@engcom-Hotel thanks for the update. I'm disappointed with this response. It makes sense to avoid using a potentially dangerous feature. How can I/the community discuss this with the "PO" (product owner?) that you mentioned? It sounds like there's a separate bug where it's possible to use a lock provider which doesn't provide a reliable locking service. That sounds like something to get fixed, but should not block this in my opinion. I'm open to updating this pull request to set on-by-default for lock providers which are known to be good / work well, and off-by-default for 'bad' lock providers, while we wait for that separate bug to be fixed.

Please can you re-open this pull request as I do not have sufficient permissions on GitHub to do this. The conversation is not yet complete.

[ihor-sviziev]

Re-opened for further discussions

[engcom-Hotel]

@magento run Unit 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.

[chernenm]

hello @fredden, @ihor-sviziev,

Thank you working on this.
Before changing default system behavior, could you please share more details about why it is critical to have this functionality to be enabled by default and why enabling this feature in configuration whenever this is necessary is not sufficient?

[fredden]

From https://developer.adobe.com/commerce/php/development/components/indexing/#using-application-lock-mode-for-reindex-processes:

having this mode enabled will return a more accurate status of the indexer.

sometimes [an indexer status does] not get updated when an indexer fails.

An additional benefit of this mode is that the application will [...] see a more accurate status of the indexers

if an indexer failed, the application will now see this

if an indexer failed, [...] the cronjob will pick up the indexer to try it again

Without this mode, it was necessary to manually reset the indexer when it failed. With this mode enabled, this should no longer be the case if the reindexing doesn't fail again during the next attempt.

These all sound like good things to offer by default. Is there a compelling reason to not provide such functionality by default? Why hide such a positive/useful feature behind a flag that many will not know about?

[fredden]

Here's an example of how "hidden" the feature is: #30012 (comment)

[hostep]

Just found this old comment from some time ago: magento/devdocs#8517 (comment)

[fredden]

Just found this old comment from some time ago: magento/devdocs#8517 (comment)

Thanks, that's a useful insight. I suspect the improvements being made in #30012 are likely to resolve the concern raised there.

[fritzmg]

I agree with @engcom-Hotel 's assessment. We enabled this feature but quickly came into to the opposite situation actually: because the indexer process crashed at some point, the lock file is still present and thus Magento never ever executed that particular cronjob (indexer_update_all_views) again. Also there does not seem to be sufficient documentation on how to manually reset the locks (looking at the code I could not figure out where these locks are actually stored). With the database based locks I was able to just run TRUNCATE TABLE cron_schedule;.

[hostep]

@fritzmg: what was your lock provider set to at that moment (it's defined in the app/etc/env.php file usually)? The default one is to use mysql server as locking engine, but that's sometimes unreliable in my experience especially when your server has a very high load, the lock can then sometimes disappear when it shouldn't disappear. The file based locking setup is much more reliable, and you can specify in which paths the lock files should be written (make sure that path isn't affected by your deploy process).
Also, if file based locking is used, the lock should automatically be removed whenever a php process crashes, which is very good in my opinion (with mysql that probably also happens, but I'm not sure about this).

[fritzmg]

@fritzmg: what was your lock provider set to at that moment (it's defined in the app/etc/env.php file usually)?

'lock' => [
    'provider' => 'db',
    'config' => [
        'prefix' => ''
    ]
],

[hostep]

Okay, so it uses mysql (also note that the prefix is configured empty, which means that it could potentially conflict with multiple magento installations on the same server - if any). I would suggest you try switching to file based locking, it's more stable/reliable in my experience...

[fritzmg]

Okay, so it uses mysql (also note that the prefix is configured empty, which means that it could potentially conflict with multiple magento installations on the same server - if any).

These are just the defaults that Magento provided. There are no other Magento instances on the same server.

I would suggest you try switching to file based locking, it's more stable/reliable in my experience...

I would like to, however apart from this page I cannot find any documentation as to what the recommended settings would be.

[hostep]

You can choose and experiment yourself. Like I said before, in my experience over the past couple of years, the default one (db) is not super reliable, the file is the one to use in my opinion, so if you want to try it out, you'll need to run the command with these two flags: --lock-provider=file & --lock-file-path=/some/path/to/a/directory/to/hold/your/lock/files

[fritzmg]

You can choose and experiment yourself.

I think I lack the required knowledge to do that ;). Aren't there best practises that Magento can provide?

[ihor-sviziev]

@engcom-Hotel any updates on this?