I think "must rollback after error" behavior would be nice to have enforced more so in MW. Right now there are just conventions:
If an error occured, then either that statement *or* everything in the transaction must be rolled back (depending on RDBMs, its server config, and the query). Without knowing what, if anything, can be salvaged, the best thing is to rollback everything, included other "peer" transactions active (e.g. LBFactory::rollbackMasterChanges).
To do
- Deprecate and remove DBO_IGNORE flag.
- Make setFlag() disallow and throw for toggling DBO_IGNORE – e0805d32e4
- Remove support for DBO_IGNORE flag in reportQueryError(). This makes the flag a no-op. https://gerrit.wikimedia.org/r421694
- Silently strip DBO_IGNORE from Database constructor args. https://gerrit.wikimedia.org/r421694
Example:
BEGIN
[PHP func X] write A_X
[PHP func Y] try {
write B_Y (FAILED, transaction reset to A_X, BEGIN, or gone?)
} catch ( ... ) {
<calls rollbackMasterChanges()>
ROLLBACK
<rethrows error>
}
<any higher try/catch call rollbackMasterChanges() too>
onTransaction(PreCommitOr)Idle:
[PHP func X] <cancelled>
AtomicSectionUpdate/AutoCommitUpdate:
[PHP func Z] <cancelled>
Writes ultimately applied: [] (SAFE)The ability to try/catch and keep writing to mysql always scared me,
since it could horribly botch things depending on the case...
Case I: failing query aborts only itself (e.g. duplicate key)
BEGIN
[PHP func X] write A_X
[PHP func Y] try {
write B_Y (FAILED, server rolls back to A_X)
} catch ( ... ) {
write C_Y
}
[PHP func Z] write D_Z
COMMIT
onTransaction(PreCommitOr)Idle:
[PHP func X] write E_X [AUTO_COMMIT]
AtomicSectionUpdate/AutoCommitUpdate:
[PHP func Z] BEGIN
write F_Z
COMMIT
Writes ultimately applied: [A_X, C_Y, D_Z, E_X, F_Z] (SAFE)Case II: failing query aborts transaction content (e.g. deadlock)
BEGIN
[PHP func X] write A_X
[PHP func Y] try {
write B_Y (FAILED, server rolls back to BEGIN)**
} catch ( ... ) {
write C_Y
}
[PHP func Z] write D_Z
COMMIT
onTransaction(PreCommitOr)Idle:
[PHP func X] <cancelled>
AtomicSectionUpdate:
[PHP func Z] <cancelled>
Writes ultimately applied: [C_Y, D_Z] (UNSAFE; what if A <=> D?)
** I see a bug where trxLevel is set to 0 by a wasDeadlock() check,
but the BEGIN survives server side; the code should issue ROLLBACK.
The above assumes this is fixed.Case III: failing query aborts entire transaction (e.g. server has gone away)
BEGIN
[PHP func X] write A_X
[PHP func Y] try {
write B_Y (FAILED, server already rolled back transaction on disconnect)
<reconnect> [DB layer]
BEGIN [DB layer]
<no retry of write due to pending A_X write at the time>
} catch ( ... ) {
write C_Y
}
[PHP func Z] write D_Z
COMMIT
onTransaction(PreCommitOr)Idle:
[PHP func X] <cancelled>
AtomicSectionUpdate:
[PHP func Z] <cancelled>
Writes ultimately applied: [C_Y, D_Z] (UNSAFE; what if A <=> D?)Some mechanisms to prevent this bugs should be added. Once an error happens, the IDatabase handle should be in an error state, disallowing most queries.
To get out of the error state *using* ROLLBACK, perhaps:
- The owner of the transaction is explicit and issues rollback()
- A rollback() is issued with FLUSHING_INTERNAL (e.g. from LB/LBFactory)
To get it out of an error state other than ROLLBACK, perhaps:
- For SQL errors known to cause the server to rollback only the failed query:
- An atomic section was active and the owner issues resumeAtomic() or endAtomic()
- An atomic section with a savepoint was active (https://gerrit.wikimedia.org/r/#/c/420219) and the savepoint owner issues cancelAtomic()
- The owner of the transaction issues commit()
- A commit() is issued with FLUSHING_INTERNAL (e.g. from LB/LBFactory)
- For SQL errors known to cause the server to rollback everything
- (no way to continue aside from rollback)
- For SQL that might or might not cause the server to rollback everything
- (no way to continue aside from rollback)
We sort-of enforce who can rollback in that a rollback() triggered by some other code between a functions's startAtomic('x') and endAtomic('x') calls will result in an error ('Invalid atomic section') on endAtomic()...if that bubbles up it will cause rollback. Unexpected commit()s like that also error out. However, not everything is wrapped in atomic sections, or related writes might be in three separate atomic sections. There is also the check in rollback() against DBO_TRX for multi-DB transaction rounds.