![](https://secure.gravatar.com/avatar/32b3f5fe52f3e988b4cd6460e7f68d91.jpg?s=120&d=mm&r=g)
1 Jun
2016
1 Jun
'16
12:47 p.m.
Hi Sergei, On Wed, Jun 01, 2016 at 08:39:36AM +0200, Sergei Golubchik wrote: > Hi, Sergey! > > On Jun 01, Sergey Vojtovich wrote: > > On Tue, May 31, 2016 at 06:48:31PM +0200, Sergei Golubchik wrote: > > > > > > I think it's fine. I had a few questions though, see below: > > Thanks for positive feedback. As I mentioned before, I'm less > > optimistic about this patch due to many behavior changes. I can try to > > make a list if you like. > > Yes, please! - InnoDB lock wait timeouts are now honored which are much shorter by default than server lock wait timeouts (1 year vs 50 seconds) - with @@autocommit= 1 LOCK TABLES disables autocommit implicitely, though user still sees @@autocommt= 1 - the above starts implicit transaction - transactions started by LOCK TABLES are now rolled back on disconnect (previously everything was committed due to autocommit) - transactions started by LOCK TABLES are now rolled back by ROLLBACK (previously everything was committed due to autocommit) - it is now impossible to change BINLOG_FORMAT under LOCK TABLES (at least to statement) due to running transaction - LOCK TABLES WRITE is additionally handled by MDL, except for HANDLER READ (which causes deadlock) - the above makes impossible to break HANDLER READ lock - ...in contrast LOCK TABLES READ protection against DML is pure InnoDB - combining transactional and non-transactional tables under LOCK TABLES may cause rolled back changes in transactional table and "committed" changes in non-transactional table - user may disable innodb_table_locks, which will cause LOCK TABLES to be noop basically There're likely more minor behavior changes that I missed. > > > > commit 5645626 > > > > Author: Sergey Vojtovich> > > > Date: Tue May 24 12:25:56 2016 +0400 > > > > > > > > MDEV-7660 - MySQL WL#6671 "Improve scalability by not using thr_lock.c locks > > > > for InnoDB tables" > > > > > > > > - InnoDB now acquires shared lock for HANDLER ... READ > > > > > > Why? > > It is about change in ha_innobase::init_table_handle_for_HANDLER(). > > To isolate HANDLER READ from LOCK TABLES WRITE. > > But should it be done? SELECT works without locks, why should HANDLER be > different? > On the other hand, I don't have strong preference for either case. > Do whatever you think is better. LOCK TABLES WRITE acquires MDL_SHARED_NO_READ_WRITE SELECT acquires MDL_SHARED read (incompatible with MDL_SHARED_NO_READ_WRITE) HANDLER READ acquires no MDL lock In other words SELECT is isolated from LOCK TABLES WRITE on MDL level, while HANDLER READ was isolated on THR_LOCK level. Now HANDLER READ is isolated on InnoDB level. > > > > 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; > > > > > > I don't understand the original test case. But after your changes it > > > actually makes sense :) > > Previously "INSERT INTO t1 SELECT get_lock('bug42147_lock', 60)" > > was able to complete while concurrent thread was holding "LOCK TABLES t1 READ". > > Exactly! This didn't make much sense, it actually looked like a bug. > > > Now it's blocked and I had to move "UNLOCK TABLES" before reap. > > Which is good. I supposed there's a bug in THR_LOCK still. > > > > > 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. > > > > > > what happens for InnoDB then? > > > I'd expect "handler t1 read a next" to get blocked inside the InnoDB. > > Yes, that's exactly what happens. > > > > > what does then "drop table t1" do? > > It gets blocked too, because it can't abort InnoDB locks. > > Is drop blocked by MDL? > > Is it a deadlock then? Yes. :( DROP TABLE is waiting for open handler to get closed, while HANDLER READ is waiting for LOCK TABLES. Below is simplified test that gets stuck on "drop table": --source include/have_xtradb.inc create table t1 (a int, key using btree (a)) engine=InnoDB; lock tables t1 write; connect(con1, localhost, root); handler t1 open; --send handler t1 read a next connection default; --sleep 1 drop table t1; Regards, Sergey