Problem/Motivation
Ensure post transaction callbacks are always executed ONLY at the end of the root Drupal transaction.
Proposed resolution
We need to keep track of the very first 'root' transaction object created, because under some circumstances we might end up processing more than one of them during the transaction stack lifecycle.
For example, a root transaction is opened, then a DDL statement is executed in a database that does not support transactional DDL, and another root opened before the original one is closed. By having the first one tracked, we can ensure that post transaction callbacks are processed only upon its destruction and not before.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3440848
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3440848-ensure-post-transaction
changes, plain diff MR !7484
Comments
Comment #3
mondrakeComment #4
mondrakeComment #5
mondrakeComment #6
mondrakeComment #7
daffie commentedAll the code changes look good to me.
The added code comments are clear.
For the different situations is testing added.
The testbot is green for all supported databases.
For me it is RTBC.
Comment #8
mondrakeComment #10
quietone commentedI read the issue summary and the MR. I did not do a code review, just the comments. I applied my own suggestions for minor grammar fixes. The comments on the whole make sense but I suggested some improvements. And I had a question about comments in the tests. I am going to set this to Needs Work so someone checks the suggestions.
I have added credit as well.
Comment #11
mondrakeApplied @quietone suggestions, but then changed a bit again as I thought the end result was still not accurate. Not a native speaker, so please bear with me.
Comment #12
quietone commented@mondrake, thanks for updating the comments. They are much easier to understand now and will benefit those who come after us. Thanks!
I was about to restore the RTBC when I checked the tests. There is a failing test, core/tests/Drupal/FunctionalTests/Installer/InstallerTranslationExistingFileTest.php and I have restarted that set of functional tests. Once tests are passing this can go back to RTBC.
Comment #13
mondrakeRebased, tests are green now so back to RTBC per #12.
Comment #14
mondrakeComment #15
quietone commentedComment #16
mondrake@quietone after your change, the title text no longer reads. Unsure what you meant to change, so leaving as is.
Comment #22
larowlanFixed a typo in a comment, waited for a green test-run - which passed.
Committed to 11.x and backported 11.0.x, 10.4.x and 10.3.x
Thanks folks 🙌
Comment #25
wim leersWow! What an epic bug and fascinating fix! 👏