Hi, Sergey! Here's a review of the last two commits in bb-10.2-mdev7660 (I've quickly looked through other, cleanup, commits, didn't have any comments, so I've excluded them from the diff to make it smaller)
diff --git a/mysql-test/t/innodb_mysql_lock.test b/mysql-test/t/innodb_mysql_lock.test index cb57c09..85ba418 100644 --- a/mysql-test/t/innodb_mysql_lock.test +++ b/mysql-test/t/innodb_mysql_lock.test @@ -150,14 +150,16 @@ let $wait_condition= --source include/wait_condition.inc LOCK TABLES t1 READ; SELECT release_lock('bug42147_lock'); +let $wait_condition= + SELECT COUNT(*) > 0 FROM information_schema.processlist + WHERE state = 'executing' + AND info = 'INSERT INTO t1 SELECT get_lock(\'bug42147_lock\', 60)'; +--source include/wait_condition.inc +UNLOCK TABLES;
1. What did this test do? 2. What does this 'executing' mean? Really executing or waiting inside InnoDB on a lock?
connection default; --reap
-connection con2; -UNLOCK TABLES; - -connection default; disconnect con2; DROP TABLE t1;
diff --git a/mysql-test/t/lock_sync.test b/mysql-test/t/lock_sync.test index c090e3a..07c16ac 100644 --- a/mysql-test/t/lock_sync.test +++ b/mysql-test/t/lock_sync.test @@ -873,116 +879,6 @@ set @@global.concurrent_insert= @old_concurrent_insert;
--echo # ---echo # Test for bug #45143 "All connections hang on concurrent ALTER TABLE". ---echo # ---echo # Concurrent execution of statements which required weak write lock ---echo # (TL_WRITE_ALLOW_WRITE) on several instances of the same table and ---echo # statements which tried to acquire stronger write lock (TL_WRITE, ---echo # TL_WRITE_ALLOW_READ) on this table might have led to deadlock.
Is this test no longer applicable? Why?
-# -# Suppress warnings for INSERTs that use get_lock(). -# -disable_query_log; -call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); -enable_query_log; - ---disable_warnings -drop table if exists t1; -drop view if exists v1; ---enable_warnings ---echo # Create auxiliary connections used through the test. -connect (con_bug45143_1,localhost,root,,test,,); -connect (con_bug45143_3,localhost,root,,test,,); -connect (con_bug45143_2,localhost,root,,test,,); -connection default; ---echo # Reset DEBUG_SYNC facility before using it. -set debug_sync= 'RESET'; ---echo # Turn off logging so calls to locking subsystem performed ---echo # for general_log table won't interfere with our test. -set @old_general_log = @@global.general_log; -set @@global.general_log= OFF; - -create table t1 (i int) engine=InnoDB; ---echo # We have to use view in order to make LOCK TABLES avoid ---echo # acquiring SNRW metadata lock on table. -create view v1 as select * from t1; -insert into t1 values (1); ---echo # Prepare user lock which will be used for resuming execution of ---echo # the first statement after it acquires TL_WRITE_ALLOW_WRITE lock. -select get_lock("lock_bug45143_wait", 0); - -connection con_bug45143_1; ---echo # Sending: ---send insert into t1 values (get_lock("lock_bug45143_wait", 100)); - -connection con_bug45143_2; ---echo # Wait until the above INSERT takes TL_WRITE_ALLOW_WRITE lock on 't1' ---echo # and then gets blocked on user lock 'lock_bug45143_wait'. -let $wait_condition= select count(*)= 1 from information_schema.processlist - where state= 'User lock' and - info='insert into t1 values (get_lock("lock_bug45143_wait", 100))'; ---source include/wait_condition.inc ---echo # Ensure that upcoming SELECT waits after acquiring TL_WRITE_ALLOW_WRITE ---echo # lock for the first instance of 't1'. -set debug_sync='thr_multi_lock_after_thr_lock SIGNAL parked WAIT_FOR go'; ---echo # Sending: ---send select count(*) > 0 from t1 as a, t1 as b for update; - -connection con_bug45143_3; ---echo # Wait until the above SELECT ... FOR UPDATE is blocked after ---echo # acquiring lock for the the first instance of 't1'. -set debug_sync= 'now WAIT_FOR parked'; ---echo # Send LOCK TABLE statement which will try to get TL_WRITE lock on 't1': ---send lock table v1 write; - -connection default; ---echo # Wait until this LOCK TABLES statement starts waiting for table lock. -let $wait_condition= select count(*)= 1 from information_schema.processlist - where state= 'Waiting for table level lock' and - info='lock table v1 write'; ---source include/wait_condition.inc ---echo # Allow SELECT ... FOR UPDATE to resume. ---echo # Since it already has TL_WRITE_ALLOW_WRITE lock on the first instance ---echo # of 't1' it should be able to get lock on the second instance without ---echo # waiting, even although there is another thread which has such lock ---echo # on this table and also there is a thread waiting for a TL_WRITE on it. -set debug_sync= 'now SIGNAL go'; - -connection con_bug45143_2; ---echo # Reap SELECT ... FOR UPDATE ---reap - -connection default; ---echo # Resume execution of the INSERT statement. -select release_lock("lock_bug45143_wait"); - -connection con_bug45143_1; ---echo # Reap INSERT statement. ---echo # In Statement and Mixed replication mode we get here "Unsafe ---echo # for binlog" warnings. In row mode there are no warnings. ---echo # Hide the discrepancy. ---disable_warnings ---reap ---enable_warnings - - -connection con_bug45143_3; ---echo # Reap LOCK TABLES statement. ---reap -unlock tables; - -connection default; ---echo # Do clean-up. -disconnect con_bug45143_1; -disconnect con_bug45143_2; -disconnect con_bug45143_3; -set debug_sync= 'RESET'; -set @@global.general_log= @old_general_log; -drop view v1; -drop table t1; - - ---echo # --echo # Bug#50821 Deadlock between LOCK TABLES and ALTER TABLE --echo #
@@ -1051,55 +947,6 @@ SET DEBUG_SYNC="RESET";
--echo # ---echo # Bug#55930 Assertion `thd->transaction.stmt.is_empty() || ---echo # thd->in_sub_stmt || (thd->state.. ---echo #
what about this one?
---disable_warnings -DROP TABLE IF EXISTS t1; ---enable_warnings - -CREATE TABLE t1(a INT) engine=InnoDB; -INSERT INTO t1 VALUES (1), (2); - -connect (con1, localhost, root); -connect (con2, localhost, root); - -connection con1; -SET SESSION lock_wait_timeout= 1; -SET DEBUG_SYNC= 'ha_admin_open_ltable SIGNAL opti_recreate WAIT_FOR opti_analyze'; ---echo # Sending: ---send OPTIMIZE TABLE t1 - -connection con2; -SET DEBUG_SYNC= 'now WAIT_FOR opti_recreate'; -SET DEBUG_SYNC= 'after_lock_tables_takes_lock SIGNAL thrlock WAIT_FOR release_thrlock'; ---echo # Sending: ---send INSERT INTO t1 VALUES (3) - -connection default; -SET DEBUG_SYNC= 'now WAIT_FOR thrlock'; -SET DEBUG_SYNC= 'now SIGNAL opti_analyze'; - -connection con1; ---echo # Reaping: OPTIMIZE TABLE t1 ---reap -SET DEBUG_SYNC= 'now SIGNAL release_thrlock'; -disconnect con1; ---source include/wait_until_disconnected.inc - -connection con2; ---echo # Reaping: INSERT INTO t1 VALUES (3) ---reap -disconnect con2; ---source include/wait_until_disconnected.inc - -connection default; -DROP TABLE t1; -SET DEBUG_SYNC= 'RESET'; - - ---echo # --echo # Bug#57130 crash in Item_field::print during SHOW CREATE TABLE or VIEW --echo #
diff --git a/mysql-test/r/lock_tables_lost_commit.result b/mysql-test/r/lock_tables_lost_commit.result index 769e973..394ef0a 100644 --- a/mysql-test/r/lock_tables_lost_commit.result +++ b/mysql-test/r/lock_tables_lost_commit.result @@ -9,7 +9,6 @@ disconnect con1; connection con2; SELECT * FROM t1; a -10
This test doesn't make sense after your fix. and the original bug#578 reappears. The problem is that mysqlimport issues LOCK TABLES, but does not issue UNLOCK TABLES at the end. I suspect this has to be fixed now. Alternatively, you can make LOCK TABLE to commit (not rollback) a transaction at disconnect. But that would be weird, wouldn't it?
DROP TABLE t1; connection default; disconnect con2; diff --git a/mysql-test/r/partition_explicit_prune.result b/mysql-test/r/partition_explicit_prune.result index 765803d..7b9c53d 100644 --- a/mysql-test/r/partition_explicit_prune.result +++ b/mysql-test/r/partition_explicit_prune.result @@ -281,7 +281,7 @@ UNLOCK TABLES; SELECT * FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME LIKE 'HANDLER_%' AND VARIABLE_VALUE > 0; VARIABLE_NAME VARIABLE_VALUE -HANDLER_COMMIT 2 +HANDLER_COMMIT 3
is this expected?
HANDLER_READ_RND_NEXT 52 HANDLER_TMP_WRITE 72 HANDLER_WRITE 2 diff --git a/mysql-test/suite/handler/handler.inc b/mysql-test/suite/handler/handler.inc index b1e881f..8cad6a5 100644 --- a/mysql-test/suite/handler/handler.inc +++ b/mysql-test/suite/handler/handler.inc @@ -1091,6 +1091,12 @@ connection default; --reap drop table t2;
+# This test expects "handler t1 read a next" to get blocked on table level +# lock so that further "drop table t1" can break the lock and close handler. +# This notification mechanism doesn't work with InnoDB since it bypasses +# table level locks.
This is interesting. The test does begin; handler open; commit; handler read next; if logically the handler is just manually doing select-like access, it should not keep the table open over the transaction boundaries. how does it work now?
+if ($engine_type != 'InnoDB') +{ --echo # --echo # Bug #46224 HANDLER statements within a transaction might --echo # lead to deadlocks diff --git a/mysql-test/suite/handler/innodb.result b/mysql-test/suite/handler/innodb.result index fc1089e..9f2a0dd 100644 --- a/mysql-test/suite/handler/innodb.result +++ b/mysql-test/suite/handler/innodb.result @@ -545,7 +545,6 @@ optimize table t1; connection default; handler t1 read next; c1 -1
why is that? is optimize blocked now?
handler t1 close; connection con2; Table Op Msg_type Msg_text diff --git a/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test b/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test index 575fdb2..e5e2f7a 100644 --- a/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test +++ b/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test @@ -524,6 +524,7 @@ CREATE TABLE t11 (song VARCHAR(255)); LOCK TABLES t11 WRITE; SET SESSION BINLOG_FORMAT=ROW; INSERT INTO t11 VALUES('Several Species of Small Furry Animals Gathered Together in a Cave and Grooving With a Pict'); +--error ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_BINLOG_FORMAT
even with myisam tables?
SET SESSION BINLOG_FORMAT=STATEMENT; INSERT INTO t11 VALUES('Careful With That Axe, Eugene'); UNLOCK TABLES; diff --git a/storage/tokudb/mysql-test/tokudb_bugs/t/db806.test b/storage/tokudb/mysql-test/tokudb_bugs/t/db806.test index 3815e59..8dcebe1 100644 --- a/storage/tokudb/mysql-test/tokudb_bugs/t/db806.test +++ b/storage/tokudb/mysql-test/tokudb_bugs/t/db806.test @@ -7,7 +7,7 @@ enable_warnings; CREATE TABLE t3(a int,c int,d int)engine=TOKUDB; lock table t3 read; create temporary table t1 engine=tokudb as SELECT 1; -select * from t1; unlock tables; +select * from t1;
why?
-drop table t1,t3; +drop table t1,t3; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 3091bd6..b7c2e4a 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2824,6 +2824,11 @@ Locked_tables_list::unlock_locked_tables(THD *thd) request for metadata locks and TABLE_LIST elements. */ reset(); + if (thd->variables.option_bits & OPTION_AUTOCOMMIT) + { + thd->variables.option_bits&= ~(OPTION_NOT_AUTOCOMMIT); + thd->server_status|= SERVER_STATUS_AUTOCOMMIT; + }
Okay, you restore autocommit. Where a transaction is actually committed?
}
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 150f21a..b1dd81b 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -3267,17 +3267,6 @@ ha_innobase::init_table_handle_for_HANDLER(void)
innobase_register_trx(ht, user_thd, prebuilt->trx);
- /* We did the necessary inits in this function, no need to repeat them - in row_search_for_mysql */ - - prebuilt->sql_stat_start = FALSE; - - /* We let HANDLER always to do the reads as consistent reads, even - if the trx isolation level would have been specified as SERIALIZABLE */ - - prebuilt->select_lock_type = LOCK_NONE; - prebuilt->stored_select_lock_type = LOCK_NONE; -
why this change? (removing LOCK_NONE, moving sql_stat_start=FALSE to start_stmt)
/* Always fetch all columns in the index record */
prebuilt->hint_need_to_fetch_extra_cols = ROW_RETRIEVE_ALL_COLS;
Regards, Sergei Chief Architect MariaDB and security@mariadb.org