Skip to content

box: fix transaction "read-view" and "conflicted" states#7252

Merged
locker merged 1 commit intotarantool:masterfrom
CuriousGeorgiy:memtx-fix-conflicted-txs
Jun 16, 2022
Merged

box: fix transaction "read-view" and "conflicted" states#7252
locker merged 1 commit intotarantool:masterfrom
CuriousGeorgiy:memtx-fix-conflicted-txs

Conversation

@CuriousGeorgiy
Copy link
Member

Currently, there is a fundamental logical inconsistency with read-view and
conflicted states of transactions.

Conflicted transactions see all prepared changes (e.g., #7238), because
they are handled differently than read-view ones. At the same time, one
does not know the state of the transaction until box.commit is called.

A similar problem arises with read-view transactions: if such transactions
do any DML statements, they are de-facto conflicted, but this will only be
determined at preparation stage:

tarantool/src/box/txn.c

Lines 1006 to 1013 in 7924557

/*
* Somebody else has written some value that we have read.
* The RW transaction is not possible.
*/
if (txn->status == TXN_IN_READ_VIEW && !stailq_empty(&txn->stmts)) {
diag_set(ClientError, ER_TRANSACTION_CONFLICT);
return -1;
}

Fix this inconsistency by the following changes:

  1. Conflict "read-view" transactions on attempt to perform DML statements
    immediately — guarantee this with an assertion at preparation stage.
  2. Make conflicted transactions unconditionally throw "Transaction has been
    aborted by conflict" error on any CRUD operations (including read-only
    ones) until they are either rolled back (which will return no error) or
    committed (which will return the same error).

Closes #7238
Closes #7239
Closes #7240

@CuriousGeorgiy CuriousGeorgiy added bug Something isn't working memtx teamC mvcc labels Jun 9, 2022
@CuriousGeorgiy CuriousGeorgiy added this to the 2.10.1 milestone Jun 9, 2022
@CuriousGeorgiy CuriousGeorgiy self-assigned this Jun 9, 2022
@CuriousGeorgiy CuriousGeorgiy changed the title box: rework transaction "read-view" and "conflicted" states box: fix transaction "read-view" and "conflicted" states Jun 9, 2022
@CuriousGeorgiy CuriousGeorgiy removed the request for review from alyapunov June 9, 2022 08:31
@CuriousGeorgiy CuriousGeorgiy force-pushed the memtx-fix-conflicted-txs branch from 202b091 to cfd1d03 Compare June 9, 2022 08:59
@CuriousGeorgiy CuriousGeorgiy force-pushed the memtx-fix-conflicted-txs branch from cfd1d03 to a129984 Compare June 9, 2022 09:30
@locker locker assigned CuriousGeorgiy and unassigned locker Jun 10, 2022
Currently, there is a fundamental logical inconsistency with read-view and
conflicted states of transactions.

Conflicted transactions see all prepared changes (e.g., tarantool#7238), because
they are handled differently than read-view ones. At the same time, one
does not know the state of the transaction until `box.commit` is called.

A similar problem arises with read-view transactions: if such transactions
do any DML statements, they are de-facto conflicted, but this will only be
determined at preparation stage:
https://github.com/tarantool/tarantool/blob/79245573dabf3c1eb4eb904fd80ee84270360476/src/box/txn.c#L1006-L1013

Fix this inconsistency by the following changes:
1. Conflict "read-view" transactions on attempt to perform DML statements
immediately — guarantee this with an assertion at preparation stage.
2. Make conflicted transactions unconditionally throw "Transaction has been
aborted by conflict" error on any CRUD operations (including read-only
ones) until they are either rolled back (which will return no error) or
committed (which will return the same error).

Closes tarantool#7238
Closes tarantool#7239
Closes tarantool#7240

@TarantoolBot document
Title: new  behaviour of "conflicted" transactions

"Conflicted" transactions now return "Transaction aborted by conflicted"
error on any CRUD operations (including read-only ones), until they are
either rolled back (which will return no error) or committed (which will
return the same error).
@CuriousGeorgiy CuriousGeorgiy force-pushed the memtx-fix-conflicted-txs branch from a129984 to 64b18c1 Compare June 15, 2022 13:24
@CuriousGeorgiy CuriousGeorgiy requested a review from locker June 15, 2022 13:42
@locker locker added the full-ci Enables all tests for a pull request label Jun 16, 2022
@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 84.857% when pulling 64b18c1 on CuriousGeorgiy:memtx-fix-conflicted-txs into b3f462b on tarantool:master.

@locker locker merged commit 4d52199 into tarantool:master Jun 16, 2022
@locker
Copy link
Member

locker commented Jun 16, 2022

@alyapunov @kyukhin Do we need to backport this fix to 2.10? This is a bug fix, but it introduces a potentially breaking change: conflicting transactions raise errors right on DML/DQL, not on commit, as they used to.

None of the tickets it closes has a milestone...

@locker
Copy link
Member

locker commented Jun 16, 2022

According to @alyapunov it's okay to backport this fix to 2.10. PMs don't mind. Backported to 2.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working full-ci Enables all tests for a pull request memtx mvcc vinyl

Projects

None yet

3 participants