Commit 4d2b5523 authored by Daniele Sciascia's avatar Daniele Sciascia Committed by Jan Lindström

Fix FK constraint violation in applier, after ALTER TABLE ADD FK

Adding a FK constraint to an existing table (ALTER TABLE ADD FOREIGN
KEY) causes the applier to fail, if a concurrent DML statement that
violate the new constraint (i.e. a DELETE or UPDATE of record in the
parent table).

For exmaple, the following scenario causes a crash in the applier:

1. ALTER successfully adds FK constraint in node_1
2. On node_2 is UPDATE is in pre_commit() and has certified successfully
3. ALTER is delivered in node_2 and BF aborts DML
4. Applying UPDATE event causes FK violation in node_1

To avoid this situation it is necessary for UPDATE to fail during
certification. And for the UPDATE to fail certfication it is necessary
that ALTER appends certification keys for both the child and the parent
table. Before this patch, ALTER TABLE ADD FK only appended keys for
child table which is ALTERed.
parent b286a9f8
CREATE TABLE p (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id INTEGER) ENGINE=INNODB;
INSERT INTO p VALUES (1, 0);
INSERT INTO p VALUES (2, 0);
INSERT INTO c VALUES (1, 1);
INSERT INTO c VALUES (2, 2);
SET AUTOCOMMIT=ON;
START TRANSACTION;
UPDATE p SET f1 = f1 + 100;
SET SESSION wsrep_sync_wait = 0;
SET GLOBAL wsrep_provider_options = 'dbug=d,apply_monitor_slave_enter_sync';
ALTER TABLE c ADD FOREIGN KEY (p_id) REFERENCES p(f1);
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'dbug=';
SET GLOBAL wsrep_provider_options = 'dbug=d,local_monitor_enter_sync';
COMMIT;
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'signal=apply_monitor_slave_enter_sync';
SET GLOBAL wsrep_provider_options = 'signal=local_monitor_enter_sync';
SET GLOBAL wsrep_provider_options = 'dbug=';
ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
SELECT * FROM p;
f1 f2
1 0
2 0
SELECT * FROM c;
f1 p_id
1 1
2 2
DROP TABLE c;
DROP TABLE p;
CREATE TABLE p1 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE p2 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id1 INTEGER, p_id2 INTEGER) ENGINE=INNODB;
INSERT INTO p1 VALUES (1, 0), (2, 0);
INSERT INTO p2 VALUES (1, 0), (2, 0);
INSERT INTO c VALUES (1, 1, 1);
INSERT INTO c VALUES (2, 2, 2);
SET AUTOCOMMIT=ON;
START TRANSACTION;
UPDATE p1 SET f1 = f1 + 100;
SET SESSION wsrep_sync_wait = 0;
SET GLOBAL wsrep_provider_options = 'dbug=d,apply_monitor_slave_enter_sync';
ALTER TABLE c ADD FOREIGN KEY (p_id1) REFERENCES p1(f1), ADD FOREIGN KEY (p_id2) REFERENCES p2(f1);
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'dbug=';
SET GLOBAL wsrep_provider_options = 'dbug=d,local_monitor_enter_sync';
COMMIT;
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'signal=apply_monitor_slave_enter_sync';
SET GLOBAL wsrep_provider_options = 'signal=local_monitor_enter_sync';
SET GLOBAL wsrep_provider_options = 'dbug=';
ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
SELECT * FROM p1;
f1 f2
1 0
2 0
SELECT * FROM p2;
f1 f2
1 0
2 0
SELECT * FROM c;
f1 p_id1 p_id2
1 1 1
2 2 2
DROP TABLE c;
DROP TABLE p1;
DROP TABLE p2;
CREATE TABLE p1 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE p2 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id1 INTEGER, p_id2 INTEGER) ENGINE=INNODB;
INSERT INTO p1 VALUES (1, 0), (2, 0);
INSERT INTO p2 VALUES (1, 0), (2, 0);
INSERT INTO c VALUES (1, 1, 1);
INSERT INTO c VALUES (2, 2, 2);
SET AUTOCOMMIT=ON;
START TRANSACTION;
UPDATE p2 SET f1 = f1 + 100;
SET SESSION wsrep_sync_wait = 0;
SET GLOBAL wsrep_provider_options = 'dbug=d,apply_monitor_slave_enter_sync';
ALTER TABLE c ADD FOREIGN KEY (p_id1) REFERENCES p1(f1), ADD FOREIGN KEY (p_id2) REFERENCES p2(f1);
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'dbug=';
SET GLOBAL wsrep_provider_options = 'dbug=d,local_monitor_enter_sync';
COMMIT;
SET SESSION wsrep_on = 0;
SET SESSION wsrep_on = 1;
SET GLOBAL wsrep_provider_options = 'signal=apply_monitor_slave_enter_sync';
SET GLOBAL wsrep_provider_options = 'signal=local_monitor_enter_sync';
SET GLOBAL wsrep_provider_options = 'dbug=';
ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
SELECT * FROM p1;
f1 f2
1 0
2 0
SELECT * FROM p2;
f1 f2
1 0
2 0
SELECT * FROM c;
f1 p_id1 p_id2
1 1 1
2 2 2
DROP TABLE c;
DROP TABLE p1;
DROP TABLE p2;
--source include/galera_cluster.inc
--source include/have_innodb.inc
--source include/have_debug_sync.inc
--source suite/galera/include/galera_have_debug_sync.inc
# Open connection node_1a here, MW-369.inc will use it later
--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
#
# Test the scenario where a foreign key is added to an existing child table, and
# concurrently UPDATE the parent table so that it violates the constraint.
#
# We expect that ALTER TABLE ADD FOREIGN KEY adds a table level key on both
# parent and child table. And therefore we also expect the UPDATE to fail
# certification.
#
--connection node_1
CREATE TABLE p (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id INTEGER) ENGINE=INNODB;
INSERT INTO p VALUES (1, 0);
INSERT INTO p VALUES (2, 0);
INSERT INTO c VALUES (1, 1);
INSERT INTO c VALUES (2, 2);
--let $mw_369_parent_query = UPDATE p SET f1 = f1 + 100
--let $mw_369_child_query = ALTER TABLE c ADD FOREIGN KEY (p_id) REFERENCES p(f1)
--source MW-369.inc
# Expect certification failure
--connection node_1
--error ER_LOCK_DEADLOCK
--reap
--connection node_2
SELECT * FROM p;
SELECT * FROM c;
DROP TABLE c;
DROP TABLE p;
#
# Same as above, except that two foreign keys pointing to different parent
# tables are added, p1 and p2. Concurrently UPDATE p1.
#
# Expect certification error on UPDATE.
#
--connection node_1
CREATE TABLE p1 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE p2 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id1 INTEGER, p_id2 INTEGER) ENGINE=INNODB;
INSERT INTO p1 VALUES (1, 0), (2, 0);
INSERT INTO p2 VALUES (1, 0), (2, 0);
INSERT INTO c VALUES (1, 1, 1);
INSERT INTO c VALUES (2, 2, 2);
--let $mw_369_parent_query = UPDATE p1 SET f1 = f1 + 100
--let $mw_369_child_query = ALTER TABLE c ADD FOREIGN KEY (p_id1) REFERENCES p1(f1), ADD FOREIGN KEY (p_id2) REFERENCES p2(f1)
--source MW-369.inc
# Expect certification failure
--connection node_1
--error ER_LOCK_DEADLOCK
--reap
--connection node_2
SELECT * FROM p1;
SELECT * FROM p2;
SELECT * FROM c;
DROP TABLE c;
DROP TABLE p1;
DROP TABLE p2;
#
# Same as above, except that UPDATE is on p2.
#
--connection node_1
CREATE TABLE p1 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE p2 (f1 INTEGER PRIMARY KEY, f2 INTEGER) ENGINE=INNODB;
CREATE TABLE c (f1 INTEGER PRIMARY KEY, p_id1 INTEGER, p_id2 INTEGER) ENGINE=INNODB;
INSERT INTO p1 VALUES (1, 0), (2, 0);
INSERT INTO p2 VALUES (1, 0), (2, 0);
INSERT INTO c VALUES (1, 1, 1);
INSERT INTO c VALUES (2, 2, 2);
--let $mw_369_parent_query = UPDATE p2 SET f1 = f1 + 100
--let $mw_369_child_query = ALTER TABLE c ADD FOREIGN KEY (p_id1) REFERENCES p1(f1), ADD FOREIGN KEY (p_id2) REFERENCES p2(f1)
--source MW-369.inc
# Expect certification failure
--connection node_1
--error ER_LOCK_DEADLOCK
--reap
--connection node_2
SELECT * FROM p1;
SELECT * FROM p2;
SELECT * FROM c;
DROP TABLE c;
DROP TABLE p1;
DROP TABLE p2;
......@@ -53,7 +53,7 @@ my $cwd = cwd();
my $basedir;
my $socket = '/tmp/mysql.sock';
my $version = '5.5.60';
my $version = '5.5.61';
sub which
{
......
......@@ -106,14 +106,10 @@ bool Alter_table_statement::execute(THD *thd)
(!thd->is_current_stmt_binlog_format_row() ||
!find_temporary_table(thd, first_table)))
{
if (wsrep_to_isolation_begin(thd,
lex->name.str ? select_lex->db : NULL,
lex->name.str ? lex->name.str : NULL,
first_table))
{
WSREP_WARN("ALTER TABLE isolation failure");
DBUG_RETURN(TRUE);
}
WSREP_TO_ISOLATION_BEGIN_ALTER(((lex->name.str) ? select_lex->db : NULL),
((lex->name.str) ? lex->name.str : NULL),
first_table,
&alter_info);
thd->variables.auto_increment_offset = 1;
thd->variables.auto_increment_increment = 1;
......@@ -128,6 +124,11 @@ bool Alter_table_statement::execute(THD *thd)
lex->ignore, lex->online);
#ifdef WITH_WSREP
error:
{
WSREP_WARN("ALTER TABLE isolation failure");
DBUG_RETURN(TRUE);
}
#endif /* WITH_WSREP */
DBUG_RETURN(result);
}
......@@ -214,6 +214,11 @@ inline bool is_supported_parser_charset(CHARSET_INFO *cs)
#define WSREP_TO_ISOLATION_BEGIN(db_, table_, table_list_) \
if (WSREP(thd) && wsrep_to_isolation_begin(thd, db_, table_, table_list_)) goto error;
#define WSREP_TO_ISOLATION_BEGIN_ALTER(db_, table_, table_list_, alter_info_) \
if (WSREP(thd) && wsrep_to_isolation_begin(thd, db_, table_, \
table_list_, alter_info_)) \
goto error;
#define WSREP_TO_ISOLATION_END \
if (WSREP(thd) || (thd && thd->wsrep_exec_mode==TOTAL_ORDER)) \
wsrep_to_isolation_end(thd);
......
......@@ -996,8 +996,6 @@ static bool wsrep_prepare_key_for_isolation(const char* db,
if (db)
{
// sql_print_information("%s.%s", db, table);
if (db)
{
key[*key_len].ptr= db;
key[*key_len].len= strlen(db);
++(*key_len);
......@@ -1008,95 +1006,120 @@ static bool wsrep_prepare_key_for_isolation(const char* db,
++(*key_len);
}
}
}
break;
}
default:
return false;
}
return true;
}
/* Prepare key list from db/table and table_list */
bool wsrep_prepare_keys_for_isolation(THD* thd,
const char* db,
static bool wsrep_prepare_key_for_isolation(const char* db,
const char* table,
const TABLE_LIST* table_list,
wsrep_key_arr_t* ka)
{
ka->keys= 0;
ka->keys_len= 0;
extern TABLE* find_temporary_table(THD*, const TABLE_LIST*);
if (db || table)
{
TABLE_LIST tmp_table;
bzero((char*) &tmp_table,sizeof(tmp_table));
tmp_table.table_name= (char*)db;
tmp_table.db= (char*)table;
if (!table || !find_temporary_table(thd, &tmp_table))
{
if (!(ka->keys= (wsrep_key_t*)my_malloc(sizeof(wsrep_key_t), MYF(0))))
wsrep_key_t* tmp;
tmp= (wsrep_key_t*)my_realloc(ka->keys,
(ka->keys_len + 1) * sizeof(wsrep_key_t),
MYF(0));
if (!tmp)
{
WSREP_ERROR("Can't allocate memory for key_array");
goto err;
return false;
}
ka->keys_len= 1;
if (!(ka->keys[0].key_parts= (wsrep_buf_t*)
ka->keys= tmp;
if (!(ka->keys[ka->keys_len].key_parts= (wsrep_buf_t*)
my_malloc(sizeof(wsrep_buf_t)*2, MYF(0))))
{
WSREP_ERROR("Can't allocate memory for key_parts");
goto err;
return false;
}
ka->keys[0].key_parts_num= 2;
if (!wsrep_prepare_key_for_isolation(
db, table,
(wsrep_buf_t*)ka->keys[0].key_parts,
&ka->keys[0].key_parts_num))
ka->keys[ka->keys_len].key_parts_num= 2;
++ka->keys_len;
if (!wsrep_prepare_key_for_isolation(db, table,
(wsrep_buf_t*)ka->keys[ka->keys_len - 1].key_parts,
&ka->keys[ka->keys_len - 1].key_parts_num))
{
WSREP_ERROR("Preparing keys for isolation failed");
goto err;
}
}
return false;
}
for (const TABLE_LIST* table= table_list; table; table= table->next_global)
return true;
}
static bool wsrep_prepare_keys_for_alter_add_fk(char* child_table_db,
Alter_info* alter_info,
wsrep_key_arr_t* ka)
{
Key *key;
List_iterator<Key> key_iterator(alter_info->key_list);
while ((key= key_iterator++))
{
if (!find_temporary_table(thd, table))
if (key->type == Key::FOREIGN_KEY)
{
wsrep_key_t* tmp;
tmp= (wsrep_key_t*)my_realloc(
ka->keys, (ka->keys_len + 1) * sizeof(wsrep_key_t), MYF(0));
if (!tmp)
Foreign_key *fk_key= (Foreign_key *)key;
const char *db_name= fk_key->ref_table->db.str;
const char *table_name= fk_key->ref_table->table.str;
if (!db_name)
{
WSREP_ERROR("Can't allocate memory for key_array");
goto err;
db_name= child_table_db;
}
ka->keys= tmp;
if (!(ka->keys[ka->keys_len].key_parts= (wsrep_buf_t*)
my_malloc(sizeof(wsrep_buf_t)*2, MYF(0))))
if (!wsrep_prepare_key_for_isolation(db_name, table_name, ka))
{
WSREP_ERROR("Can't allocate memory for key_parts");
goto err;
return false;
}
ka->keys[ka->keys_len].key_parts_num= 2;
++ka->keys_len;
if (!wsrep_prepare_key_for_isolation(
table->db, table->table_name,
(wsrep_buf_t*)ka->keys[ka->keys_len - 1].key_parts,
&ka->keys[ka->keys_len - 1].key_parts_num))
}
}
return true;
}
static bool wsrep_prepare_keys_for_isolation(THD* thd,
const char* db,
const char* table,
const TABLE_LIST* table_list,
Alter_info* alter_info,
wsrep_key_arr_t* ka)
{
ka->keys= 0;
ka->keys_len= 0;
if (db || table)
{
WSREP_ERROR("Preparing keys for isolation failed");
if (!wsrep_prepare_key_for_isolation(db, table, ka))
goto err;
}
for (const TABLE_LIST* table= table_list; table; table= table->next_global)
{
if (!wsrep_prepare_key_for_isolation(table->db, table->table_name, ka))
goto err;
}
if (alter_info && (alter_info->flags & ALTER_FOREIGN_KEY))
{
if (!wsrep_prepare_keys_for_alter_add_fk(table_list->db, alter_info, ka))
goto err;
}
return true;
return false;
err:
wsrep_keys_free(ka);
return false;
return true;
}
/* Prepare key list from db/table and table_list */
bool wsrep_prepare_keys_for_isolation(THD* thd,
const char* db,
const char* table,
const TABLE_LIST* table_list,
wsrep_key_arr_t* ka)
{
return wsrep_prepare_keys_for_isolation(thd, db, table, table_list, NULL, ka);
}
......@@ -1260,7 +1283,8 @@ create_view_query(THD *thd, uchar** buf, size_t* buf_len)
}
static int wsrep_TOI_begin(THD *thd, char *db_, char *table_,
const TABLE_LIST* table_list)
const TABLE_LIST* table_list,
Alter_info* alter_info)
{
wsrep_status_t ret(WSREP_WARNING);
uchar* buf(0);
......@@ -1296,7 +1320,9 @@ static int wsrep_TOI_begin(THD *thd, char *db_, char *table_,
wsrep_key_arr_t key_arr= {0, 0};
struct wsrep_buf buff = { buf, buf_len };
if (!buf_err &&
wsrep_prepare_keys_for_isolation(thd, db_, table_, table_list, &key_arr)&&
!wsrep_prepare_keys_for_isolation(thd, db_, table_,
table_list, alter_info, &key_arr) &&
key_arr.keys_len > 0 &&
WSREP_OK == (ret = wsrep->to_execute_start(wsrep, thd->thread_id,
key_arr.keys, key_arr.keys_len,
&buff, 1,
......@@ -1417,9 +1443,9 @@ static void wsrep_RSU_end(THD *thd)
}
int wsrep_to_isolation_begin(THD *thd, char *db_, char *table_,
const TABLE_LIST* table_list)
const TABLE_LIST* table_list,
Alter_info* alter_info)
{
/*
No isolation for applier or replaying threads.
*/
......@@ -1469,9 +1495,12 @@ int wsrep_to_isolation_begin(THD *thd, char *db_, char *table_,
if (thd->variables.wsrep_on && thd->wsrep_exec_mode==LOCAL_STATE)
{
switch (wsrep_OSU_method_options) {
case WSREP_OSU_TOI: ret = wsrep_TOI_begin(thd, db_, table_,
table_list); break;
case WSREP_OSU_RSU: ret = wsrep_RSU_begin(thd, db_, table_); break;
case WSREP_OSU_TOI:
ret= wsrep_TOI_begin(thd, db_, table_, table_list, alter_info);
break;
case WSREP_OSU_RSU:
ret= wsrep_RSU_begin(thd, db_, table_);
break;
}
if (!ret)
{
......
......@@ -305,8 +305,10 @@ extern PSI_mutex_key key_LOCK_wsrep_slave_threads;
extern PSI_mutex_key key_LOCK_wsrep_desync;
#endif /* HAVE_PSI_INTERFACE */
struct TABLE_LIST;
class Alter_info;
int wsrep_to_isolation_begin(THD *thd, char *db_, char *table_,
const TABLE_LIST* table_list);
const TABLE_LIST* table_list,
Alter_info* alter_info = NULL);
void wsrep_to_isolation_end(THD *thd);
void wsrep_cleanup_transaction(THD *thd);
int wsrep_to_buf_helper(
......
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