revision-id: b8e75a29dd371913772a7eaaddb56430c9e8ac8e parent(s): a47797f16671a44a92632935fc93e302e0c969ff committer: Sergei Petrunia branch nick: mysql-5.6-rocksdb-spetrunia timestamp: 2018-07-18 12:26:31 +0300 message: 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 last 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. Squash with D7509380 --- mysql-test/suite/rocksdb/r/transaction.result | 17 +++++++++++++++++ mysql-test/suite/rocksdb/t/transaction.test | 23 +++++++++++++++++++++++ storage/rocksdb/ha_rocksdb.cc | 16 ++++++++-------- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/transaction.result b/mysql-test/suite/rocksdb/r/transaction.result index 006baaf..8a5825b 100644 --- a/mysql-test/suite/rocksdb/r/transaction.result +++ b/mysql-test/suite/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 multiple 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/mysql-test/suite/rocksdb/t/transaction.test b/mysql-test/suite/rocksdb/t/transaction.test index 3350db9..129484b 100644 --- a/mysql-test/suite/rocksdb/t/transaction.test +++ b/mysql-test/suite/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 multiple 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; + diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 57eaa83..a71a5b3 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -1875,8 +1875,6 @@ protected: 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 @@ -2436,7 +2434,6 @@ public: entire transaction. */ do_set_savepoint(); - m_n_savepoints= 1; m_writes_at_last_savepoint= m_write_count; } @@ -2453,7 +2450,6 @@ public: { do_set_savepoint(); m_writes_at_last_savepoint= m_write_count; - m_n_savepoints++; } } @@ -2464,10 +2460,14 @@ public: 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; } }