Skip to content

Fix Issue: “Asymmetric transaction rollback error” if commit callbacks throw exceptions #9955

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

Merged
merged 19 commits into from
Jul 11, 2017

Conversation

waynetheisinger
Copy link
Contributor

@waynetheisinger waynetheisinger commented Jun 15, 2017

If a callback in commit throws an excecption, the calling plugin needs to be able to distinguish this from a unsuccessful commit, so that it doesn't attempt a rollback on an already commited transaction. (Which would cause an “Asymmetric transaction rollback" error)

Description

Fixed Issues (if relevant)

  1. “Asymmetric transaction rollback error” if commit callbacks throw exceptions #6497: “Asymmetric transaction rollback error” if commit callbacks throw exceptions

Manual testing scenarios

Let a commit callback throw an exception

Example:

  1. create a new search engine

     <type name="Magento\Search\Model\AdapterFactory">
         <arguments>
             <argument name="adapters" xsi:type="array">
                 <item name="dummy" xsi:type="string">Magento\Framework\Search\Adapter\Mysql\Adapter</item>
             </argument>
         </arguments>
     </type>
    
  2. Do not register a corresponding indexer handler for Magento\CatalogSearch\Model\Indexer\IndexerHandlerFactory

  3. The commit callback from the fulltext indexer plugin will cause an exception "There is no such indexer handler: dummy"

  4. Trigger this commit (for example, save a product)

Expected result

Exception from the commit callback is thrown and can be catched later (for ex. by PHPUnit)

Actual result

rollback() is called after commit() already has been called for the same transaction, resulting in an “Asymmetric transaction rollback error” exception when Magento tries to commit or rollback the last transaction on the stack.

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 on Travis CI are green)

If a callback in commit throws an excecption, the calling plugin needs to be able to distinguish this from a unsuccessful commit, so that it doesn't attempt a rollback on an already commited transaction. (Which would cause an  “Asymmetric transaction rollback" error)
If a callback in commit throws an excecption, the calling plugin needs to be able to distinguish this from a unsuccessful commit, so that it doesn't attempt a rollback on an already commited transaction. (Which would cause an  “Asymmetric transaction rollback" error)
If a callback in commit throws an excecption, the calling plugin needs to be able to distinguish this from a unsuccessful commit, so that it doesn't attempt a rollback on an already commited transaction. (Which would cause an  “Asymmetric transaction rollback" error)
If a callback in commit throws an excecption, the calling plugin needs to be able to distinguish this from a unsuccessful commit, so that it doesn't attempt a rollback on an already commited transaction. (Which would cause an  “Asymmetric transaction rollback" error)
@maghamed maghamed self-assigned this Jun 15, 2017
@ishakhsuvarov ishakhsuvarov self-assigned this Jun 15, 2017
@ishakhsuvarov ishakhsuvarov added this to the June 2017 milestone Jun 15, 2017
Copy link
Contributor

@maghamed maghamed left a comment

Choose a reason for hiding this comment

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

Make sense for me.

Thanks for contribution

@maghamed
Copy link
Contributor

maghamed commented Jul 3, 2017

I left a comment regarding this PR in the GitHub issue #6497

#6497 (comment)

@waynetheisinger
Copy link
Contributor Author

waynetheisinger commented Jul 3, 2017

@maghamed I have replied to your comment regarding this PR in the GitHub issue #6497

#6497 (my reply)

If a callback in commit throws an excecption, we mustn't attempt a rollback on an already commited transaction. (Which would cause an  “Asymmetric transaction rollback" error) - so rather than throwing the exception we will just log it.
@waynetheisinger
Copy link
Contributor Author

@maghamed - I've reverted the original changes and added the logging - so this is ready for another look.

@magento-team magento-team merged commit e2c1bb7 into magento:develop Jul 11, 2017
magento-team pushed a commit that referenced this pull request Jul 11, 2017
magento-team pushed a commit that referenced this pull request Oct 3, 2017
magento-team pushed a commit that referenced this pull request Oct 3, 2017
magento-team pushed a commit that referenced this pull request Oct 6, 2017
…ck error” if commit callbacks throw exceptions #9955 - for 2.2
magento-team pushed a commit that referenced this pull request Oct 6, 2017
…ck error” if commit callbacks throw exceptions #9955 - for 2.2
magento-team pushed a commit that referenced this pull request Oct 6, 2017
…ck error” if commit callbacks throw exceptions #9955 - for 2.2
magento-team pushed a commit that referenced this pull request Oct 6, 2017
…ck error” if commit callbacks throw exceptions #9955 - for 2.2
lsrocha pushed a commit to chaordic/magento2 that referenced this pull request Jun 27, 2018
…ck error” if commit callbacks throw exceptions magento#9955 - for 2.2
lsrocha pushed a commit to chaordic/magento2 that referenced this pull request Jun 27, 2018
…ck error” if commit callbacks throw exceptions magento#9955 - for 2.2
lsrocha pushed a commit to chaordic/magento2 that referenced this pull request Jun 28, 2018
…ck error” if commit callbacks throw exceptions magento#9955 - for 2.2
lsrocha pushed a commit to chaordic/magento2 that referenced this pull request Jun 28, 2018
…ck error” if commit callbacks throw exceptions magento#9955 - for 2.2
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