Commit ade71b25 authored by Guilhem Bichot's avatar Guilhem Bichot

Fix for BUG#39697 "Maria: hang when failing to insert due to UNIQUE (seen in pushbuild2 too)"

It was a forgotten rw_unlock(), due to the deadlock detector feature (so bug was only in 5.1-maria, not
6.0-maria).

mysql-test/suite/maria/r/maria3.result:
  result, all fine
mysql-test/suite/maria/t/maria3.test:
  Test of BUG#39697: two scenarios (transactional tables, and non-transactional table but dynamic row format so still taking the rwlock) where the hang happened.
  t2 added by this test was masked by a temporary table created earlier in the test, which we forgot to drop.
storage/maria/ha_maria.cc:
  use new macro
storage/maria/ma_blockrec.c:
  use new macro
storage/maria/ma_commit.c:
  use new macro
storage/maria/ma_init.c:
  putting address of dummy_transaction_object in --debug trace can be useful
storage/maria/ma_open.c:
  use new macro
storage/maria/ma_write.c:
  if local_lock_tree is true, we have acquired keyinfo->root_lock so need to release it before "goto err".
  A pair of assertions so that our usage of TrIDs is kept sensible.
storage/maria/maria_def.h:
  A macro so that changes of MARIA_HA::trn can be tracked with --debug. It helped to understand in what cases,
  in maria_write(), we could have !(info->dup_key_trid == info->trn->trid) && !share->now_transactional
  (answer: ALTER TABLE adding UNIQUE index on transactional table).
