List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:June 16 2011 12:47pm
Subject:Re: bzr commit into mysql-5.5 branch (Dmitry.Lenev:3441) Bug#12641342
View as plain text  
Hi Dmitry,

On 6/13/11 5:43 PM, Dmitry Lenev wrote:
> #At file:///home/dlenev/src/bzr/mysql-5.5-12641342/ based on
> revid:sunanda.menon@stripped
> 
>  3441 Dmitry Lenev	2011-06-14
>       Fix for bug #12641342 - "61401: UPDATE PERFORMANCE DEGRADES
>       GRADUALLY IF A TRIGGER EXISTS".
>       
>       This bug manifested itself in two ways:
>       
>       - Firstly execution of any data-changing statement which
>         required prelocking (i.e. involved stored function or
>         trigger) as part of transaction slowed down a bit all
>         subsequent statements in this transaction. So performance
>         in transaction which periodically involved such statements
>         gradually degraded over time.
>       - Secondly execution of any data-changing statement which
>         required prelocking as part of transaction prevented
>         concurrent FLUSH TABLES WITH READ LOCK from proceeding
>         until the end of transaction instead of end of particular
>         statement.
>         
>       The problem was caused by incorrect handling of metadata lock
>       used in FTWRL implementation for statements requiring prelocked 
>       mode. 
>       Each statement which changes data acquires global IX lock
>       with STATEMENT duration. This lock is supposed to block 
>       concurrent FTWRL from proceeding until the statement ends. 

Add new line.

>       When entering prelocked mode duration of all metadata locks

"prelocked mode duration of all metadata" is a bit hard to comprehend.

>       acquired so far was changed to EXPLICIT, to prevent 
>       substatements from releasing these locks. When prelocked mode
>       was left duration of metadata lock was changed to TRANSACTIONAL

"prelocked was left duration"

Missing comma after after "left"?

"when prelocked mode was left, the duration of metadata lock"

>       (with a few exceptions) so they can be properly released at
>       the end of transaction. 
>       Unfortunately, this meant that global IX lock blocking FTWRL
>       with STATEMENT duration was moved to TRANSACTIONAL duration 
>       after execution of statement requiring prelocking. As result
>       concurrent FTWRL was blocked until the end of transaction
>       instead of end of statement in such a situation.

That's one of the consequences, but not the one detailed in the bug report.

>       Moreover, since each subsequent statement that required 
>       prelocking and tried to acquire global IX lock with STATEMENT 
>       duration got a new instance of MDL_ticket, which was later
>       moved to TRANSACTIONAL duration, this also led to unwarranted
>       growth of number of tickets with TRANSACITONAL duration in
>       this MDL_context. As result searching for other tickets in
>       it became slow and acquisition of other metadata locks by this
>       transaction started to hog CPU.

That's the issue in the bug report. Could you highlight this first? and
later explain the other consequence.

As discussed on IRC, also please document "explicit duration" locks.

> 
> === modified file 'mysql-test/t/flush.test'
> --- a/mysql-test/t/flush.test	2011-03-07 09:08:10 +0000
> +++ b/mysql-test/t/flush.test	2011-06-13 20:43:29 +0000
> @@ -668,3 +668,36 @@ ALTER TABLE t1 COMMENT 'test';
>  
>  UNLOCK TABLES;
>  DROP TABLE t1;
> +
> +
> +--echo #
> +--echo # Test for bug #12641342 - "61401: UPDATE PERFORMANCE DEGRADES
> +--echo #                           GRADUALLY IF A TRIGGER EXISTS".
> +--echo #
> +--echo # One of side-effects of this bug was that a transaction which
> +--echo # involved DML statements requiring prelocking blocked concurrent
> +--echo # FLUSH TABLES WITH READ LOCK for the whole its duration, while
> +--echo # correct behavior in this case is to block FTWRL only for duration
> +--echo # of individual DML statement.

statements.

> +--disable_warnings
> +DROP TABLE IF EXISTS t1;
> +--enable_warnings
> +CREATE TABLE t1 (id INT PRIMARY KEY, value INT);
> +INSERT INTO t1 VALUES (1, 1);
> +CREATE TRIGGER t1_au AFTER UPDATE ON t1 FOR EACH ROW SET @var = "a";
> +BEGIN;
> +UPDATE t1 SET value= value + 1 WHERE id = 1;
> +
> +--echo # Switching to connection 'con1'.
> +connect(con1, localhost, root);
> +--echo # The below FLUSH TABLES WITH READ LOCK should succeed and
> +--echo # should not be blocked by the transaction in default connection.
> +FLUSH TABLES WITH READ LOCK;
> +UNLOCK TABLES;
> +disconnect con1;
> +--source include/wait_until_disconnected.inc
> +
> +--echo # Switching to connection 'default'.
> +connection default;
> +COMMIT;
> +DROP TABLE t1;
> 
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc	2011-05-21 09:29:10 +0000
> +++ b/sql/sql_class.cc	2011-06-13 20:43:29 +0000
> @@ -3790,16 +3790,25 @@ void THD::set_mysys_var(struct st_my_thr
>  
>  void THD::leave_locked_tables_mode()
>  {
> +  if (locked_tables_mode == LTM_LOCK_TABLES)
> +  {
> +    /*
> +      When leaving LOCK TABLES mode we should change duration for most

Suggest to rephrase to something like: "... mode we have to change the
duration of most of the metadata locks being held, except for HANDLER
and GRL locks, to transactional for them to be properly released at
UNLOCK TABLES."

Otherwise, looks good. OK to push.

Regards,

Davi
Thread
bzr commit into mysql-5.5 branch (Dmitry.Lenev:3441) Bug#12641342Dmitry Lenev14 Jun
  • Re: bzr commit into mysql-5.5 branch (Dmitry.Lenev:3441) Bug#12641342Jon Olav Hauglid16 Jun
  • Re: bzr commit into mysql-5.5 branch (Dmitry.Lenev:3441) Bug#12641342Davi Arnaut16 Jun