revision-id: b528b069a169d6e3d9ef4f9c7de68ead3e29186f parent(s): af1568668f6a910b5739fe1d7181c59f94a95196 committer: Sergei Petrunia branch nick: 10.2-r12-new-submodule timestamp: 2018-07-13 16:23:04 +0300 message: MDEV-16710, Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transaction rollback_to_stmt_savepoint() calls do_rollback_to_savepoint(). This removes the changes made by this statement, and also removes the set savepoint. Before we start processing the next statement, we need to set the new savepoint, so we will have something to rollback to if the next statement fails. Since rollback_to_stmt_savepoint always sets a new savepoint now, m_n_savepoints is now redundant and is removed. --- storage/rocksdb/ha_rocksdb.cc | 16 +++++++-------- .../mysql-test/rocksdb/r/transaction.result | 17 ++++++++++++++++ .../rocksdb/mysql-test/rocksdb/t/transaction.test | 23 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index bd2e0bf..25bab1d 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -1918,8 +1918,6 @@ class Rdb_transaction { bool m_is_two_phase = false; private: - /* Number of RockDB savepoints taken */ - int m_n_savepoints; /* Number of write operations this transaction had when we took the last savepoint (the idea is not to take another savepoint if we haven't made @@ -2493,7 +2491,6 @@ class Rdb_transaction { entire transaction. */ do_set_savepoint(); - m_n_savepoints= 1; m_writes_at_last_savepoint= m_write_count; } @@ -2510,7 +2507,6 @@ class Rdb_transaction { { do_set_savepoint(); m_writes_at_last_savepoint= m_write_count; - m_n_savepoints++; } } @@ -2521,10 +2517,14 @@ class Rdb_transaction { void rollback_to_stmt_savepoint() { if (m_writes_at_last_savepoint != m_write_count) { do_rollback_to_savepoint(); - if (!--m_n_savepoints) { - do_set_savepoint(); - m_n_savepoints= 1; - } + /* + RollbackToSavePoint "removes the most recent SetSavePoint()", so + we need to set it again so that next statement can roll back to this + stage. + It's ok to do it here at statement end (instead of doing it at next + statement start) because setting a savepoint is cheap. + */ + do_set_savepoint(); m_writes_at_last_savepoint= m_write_count; } } diff --git a/storage/rocksdb/mysql-test/rocksdb/r/transaction.result b/storage/rocksdb/mysql-test/rocksdb/r/transaction.result index 006baaf..7664a11 100644 --- a/storage/rocksdb/mysql-test/rocksdb/r/transaction.result +++ b/storage/rocksdb/mysql-test/rocksdb/r/transaction.result @@ -958,3 +958,20 @@ a rollback; drop function func; drop table t1,t2,t3; +# +# MDEV-16710: Slave SQL: Could not execute Update_rows_v1 event with RocksDB and triggers +# Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transaction +# +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=RocksDB; +INSERT INTO t1 VALUES (1); +CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=RocksDB; +CREATE TRIGGER tr AFTER INSERT ON t2 FOR EACH ROW INSERT INTO non_existing_table VALUES (NULL); +BEGIN; +DELETE FROM t1; +INSERT INTO t2 VALUES (1); +INSERT INTO t2 VALUES (2); +# Must return empty result: +SELECT * FROM t1; +a +COMMIT; +drop table t1,t2; diff --git a/storage/rocksdb/mysql-test/rocksdb/t/transaction.test b/storage/rocksdb/mysql-test/rocksdb/t/transaction.test index 3350db9..2503cbc 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/transaction.test +++ b/storage/rocksdb/mysql-test/rocksdb/t/transaction.test @@ -133,3 +133,26 @@ rollback; drop function func; drop table t1,t2,t3; +--echo # +--echo # MDEV-16710: Slave SQL: Could not execute Update_rows_v1 event with RocksDB and triggers +--echo # Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transaction +--echo # +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=RocksDB; +INSERT INTO t1 VALUES (1); + +CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=RocksDB; + +CREATE TRIGGER tr AFTER INSERT ON t2 FOR EACH ROW INSERT INTO non_existing_table VALUES (NULL); + +BEGIN; +DELETE FROM t1; +--error 0,ER_NO_SUCH_TABLE +INSERT INTO t2 VALUES (1); +--error 0,ER_NO_SUCH_TABLE +INSERT INTO t2 VALUES (2); +--echo # Must return empty result: +SELECT * FROM t1; +COMMIT; + +drop table t1,t2; +