[11.x] Fix how nested transaction callbacks are handled #48853
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A problem we discovered while working on #48705 was that if you have nested transactions and you add callbacks to them, it's possible some callbacks were mistakenly removed upon failure.
The example @fntneves shared:
The problem is since we only maintain a
$transactions
counter that goes up (when a new transaction is added) and down (when a transaction is committed, or rolled back), it's possible to have callbacks from different transactions that are on the same level intertwined.In that example, callback 1 would be removed as soon as transaction 3 failed, because the first and last transaction share the same levels.
I discussed a few options with Taylor, such as having each transaction hold a unique identifier (uuid or just a counter that only goes up), and he suggested splitting the transactions into "ready" transactions and "pending" transactions. This PR uses the latter. Let me know if you think this makes sense..
Also, thanks @fntneves for pointing that problem on the other PR!
P.S: I was unsure whether to target 11.x or 10.x. There are no contract changes, but behavior changes slightly. Let me know and I can change it.
Pending: