Bug #55027 assertion: mutex_own(&dict_sys->mutex) in dict_table_get_on_id()

Description:
this is builtin only bug which started to happen after merging the fix
to bug#54583. It has a very good reproducibility with 5.1 builtin
but not with any version of the plugin with the same fix to bug#54583 present.
The stack backtrace would look like

 #5  0x0000003acbc31d10 in abort () from /lib64/libc.so.6
 #6  0x000000000080fe10 in dict_table_get_on_id (table_id=...,
     trx=0x2aaaac07f0b8) at dict/dict0dict.c:625
 #7  0x00000000008b5c3d in row_undo_mod_parse_undo_rec (node=0x2aaaac0868b8,
     thr=0x2aaaac06c1b0) at row/row0umod.c:672
 #8  0x00000000008b5e36 in row_undo_mod (node=0x2aaaac0868b8,
     thr=0x2aaaac06c1b0) at row/row0umod.c:721
 #9  0x000000000087b79b in row_undo (node=0x2aaaac0868b8, thr=0x2aaaac06c1b0)
     at row/row0undo.c:285
 #10 0x000000000087b892 in row_undo_step (thr=0x2aaaac06c1b0)
     at row/row0undo.c:327
 #11 0x000000000085de15 in que_thr_step (thr=0x2aaaac06c1b0)
     at que/que0que.c:1254
 #12 0x000000000085e017 in que_run_threads_low (thr=0x2aaaac06c1b0)
     at que/que0que.c:1319
 #13 0x000000000085e122 in que_run_threads (thr=0x2aaaac05fdb0)
     at que/que0que.c:1355
 #14 0x000000000089299b in trx_rollback_or_clean_all_without_sess (
     arg=0x7fff7d8ee38c) at trx/trx0roll.c:538

and the table is a system table, SYS_COULMNS iirc

How to repeat:
reproducibility looks good in my standard stress/recovery testing.

Suggested fix:
not sure. This seems to be the same assert that made Marko introduce
X dictionary locking in row_undo. Temporarily removing the fix to bug#54583
is one option.

the bug I meant was bug#54538, not bug#54583. The 5.1 commit message
wrongly mentioned 54583.

The bug exists in both the builtin and the plugin  introduced by the fix
for bug# 54538. What has been confirmed now is that rename table
does not play by the rules. It does not set the dict_operation flag in
the UNDO record. Subsequently during recovery it will croak on
the failing assertion.

The recommended way to proceed then is:

  1. Revert the fix for bug# 54538
  2. Fix the rename table bug -- the actual underlying problem
  3. Reapply the fix for bug# 54538

It is trivial to reproduce this bug using GDB, break at row_rename_table_for_mysql(),
let it execute the "RENAME_TABLE" SQL procedure then sync the log files to disk and
then kill the server. On restart it will fail on the assertion.

In the MySQL client type:

 CREATE TABLE T(C INT) ENGINE=INNODB;
 RENAME TABLE T TO T1;

The real fix should really be to get rid of this check in dict_table_get_on_id():

  if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0

I don't see why it is required, either the data dictionary is X locked or it
isn't is all that should matter. If the caller already owns the dict_mutex but
hasn't X locked the data dictionary then the caller should call the alternative
dict_table_get_on_id_low() function.

setting back to verified. Question that may arise:
1. Why don't we see the bug in the plugin? One possibility is that RENAME
would have a shorter codepath, or maybe it's already calling 
dict_table_get_on_id_low(). Or maybe even RENAME does set dict_operation
flag there
2. Shouldn't RENAME set that flag still, irrespective of what we do about
dict_table_get_on_id() calls? Plugin currently rolls back all dictionary
transaction first, then proceeds to other transactions, part of some fixes Marko implemented, so if RENAME
escapes it, it could create problems.

It is easily reproducible in the plugin too using GDB. One of my
previous comments mentions howto. Marko introduced the following
in the plugin:

/** Type of data dictionary operation */
typedef enum trx_dict_op {
        /** The transaction is not modifying the data dictionary. */
        TRX_DICT_OP_NONE = 0,
        /** The transaction is creating a table or an index, or
        dropping a table.  The table must be dropped in crash
        recovery.  This and TRX_DICT_OP_NONE are the only possible
        operation modes in crash recovery. */
        TRX_DICT_OP_TABLE = 1,
        /** The transaction is creating or dropping an index in an
        existing table.  In crash recovery, the data dictionary
        must be locked, but the table must not be dropped. */
        TRX_DICT_OP_INDEX = 2
} trx_dict_op_t;

It doesn't capture the case of rename table. It is essentially targeted at dropping
temporary tables and indexes only during startup.

True, I introduced TRX_DICT_OP_INDEX in the InnoDB Plugin 1.0, for the crash recovery of Fast Index Creation aka Smart ALTER TABLE. Before that, trx->dict_operation was a Boolean flag, which told InnoDB to drop the table during crash recovery, when rolling back an incomplete transaction that was executing CREATE TABLE or a table-copying DDL operation.

We should decide how to handle RENAME TABLE here. Possible fixes include relaxing the assertion or setting the dict_operation flag to an appropriate value.

OK to push the fix:

-       if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0
-           || trx->dict_operation_lock_mode == RW_X_LATCH) {
+       if (trx->dict_operation_lock_mode == RW_X_LATCH) {

Pushed into mysql-5.1 5.1.52 (revid:sunanda.menon@sun.com-20100928083322-wangbv97uobu7g66) (version source revid:sunanda.menon@sun.com-20100928083322-wangbv97uobu7g66) (merge vers: 5.1.52) (pib:21)

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@sun.com-20100928153607-tdsxkdm5cmuym5sq) (version source revid:alik@sun.com-20100928153508-0saa6v93dinqx1u7) (merge vers: 5.6.1-m4) (pib:21)

Pushed into mysql-next-mr (revid:alik@sun.com-20100928153646-pqp8o1a92mxtuj3h) (version source revid:alik@sun.com-20100928153532-lr3gtvnyp2en4y75) (pib:21)

Pushed into mysql-5.5 5.5.7-rc (revid:alik@sun.com-20100928153459-4nudf4zgzlou4s7q) (version source revid:alik@sun.com-20100928153459-4nudf4zgzlou4s7q) (merge vers: 5.5.7-rc) (pib:21)

Pushed into mysql-5.1-telco-7.0 5.1.51-ndb-7.0.20 (revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (version source revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (merge vers: 5.1.51-ndb-7.0.20) (pib:21)

Pushed into mysql-5.1-telco-6.3 5.1.51-ndb-6.3.39 (revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (version source revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (merge vers: 5.1.51-ndb-6.3.39) (pib:21)

Pushed into mysql-5.1-telco-6.2 5.1.51-ndb-6.2.19 (revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (version source revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (merge vers: 5.1.51-ndb-6.2.19) (pib:21)

Added to change log:

If the server crashed during a RENAME TABLE operation on an InnoDB
table, subsequent crash recovery could fail. This problem could also
affect an ALTER TABLE statement that caused a rename operation
internally.