Commit 442ba20a authored by Sunny Bains's avatar Sunny Bains

Fix bug#54583. This change reverses rsvn:1350 by getting rid of a bogus assertion

and clarifies the invariant in dict_table_get_on_id().
      
In Mar 2007 Marko observed a crash during recovery, the crash resulted from
an UNDO operation on a system table. His solution was to acquire an X lock on
the data dictionary, this in hindsight was an overkill. It is unclear what
caused the crash, current hypothesis is that it was a memory corruption.
      
The X lock results in performance issues by when undoing changes due to
rollback during normal operation on regular tables.
      
Why the change is safe:
======================
The InnoDB code has changed since the original X lock change was made. In the
new code we always lock the data dictionary in X mode during startup when
UNDOing operations on the system tables (this is a given). This ensures that
the crash Marko observed cannot happen as long as all transactions that update
the system tables follow the standard rules by setting the appropriate DICT_OP
flag when writing the log records when they make the changes.
      
If transactions violate the above mentioned rule then during recovery (at
startup) the rollback code (see trx0roll.c) will not acquire the X lock
and we will see the crash again.  This will however be a different bug.
parent 93abdd67
...@@ -618,13 +618,11 @@ dict_table_get_on_id( ...@@ -618,13 +618,11 @@ dict_table_get_on_id(
if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0 if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0
|| trx->dict_operation_lock_mode == RW_X_LATCH) { || trx->dict_operation_lock_mode == RW_X_LATCH) {
/* It is a system table which will always exist in the table
cache: we avoid acquiring the dictionary mutex, because
if we are doing a rollback to handle an error in TABLE
CREATE, for example, we already have the mutex! */
ut_ad(mutex_own(&(dict_sys->mutex)) /* Note: An X latch implies that the transaction
|| trx->dict_operation_lock_mode == RW_X_LATCH); already owns the dictionary mutex. */
ut_ad(mutex_own(&dict_sys->mutex));
return(dict_table_get_on_id_low(table_id)); return(dict_table_get_on_id_low(table_id));
} }
......
...@@ -401,7 +401,7 @@ or row lock! */ ...@@ -401,7 +401,7 @@ or row lock! */
locked; see e.g. locked; see e.g.
ibuf_bitmap_get_map_page(). */ ibuf_bitmap_get_map_page(). */
#define SYNC_DICT_OPERATION 1001 /* table create, drop, etc. reserve #define SYNC_DICT_OPERATION 1001 /* table create, drop, etc. reserve
this in X-mode, implicit or backround this in X-mode; implicit or backround
operations purge, rollback, foreign operations purge, rollback, foreign
key checks reserve this in S-mode */ key checks reserve this in S-mode */
#define SYNC_DICT 1000 #define SYNC_DICT 1000
......
...@@ -272,7 +272,7 @@ row_undo( ...@@ -272,7 +272,7 @@ row_undo(
if (locked_data_dict) { if (locked_data_dict) {
row_mysql_lock_data_dictionary(trx); row_mysql_freeze_data_dictionary(trx);
} }
if (node->state == UNDO_NODE_INSERT) { if (node->state == UNDO_NODE_INSERT) {
...@@ -287,7 +287,7 @@ row_undo( ...@@ -287,7 +287,7 @@ row_undo(
if (locked_data_dict) { if (locked_data_dict) {
row_mysql_unlock_data_dictionary(trx); row_mysql_unfreeze_data_dictionary(trx);
} }
/* Do some cleanup */ /* Do some cleanup */
......
...@@ -570,13 +570,11 @@ dict_table_get_on_id( ...@@ -570,13 +570,11 @@ dict_table_get_on_id(
if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0 if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0
|| trx->dict_operation_lock_mode == RW_X_LATCH) { || trx->dict_operation_lock_mode == RW_X_LATCH) {
/* It is a system table which will always exist in the table
cache: we avoid acquiring the dictionary mutex, because
if we are doing a rollback to handle an error in TABLE
CREATE, for example, we already have the mutex! */
ut_ad(mutex_own(&(dict_sys->mutex)) /* Note: An X latch implies that the transaction
|| trx->dict_operation_lock_mode == RW_X_LATCH); already owns the dictionary mutex. */
ut_ad(mutex_own(&dict_sys->mutex));
return(dict_table_get_on_id_low(table_id)); return(dict_table_get_on_id_low(table_id));
} }
......
...@@ -438,7 +438,7 @@ or row lock! */ ...@@ -438,7 +438,7 @@ or row lock! */
#define SYNC_FILE_FORMAT_TAG 1200 /* Used to serialize access to the #define SYNC_FILE_FORMAT_TAG 1200 /* Used to serialize access to the
file format tag */ file format tag */
#define SYNC_DICT_OPERATION 1001 /* table create, drop, etc. reserve #define SYNC_DICT_OPERATION 1001 /* table create, drop, etc. reserve
this in X-mode, implicit or backround this in X-mode; implicit or backround
operations purge, rollback, foreign operations purge, rollback, foreign
key checks reserve this in S-mode */ key checks reserve this in S-mode */
#define SYNC_DICT 1000 #define SYNC_DICT 1000
......
...@@ -297,7 +297,7 @@ row_undo( ...@@ -297,7 +297,7 @@ row_undo(
if (locked_data_dict) { if (locked_data_dict) {
row_mysql_lock_data_dictionary(trx); row_mysql_freeze_data_dictionary(trx);
} }
if (node->state == UNDO_NODE_INSERT) { if (node->state == UNDO_NODE_INSERT) {
...@@ -312,7 +312,7 @@ row_undo( ...@@ -312,7 +312,7 @@ row_undo(
if (locked_data_dict) { if (locked_data_dict) {
row_mysql_unlock_data_dictionary(trx); row_mysql_unfreeze_data_dictionary(trx);
} }
/* Do some cleanup */ /* Do some cleanup */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment