Skip to content
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

Transaction isolation broken with MySQL #7528

Closed
mstilkerich opened this issue Aug 2, 2020 · 0 comments · Fixed by #7529
Closed

Transaction isolation broken with MySQL #7528

mstilkerich opened this issue Aug 2, 2020 · 0 comments · Fixed by #7529

Comments

@mstilkerich
Copy link
Contributor

On MySQL with InnoDB, situations may arise in concurrent transactions where locks acquired by transactions conflict such that MySQL detects a deadlock and rolls back one or more of the participating transactions until the deadlock situation is resolved.

MySQL Manual:

When deadlock detection is enabled (the default), InnoDB automatically detects transaction deadlocks and rolls back a transaction or transactions to break the deadlock.

At this point, the transactions chosen for rollback my the DBMS have been rolled back and terminated. The error should be signaled to the application, which can decide whether to restart the entire transaction from scratch or leave it be.

Roundcube, however, for this specific error would re-issue the last statement that raised the deadlock error up to 2 times before handing the error up to the application (in my case, the application is the rcmcarddav plugin).

See program/lib/Roundcube/db/mysql.php

    protected function handle_error($query)
    {
        $error = $this->dbh->errorInfo();

        // retry after "Deadlock found when trying to get lock" errors
        $retries = 2;
        while ($error[1] == 1213 && $retries >= 0) {
            usleep(50000);  // wait 50 ms
            $result = $this->dbh->query($query);
            if ($result !== false) {
                return $result;
            }
            $error = $this->dbh->errorInfo();
            $retries--;
        }

        return parent::handle_error($query);
    }

Now this is bad, because

  1. it continues executing statements in the middle of a transaction that has been rolled back, potentially leaving the data in an inconsistent state
  2. the application gets no indication whatsoever that something went wrong, and thus continues to execute one-by-one without being guarded by a transaction

I found this using a test with a simple lost update problem (please don't try to make sense of it, I just used some database field for the sake of the test). It reads a string from a database row, and appends a client-specific suffix to the string. The transactions interleave such that both transactions read the old value from the database and afterwards issue their updates. The transactions are set to use Serializable isolation level, so the result must be as if they executed in sequence (or some may be rolled back at the option of the DBMS).

Each client tries the following transaction:

SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE;
START TRANSACTION READ WRITE;
SELECT id,filename FROM rckrb_carddav_migrations WHERE `filename` LIKE 'UNITTEST-SYNC%';
-- 58 is the ID of the record as returned by select. XYZ is replaced with a client-specific suffix (PAR or CLD).
UPDATE rckrb_carddav_migrations SET filename='UNITTEST-SYNC-XYZ' WHERE `id` = '58';
COMMIT

And here is the MySQL server log:

2 Connect   roundcube14@localhost on roundcubemail14krb using Socket 
2 Query INSERT INTO rckrb_carddav_migrations(filename) VALUES ('UNITTEST-SYNC')
2 Query SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE
2 Query START TRANSACTION READ WRITE
3 Connect   roundcube14@localhost on roundcubemail14krb using Socket 
3 Query SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE
3 Query START TRANSACTION READ WRITE
3 Query SELECT id,filename FROM rckrb_carddav_migrations WHERE `filename` LIKE 'UNITTEST-SYNC%'
2 Query SELECT id,filename FROM rckrb_carddav_migrations WHERE `filename` LIKE 'UNITTEST-SYNC%'
2 Query UPDATE rckrb_carddav_migrations SET filename='UNITTEST-SYNC-PAR' WHERE `id` = '58'
3 Query UPDATE rckrb_carddav_migrations SET filename='UNITTEST-SYNC-CLD' WHERE `id` = '58'
3 Query UPDATE rckrb_carddav_migrations SET filename='UNITTEST-SYNC-CLD' WHERE `id` = '58'
2 Query COMMIT
3 Query COMMIT
3 Quit  
2 Query SELECT * FROM rckrb_carddav_migrations WHERE `id` = '58'
2 Query DELETE FROM rckrb_carddav_migrations WHERE `filename` LIKE 'UNITTEST-SYNC%'
2 Quit 

Now as you can see, client 3 issues the UPDATE statement a second time. This is because the first update failed with a deadlock error, and roundcube reissues the update. Since the second update is executed without the scope of a transaction, it succeeds. However, the resulting data in the DB is inconsistent from the viewpoint of the application, for which it looks as if:

  • Both transactions succeed, value of filename is UNITTEST-SYNC-CLD.

Expected/acceptable behavior from viewpoint of application (with one transaction failing being what typically happens):

  • Both transactions fail (rolled back), value of filename stays at UNITTEST-SYNC.
  • Transaction of client 2 fails, value of filename is at UNITTEST-SYNC-CLD.
  • Transaction of client 3 fails, value of filename is at UNITTEST-SYNC-PAR.
  • Both transactions succeed, value of filename is either UNITTEST-SYNC-PAR-CLD or UNITTEST-SYNC-CLD-PAR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants