-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
magento/magento2#: Add a new index for cron_schedule
table
#27391
magento/magento2#: Add a new index for cron_schedule
table
#27391
Conversation
Hi @atwixfirster. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
Query that you show use "where" condition on primary keys and so should run with a millisecond performance See
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add database version and explain for this query ?
Hi, @kandy !
Magento Cloud DB version = Table |
UPDATE here: Today, I've analyzed New Relic reports and detected that a new index did not help to reduce UPDATE execution time. In the same time, I've changed a logic of Magento\Cron\Model\ResourceModel\Schedule::trySetJobStatusAtomic():
The mentioned approach helped to reduce UPDATE queries calls. |
Hi @kandy . Could you put an appropriate label for test coverage? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atwixfirster . We have some problems with checkout to your branch. Could you update it?
Thanks!
Hi, @sidolov can you please review this PR? |
Hi @sidolov, thank you for the review. |
Before this gets merged, can somebody confirm that this won't conflict with #28007 ? |
I can confirm that it will conflict as it duplicate to #28007? |
Hi @atwixfirster, thank you for your contribution! |
@sidolov @gabrieldagama i believe this should be reverted in favor to #28007 that does the same and was approved more than 2 weeks ago |
Status field is no longer used in that query after this PR
Status field is no longer used in the query
Description (*)
Magento\Cron\Model\ResourceModel\Schedule::trySetJobStatusAtomic() executes an UPDATE SQL with an additional
status
parameter:UPDATE is a hard query and New Relic report about this:
PR adds an additional index based on
fields.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)
Resolved issues:
cron_schedule
table #29601: magento/magento2#: Add a new index forcron_schedule
table