[master < T0806-MG] Fix nested transaction error #339
Conversation
antaljanosbenjamin
requested review from
antonio2368,
gitbuda,
jbajic and
kostasrim
|
I will update the docs soon. |
| for (auto &row : result.rows) { | ||
| auto [query, parameters] = | ||
| ExtractTransformationResult(std::move(row.values), transformation_name, stream_name); | ||
| auto [query, parameters] = ExtractTransformationResult(row.values, transformation_name, stream_name); |
There was a problem hiding this comment.
How does this solve the nested transaction error?
There was a problem hiding this comment.
This doesn't solve it. The solution is on line 486 (aborting the transaction in case of serialization error).
This is just a bugfix that was hidden by the original bug: if the data is moved out here, then in the retry theere is no data, so we cannot retry the transaction. That's why I changed this to not move.
There was a problem hiding this comment.
aha, I see now it so if the transaction that has a serialization error it aborts. So why do we have both abort every time and throw after a certain amount of retries, should it not abort when it has tried several times? Does this make the counter redundant?
There was a problem hiding this comment.
@jbajic That's a good question. If you call abort() it means that the whole transaction gets aborted and a new one must start by calling begin(). This is satisfied on line 462 (if you see this is wrapped in a while(true) loop. If there is a serialization error the whole transaction will be replayed - see the inner for-loop). Therefore the counter is for the total amount of retries the whole transaction must replay either by completing successfully or throwing. After all, a transaction is a log of changes done to the storage engine, therefore you either commit it or abort but nothing inbetween
kostasrim
left a comment
kostasrim
left a comment
There was a problem hiding this comment.
Shiny and clean. Thank you for taking care of this -- I introduced this small bug ;)
[master < Task] PR
Docs PR