List:Commits« Previous MessageNext Message »
From:Konstantin Osipov Date:July 19 2010 9:20am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (davi:3478) Bug#54453
View as plain text  
* Davi Arnaut <Davi.Arnaut@stripped> [10/07/13 19:30]:
>  3478 Davi Arnaut	2010-07-13
>       Bug#54453: Failing assertion: trx->active_trans when renaming a
>                  table with active trx

The patch is approved. Please see below.

>       
>       Essentially, the problem is that InnoDB does a implicit commit
>       when a cursor (table handler) is unlocked/closed, creating
>       a dissonance between the transaction state within the server
>       layer and the storage engine layer. Theoretically, a statement
>       transaction can encompass several table instances in a similar
>       manner to a multiple statement transaction, hence it does not
>       make sense to limit a statement transaction to the lifetime of
>       the table instances (cursors) used within it.
>       
>       Since this particular instance of the problem is only triggerable
>       on 5.1 and is masked on 5.5 due to metadata locks, the solution
>       (which is less risky) is to explicitly end the transaction before
>       the cached table is unlock on rename table.
>       
>       The patch is to be null merged into trunk and the bug left open
>       to address the underlying problem in later versions.

I think our analysis that you describe here is not fully correct,
since to repeat the problem one needs a transaction started in
another connection.
I, therefore, don't think any fix is needed in 5.5.
We no longer need to require the engine to correctly abort
transactions in *all* connections upon DDL. This is no longer a
pressing issue, at least. 
Let's discuss on IRC.

Is there a test that involves a single connection only?
If there is, it is something to fix in 5.5.


>      @ mysql-test/include/commit.inc
>         Fix counters, transaction is explicitly ended now.
>      @ mysql-test/suite/innodb_plugin/r/innodb_bug54453.result
>         Add test case result for Bug#54453
>      @ mysql-test/suite/innodb_plugin/t/innodb_bug54453.test
>         Add test case for Bug#54453
>      @ sql/sql_table.cc
>         End transaction as otherwise InnoDB will end it behind our backs.
> 
>     added:
>       mysql-test/suite/innodb_plugin/r/innodb_bug54453.result
>       mysql-test/suite/innodb_plugin/t/innodb_bug54453.test
>     modified:
>       mysql-test/include/commit.inc
>       mysql-test/r/commit_1innodb.result
>       sql/sql_table.cc
> === modified file 'mysql-test/include/commit.inc'
> --- a/mysql-test/include/commit.inc	2009-08-26 23:13:03 +0000
> +++ b/mysql-test/include/commit.inc	2010-07-13 14:56:24 +0000
> @@ -725,9 +725,9 @@ call p_verify_status_increment(4, 4, 4,
>  alter table t3 add column (b int);
>  call p_verify_status_increment(2, 0, 2, 0);
>  alter table t3 rename t4;
> -call p_verify_status_increment(2, 2, 2, 2);
> +call p_verify_status_increment(1, 0, 1, 0);
>  rename table t4 to t3;
> -call p_verify_status_increment(2, 2, 2, 2);
> +call p_verify_status_increment(1, 0, 1, 0);
>  truncate table t3;
>  call p_verify_status_increment(4, 4, 4, 4);
>  create view v1 as select * from t2;
> 
> === modified file 'mysql-test/r/commit_1innodb.result'
> --- a/mysql-test/r/commit_1innodb.result	2009-08-26 23:13:03 +0000
> +++ b/mysql-test/r/commit_1innodb.result	2010-07-13 14:56:24 +0000
> @@ -841,11 +841,11 @@ call p_verify_status_increment(2, 0, 2,
>  SUCCESS
>  
>  alter table t3 rename t4;
> -call p_verify_status_increment(2, 2, 2, 2);
> +call p_verify_status_increment(1, 0, 1, 0);
>  SUCCESS

Why is it that we have fewer engine calls even though you added
a commit? Please explain in the changeset comment.

>  rename table t4 to t3;
> -call p_verify_status_increment(2, 2, 2, 2);
> +call p_verify_status_increment(1, 0, 1, 0);
>  SUCCESS
>  
>  truncate table t3;
> 
> === added file 'mysql-test/suite/innodb_plugin/r/innodb_bug54453.result'
> --- a/mysql-test/suite/innodb_plugin/r/innodb_bug54453.result	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/innodb_plugin/r/innodb_bug54453.result	2010-07-13 14:56:24
> +0000
> @@ -0,0 +1,12 @@
> +#
> +# Bug#54453: Failing assertion: trx->active_trans when renaming a table with
> active trx
> +#
> +DROP TABLE IF EXISTS bug54453;
> +CREATE TABLE bug54453(a INT) ENGINE=InnoDB;
> +BEGIN;
> +INSERT INTO bug54453 VALUES (1);
> +ALTER TABLE bug54453 RENAME TO bug54453_2;
> +ROLLBACK;
> +SELECT * FROM bug54453_2;
> +a
> +DROP TABLE bug54453_2;
> 
> === added file 'mysql-test/suite/innodb_plugin/t/innodb_bug54453.test'
> --- a/mysql-test/suite/innodb_plugin/t/innodb_bug54453.test	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/innodb_plugin/t/innodb_bug54453.test	2010-07-13 14:56:24
> +0000
> @@ -0,0 +1,23 @@
> +--source include/have_innodb_plugin.inc
> +--source include/have_log_bin.inc
> +
> +--echo #
> +--echo # Bug#54453: Failing assertion: trx->active_trans when renaming a table
> with active trx
> +--echo #
> +
> +--disable_warnings
> +DROP TABLE IF EXISTS bug54453;
> +--enable_warnings
> +
> +CREATE TABLE bug54453(a INT) ENGINE=InnoDB;
> +--connect(con1,localhost,root,,)
> +BEGIN;
> +INSERT INTO bug54453 VALUES (1);
> +--connection default

Please add --echo # --> connection default (or connection con1)
to the test to make the result file more readable.

> +ALTER TABLE bug54453 RENAME TO bug54453_2;
> +--connection con1
> +ROLLBACK;
> +--disconnect con1
> +--connection default
> +SELECT * FROM bug54453_2;
> +DROP TABLE bug54453_2;
> 
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc	2010-06-10 20:45:22 +0000
> +++ b/sql/sql_table.cc	2010-07-13 14:56:24 +0000
> @@ -6848,6 +6848,14 @@ view_err:
>      if (!error && (new_name != table_name || new_db != db))
>      {
>        thd_proc_info(thd, "rename");
> +
> +      /*
> +        Workaround InnoDB ending the transaction when the table instance
> +        is unlocked/closed (close_cached_table below), otherwise the trx
> +        state will differ between the server and storage engine layers.
> +      */
> +      ha_autocommit_or_rollback(thd, 0);

Would it help if we put an implicit commit before ALTER starts
instead, like is already done in 5.5? Could it be that the problem
is not repeatable in 5.5 because there we have
CF_CAUSES_IMPLICIT_COMMMIT sql command flag?

-- 
Thread
bzr commit into mysql-5.1-bugteam branch (davi:3478) Bug#54453Davi Arnaut13 Jul
Re: bzr commit into mysql-5.1-bugteam branch (davi:3478) Bug#54453Konstantin Osipov19 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (davi:3478) Bug#54453Davi Arnaut19 Jul