parent c0157e8a
...@@ -506,7 +506,7 @@ count(*) ...@@ -506,7 +506,7 @@ count(*)
select count(*) from t1 where a >= 4; select count(*) from t1 where a >= 4;
count(*) count(*)
1 1
drop table t1; drop table t1, t2;
create table t1 (i int auto_increment not null primary key) transactional=0; create table t1 (i int auto_increment not null primary key) transactional=0;
check table t1 extended; check table t1 extended;
Table Op Msg_type Msg_text Table Op Msg_type Msg_text
...@@ -540,3 +540,14 @@ TABLE_SCHEMA='test' and TABLE_NAME='t1'; ...@@ -540,3 +540,14 @@ TABLE_SCHEMA='test' and TABLE_NAME='t1';
CREATE_OPTIONS CREATE_OPTIONS
transactional=1 transactional=1
drop table t1; drop table t1;
create table t1 (a int, unique(a)) engine=maria transactional=1;
insert into t1 values(1);
insert into t1 values(2),(2);
ERROR 23000: Duplicate entry '2' for key 'a'
create table t2 (a int, unique(a)) engine=maria transactional=0 row_format=dynamic;
insert into t2 values(1);
insert into t2 values(2),(2);
ERROR 23000: Duplicate entry '2' for key 'a'
insert into t1 values(3);
insert into t2 values(3);
drop table t1, t2;
...@@ -406,7 +406,7 @@ insert into t2 select * from t1; ...@@ -406,7 +406,7 @@ insert into t2 select * from t1;
insert into t1 select NULL from t2; insert into t1 select NULL from t2;
select count(*) from t1; select count(*) from t1;
select count(*) from t1 where a >= 4; select count(*) from t1 where a >= 4;
drop table t1; drop table t1, t2;
# #
# Test problems with small rows and row_type=page # Test problems with small rows and row_type=page
...@@ -461,6 +461,24 @@ select CREATE_OPTIONS from information_schema.TABLES where ...@@ -461,6 +461,24 @@ select CREATE_OPTIONS from information_schema.TABLES where
TABLE_SCHEMA='test' and TABLE_NAME='t1'; TABLE_SCHEMA='test' and TABLE_NAME='t1';
drop table t1; drop table t1;
#
# BUG#39697 - Maria: hang when failing to insert due to UNIQUE
#
create table t1 (a int, unique(a)) engine=maria transactional=1;
insert into t1 values(1);
--error 1062
insert into t1 values(2),(2);
create table t2 (a int, unique(a)) engine=maria transactional=0 row_format=dynamic;
insert into t2 values(1);
--error 1062
insert into t2 values(2),(2);
connect (root,localhost,root,,test,$MASTER_MYPORT,$MASTER_MYSOCK);
connection root;
insert into t1 values(3);
insert into t2 values(3);
connection default;
drop table t1, t2;
# End of 5.1 tests # End of 5.1 tests
--disable_result_log --disable_result_log
......
...@@ -2223,7 +2223,8 @@ int ha_maria::extra(enum ha_extra_function operation) ...@@ -2223,7 +2223,8 @@ int ha_maria::extra(enum ha_extra_function operation)
operation == HA_EXTRA_PREPARE_FOR_RENAME)) operation == HA_EXTRA_PREPARE_FOR_RENAME))
{ {
THD *thd= table->in_use; THD *thd= table->in_use;
file->trn= THD_TRN; TRN *trn= THD_TRN;
_ma_set_trn_for_table(file, trn);
} }
return maria_extra(file, operation, 0); return maria_extra(file, operation, 0);
} }
...@@ -2296,7 +2297,7 @@ int ha_maria::external_lock(THD *thd, int lock_type) ...@@ -2296,7 +2297,7 @@ int ha_maria::external_lock(THD *thd, int lock_type)
if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
trans_register_ha(thd, TRUE, maria_hton); trans_register_ha(thd, TRUE, maria_hton);
} }
file->trn= trn; _ma_set_trn_for_table(file, trn);
if (!trnman_increment_locked_tables(trn)) if (!trnman_increment_locked_tables(trn))
{ {
trans_register_ha(thd, FALSE, maria_hton); trans_register_ha(thd, FALSE, maria_hton);
...@@ -2352,7 +2353,7 @@ int ha_maria::external_lock(THD *thd, int lock_type) ...@@ -2352,7 +2353,7 @@ int ha_maria::external_lock(THD *thd, int lock_type)
if (_ma_reenable_logging_for_table(file, TRUE)) if (_ma_reenable_logging_for_table(file, TRUE))
DBUG_RETURN(1); DBUG_RETURN(1);
/** @todo zero file->trn also in commit and rollback */ /** @todo zero file->trn also in commit and rollback */
file->trn= 0; // Safety _ma_set_trn_for_table(file, NULL); // Safety
/* /*
Ensure that file->state points to the current number of rows. This Ensure that file->state points to the current number of rows. This
is needed if someone calls maria_info() without first doing an is needed if someone calls maria_info() without first doing an
...@@ -2409,7 +2410,7 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type) ...@@ -2409,7 +2410,7 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type)
different ha_maria than 'this' then this->file->trn is a stale different ha_maria than 'this' then this->file->trn is a stale
pointer. We fix it: pointer. We fix it:
*/ */
file->trn= trn; _ma_set_trn_for_table(file, trn);
/* /*
As external_lock() was already called, don't increment locked_tables. As external_lock() was already called, don't increment locked_tables.
Note that we call the function below possibly several times when Note that we call the function below possibly several times when
...@@ -2501,7 +2502,7 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn) ...@@ -2501,7 +2502,7 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn)
MARIA_HA *handler= ((ha_maria*) table->file)->file; MARIA_HA *handler= ((ha_maria*) table->file)->file;
if (handler->s->base.born_transactional) if (handler->s->base.born_transactional)
{ {
handler->trn= trn; _ma_set_trn_for_table(handler, trn);
if (handler->s->lock.get_status) if (handler->s->lock.get_status)
{ {
if (_ma_setup_live_state(handler)) if (_ma_setup_live_state(handler))
......
...@@ -7081,7 +7081,7 @@ void maria_ignore_trids(MARIA_HA *info) ...@@ -7081,7 +7081,7 @@ void maria_ignore_trids(MARIA_HA *info)
if (info->s->base.born_transactional) if (info->s->base.born_transactional)
{ {
if (!info->trn) if (!info->trn)
info->trn= &dummy_transaction_object; _ma_set_trn_for_table(info, &dummy_transaction_object);
/* Ignore transaction id when row is read */ /* Ignore transaction id when row is read */
info->trn->min_read_from= ~(TrID) 0; info->trn->min_read_from= ~(TrID) 0;
} }
......
...@@ -111,7 +111,7 @@ int maria_begin(MARIA_HA *info) ...@@ -111,7 +111,7 @@ int maria_begin(MARIA_HA *info)
DBUG_RETURN(HA_ERR_OUT_OF_MEM); DBUG_RETURN(HA_ERR_OUT_OF_MEM);
DBUG_PRINT("info", ("TRN set to 0x%lx", (ulong) trn)); DBUG_PRINT("info", ("TRN set to 0x%lx", (ulong) trn));
info->trn= trn; _ma_set_trn_for_table(info, trn);
} }
DBUG_RETURN(0); DBUG_RETURN(0);
} }
......
...@@ -68,6 +68,8 @@ int maria_init(void) ...@@ -68,6 +68,8 @@ int maria_init(void)
} }
hash_init(&maria_stored_state, &my_charset_bin, 32, hash_init(&maria_stored_state, &my_charset_bin, 32,
0, sizeof(LSN), 0, (hash_free_key) history_state_free, 0); 0, sizeof(LSN), 0, (hash_free_key) history_state_free, 0);
DBUG_PRINT("info",("dummy_transaction_object: %p",
&dummy_transaction_object));
return 0; return 0;
} }
......
...@@ -178,7 +178,8 @@ static MARIA_HA *maria_clone_internal(MARIA_SHARE *share, int mode, ...@@ -178,7 +178,8 @@ static MARIA_HA *maria_clone_internal(MARIA_SHARE *share, int mode,
if (!share->base.born_transactional) /* For transactional ones ... */ if (!share->base.born_transactional) /* For transactional ones ... */
{ {
info.trn= &dummy_transaction_object; /* ... force crash if no trn given */ /* ... force crash if no trn given */
_ma_set_trn_for_table(&info, &dummy_transaction_object);
info.state= &share->state.state; /* Change global values by default */ info.state= &share->state.state; /* Change global values by default */
} }
else else
......
...@@ -194,8 +194,23 @@ int maria_write(MARIA_HA *info, uchar *record) ...@@ -194,8 +194,23 @@ int maria_write(MARIA_HA *info, uchar *record)
Also, filter out non-thread maria use, and table modified in Also, filter out non-thread maria use, and table modified in
the same transaction. the same transaction.
*/ */
if (!local_lock_tree || info->dup_key_trid == info->trn->trid) if (!local_lock_tree)
goto err; goto err;
if (info->dup_key_trid == info->trn->trid)
{
rw_unlock(&keyinfo->root_lock);
goto err;
}
/* Different TrIDs: table must be transactional */
DBUG_ASSERT(share->base.born_transactional);
/*
If transactions are disabled, and dup_key_trid is different from
our TrID, it must be ALTER TABLE with dup_key_trid==0 (no
transaction). ALTER TABLE does have MARIA_HA::TRN not dummy but
puts TrID=0 in rows/keys.
*/
DBUG_ASSERT(share->now_transactional ||
(info->dup_key_trid == 0));
blocker= trnman_trid_to_trn(info->trn, info->dup_key_trid); blocker= trnman_trid_to_trn(info->trn, info->dup_key_trid);
/* /*
if blocker TRN was not found, it means that the conflicting if blocker TRN was not found, it means that the conflicting
......
...@@ -698,6 +698,19 @@ struct st_maria_handler ...@@ -698,6 +698,19 @@ struct st_maria_handler
#define get_pack_length(length) ((length) >= 255 ? 3 : 1) #define get_pack_length(length) ((length) >= 255 ? 3 : 1)
#define _ma_have_versioning(info) ((info)->row_flag & ROW_FLAG_TRANSID) #define _ma_have_versioning(info) ((info)->row_flag & ROW_FLAG_TRANSID)
/**
Sets table's trn and prints debug information
@param tbl MARIA_HA of table
@param newtrn what to put into tbl->trn
@note cast of newtrn is because %p of NULL gives warning (NULL is int)
*/
#define _ma_set_trn_for_table(tbl, newtrn) do { \
DBUG_PRINT("info",("table: %p trn: %p -> %p", \
(tbl), (tbl)->trn, (void *)(newtrn))); \
(tbl)->trn= (newtrn); \
} while (0)
#define MARIA_MIN_BLOCK_LENGTH 20 /* Because of delete-link */ #define MARIA_MIN_BLOCK_LENGTH 20 /* Because of delete-link */
/* Don't use to small record-blocks */ /* Don't use to small record-blocks */
#define MARIA_EXTEND_BLOCK_LENGTH 20 #define MARIA_EXTEND_BLOCK_LENGTH 20
......
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