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

Catch throwables in mview updating #23124

Conversation

QuentinFarizon
Copy link

@QuentinFarizon QuentinFarizon commented Jun 4, 2019

2.2 : #23124
2.3 : #23125

Description (*)

Certain type of runtime exception weren't caught during mview updating, which caused some mview to be stuck in "processing" mode.
Refer to #23054 for more detail about the issue.

This modification is very similar to the one made here : 5927a75

Fixed Issues (if relevant)

#23054

Manual testing scenarios (*)

  • make an mview updating crash with a Throwable (not an Exception)
  • process ends, mview is still in 'processing' in database
  • it is never picked up again

Questions or comments

This does not prevent against all types of crashes (like process out of memory or server stop abruptly, so it is only a step in making indexing crons more resilient

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)

@m2-assistant
Copy link

m2-assistant bot commented Jun 4, 2019

Hi @QuentinFarizonAfrimarket. 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 give me test instance - deploy test instance based on PR changes
  • @magento give me 2.2-develop instance - deploy vanilla Magento instance

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

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jun 4, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuentinFarizonAfrimarket all changes must be applied to 2.3-develop first.

If they are already fixed there, instead of creating new commits appropriate ones should be backported.

@QuentinFarizon
Copy link
Author

Hello @orlangur ok thanks, I have opened the PR on 2.3 also : #23125

@VladimirZaets
Copy link
Contributor

Hi @QuentinFarizonAfrimarket . The branch 2.2-develop will be closed for contribution from July 15, 2019. Please finish all your Pull Requests till this date, otherwise, they will be closed.
Thanks for collaboration.

@orlangur
Copy link
Contributor

@VladimirZaets any idea why #23125 is still not merged having approval since Jun 5th?

@QuentinFarizon QuentinFarizon force-pushed the mview_catch_throwables branch from 26e9f35 to bdc588f Compare July 15, 2019 10:49
@QuentinFarizon
Copy link
Author

I was indeed waiting for #23125 to be merged to bring changes here, but with the due date being today I have pushed a version which fixes static tests, in the hope it will be taken in 2.2-develop

@QuentinFarizon QuentinFarizon requested a review from orlangur July 15, 2019 10:53
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @QuentinFarizonAfrimarket! Please re-resolve conflict though :)

@QuentinFarizon QuentinFarizon force-pushed the mview_catch_throwables branch from e11db82 to e05e46d Compare July 15, 2019 12:50
@QuentinFarizon
Copy link
Author

Sorry, should be better like this :)

@orlangur
Copy link
Contributor

@QuentinFarizonAfrimarket definitely ;) A small thing left, unit test needs to be adjusted.

@VladimirZaets
Copy link
Contributor

Hi @QuentinFarizonAfrimarket.
The next 2.2 release will be the last of functional releases.
Due to it, we are closing 2.2-develop for contribution.
Thanks for collaboration.

@m2-assistant
Copy link

m2-assistant bot commented Jul 16, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants