Skip to content
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

[Issue] Avoids endless loop of indexers being marked as invalid #29478

Closed
2 of 4 tasks
m2-assistant bot opened this issue Aug 11, 2020 · 2 comments · Fixed by #29196
Closed
2 of 4 tasks

[Issue] Avoids endless loop of indexers being marked as invalid #29478

m2-assistant bot opened this issue Aug 11, 2020 · 2 comments · Fixed by #29196
Assignees
Labels
Component: Indexer Event: MageCONF CD 2020 Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.3.5 Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@m2-assistant
Copy link

m2-assistant bot commented Aug 11, 2020

This issue is automatically created based on existing pull request: #29196: Avoids endless loop of indexers being marked as invalid

Magento 2.4-develop

Description (*)

This was discovered on a particular project where we noticed that the indexers catalogrule_rule and catalog_category_product are constantly being marked as Reindex required.

The cronjob indexer_reindex_all_invalid is running and is supposed to fix this, but that doesn't happen.
If however, I stop the cronjobs, and manually execute bin/magento indexer:reindex catalogrule_rule catalog_category_product we see a different behavior and only catalog_category_product is then marked as Reindex required.. If I then execute either bin/magento indexer:reindex catalog_category_product or let the cronjob indexer_reindex_all_invalid take over again, everything is getting back to a stable state and all indexers are marked as Ready.

So this means that the execution of the indexer_reindex_all_invalid cronjob and bin/magento indexer:reindex {all-invalid-indexers} are behaving differently which isn't correct in my opinion.

We've seen this happening with Magento OS 2.3.5, and I was able to reproduce it on the 2.4-develop branch (using commit f39a2e0)

I currently only have a way to reproduce it by using the database of this particular project and we also need a particular module from Amasty installed as well (the Improved Layered Navigation module version 2.14.3).
But in my opinion it's not a bug in the Amasty module but a problem in core Magento since we see a difference in behavior in how invalid indexers are reindexed if you do it manually or if the cronjob executes it.

After searching around a bit I noticed a particular commit which was only applied to the manual reindexing command: MAGETWO-51540: Linked indexers catalog_category_product and catalog_product_cateogry do not set valid status after shell run.

So I now introduced these same changes to the code which is used by the indexer_reindex_all_invalid cronjob and after testing these changes on both our project (on Magento 2.3.5) as on my test on the 2.4-develop branch, it looks like it fixes the issue.

In my opinion, this is a pretty important bugfix, because this bug causes the server of this particular project to have a high CPU load because of the constant reindexing which happens with every cron run which is not necessary.

In my opinion, we should also try to refactor the code so both bin/magento indexer:reindex as the cronjob indexer_reindex_all_invalid should use the same code so we can avoid in the future where one fix is applied to one part and forgotten about in the other part.
I've currently chosen not to refactor this so it's easier to review these changes, and maybe somebody can follow up in a later PR to refactor this?

Related Pull Requests

Fixed Issues (if relevant)

  1. None that I could find

Manual testing scenarios (*)

  1. Have magerun2 available to manually execute cronjobs (https://files.magerun.net/)
  2. Setup a clean Magento
  3. Install Amasty's Improved Layered Navigation module version 2.14.3
  4. Use a specific database from our project (sorry, I can't publicly share this database)
  5. Run bin/magento setup:upgrade
  6. Run bin/magento indexer:reindex so all indexers are marked as valid
  7. Invalidate the two specific 2 indexers: bin/magento indexer:reset catalogrule_rule catalog_category_product
  8. Look at the indexers statuses: bin/magento indexer:status (the two indexers should be marked as invalid)
  9. Run php n98-magerun2.phar sys:cron:run indexer_reindex_all_invalid
  10. Look at the indexers statuses: bin/magento indexer:status (the two indexers are still marked as invalid)
  11. Run php n98-magerun2.phar sys:cron:run indexer_reindex_all_invalid
  12. Look at the indexers statuses: bin/magento indexer:status (the two indexers are still marked as invalid)
  13. Run bin/magento indexer:reindex catalogrule_rule catalog_category_product
  14. Look at the indexers statuses: bin/magento indexer:status (only one indexers is marked as invalid: catalog_category_product)
  15. Run php n98-magerun2.phar sys:cron:run indexer_reindex_all_invalid
  16. Look at the indexers statuses: bin/magento indexer:status (All indexers are now finally marked as valid)

With this PR applied, after step 8, we can jump immediately to step 13. This is probably still not 100% fixing the problem since I would expect all indexers to be marked as valid after this, but maybe that Amasty module is intervening here, not exactly sure without more deeper debugging.

If somebody needs the database from this project, I'm willing to provide it to you in private via Slack for example, but it can contain confidential information, so please I'm asking internal Magento devs to not abuse the information in the database.

Thanks!

Questions or comments

Maybe a reproducible case is written down in internal ticket MAGETWO-51540 which you can try to reproduce using the indexer_reindex_all_invalid cronjob?

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)
@magento-engcom-team magento-engcom-team added the Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed label Aug 11, 2020
@sdzhepa sdzhepa added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Aug 11, 2020
@ghost ghost added Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Priority: P2 A defect with this priority could have functionality issues which are not to expectations. labels Sep 8, 2020
@m2-community-project m2-community-project bot added Progress: PR Created Indicates that Pull Request has been created to fix issue and removed Progress: PR in progress labels Sep 24, 2020
@m2-community-project m2-community-project bot added Progress: PR Created Indicates that Pull Request has been created to fix issue and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Sep 24, 2020
engcom-Golf pushed a commit to hostep/magento2 that referenced this issue Sep 28, 2020
@engcom-Alfa engcom-Alfa added Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed labels Oct 16, 2020
@m2-community-project m2-community-project bot added Progress: ready for dev and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Oct 16, 2020
@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Oct 16, 2020
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Alfa
Thank you for verifying the issue. Based on the provided information internal tickets MC-38495 were created

Issue Available: @engcom-Alfa, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@magento-engcom-team magento-engcom-team added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Oct 24, 2020
@magento-engcom-team
Copy link
Contributor

Hi @m2-assistant[bot]. Thank you for your report.
The issue has been fixed in #29196 by @hostep in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.2 release.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Indexer Event: MageCONF CD 2020 Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.3.5 Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants