mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-01-29 02:05:57 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDEV-29069 ER_KEY_NOT_FOUND on online autoinc addition + concurrent DELETE

Browse source 
We can't rely on keys formed with columns that were added during this
ALTER. These columns can be set with non-deterministic values, which can
end up with broken or incorrect search.

The same applies to the keys that contain reliable columns, but also have
bogus ones. Using them can narrow the search, but they're also ignored.

Also, added columns shouldn't be considered during the record match. To
determine them, table->has_value_set bitmap is used.

To fill has_value_set bitmap in the find_key call, extra unpack_row call
has been added.

For replication case, extra replica columns can be considered for this
case. We try to ignore them, too.
This commit is contained in:
Nikita Malyavin 2022-11-21 12:25:48 +01:00 • committed by Sergei Golubchik
parent d1e09972f0
commit 2be4c836e5
8 changed files with 415 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

137
mysql-test/main/alter_table_online_debug.result
View file

@ -979,5 +979,142 @@ xa commit 'xid' one phase;
drop table t;
set debug_sync= reset;
#
# MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and
# concurrent DELETE
#
set @old_dbug=@@debug_dbug;
set debug_dbug="+d,rpl_report_chosen_key";
create table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add pk int auto_increment primary key, algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select * from t;
a pk
11 1
30 3
#
# Add clumsy DEFAULT
#
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add b int default(RAND() * 20), add key(b),
algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select a from t;
a
11
30
# CURRENT_TIMESTAMP
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b),
algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select a from t;
a
11
30
# CURRENT_TIMESTAMP, mixed key
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b),
algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select a from t;
a
11
30
# Mixed primary key
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add b int default (a+1), add primary key(b, a),
algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select a from t;
a
11
30
#
# Normal row, could be used as a key
#
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add b int as (a * 10) unique, algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
#
# Add key for old row
#
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add unique(a), algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: 0
Note 1105 Key chosen: 0
drop table t;
set debug_sync= reset;
set debug_dbug= @old_debug;
connection default;
#
# End of 11.2 tests
#

138
mysql-test/main/alter_table_online_debug.test
View file

@ -1152,6 +1152,144 @@ xa commit 'xid' one phase;
drop table t;
set debug_sync= reset;
--echo #
--echo # MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and
--echo # concurrent DELETE
--echo #
set @old_dbug=@@debug_dbug;
set debug_dbug="+d,rpl_report_chosen_key";
create table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add pk int auto_increment primary key, algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
select * from t;
--echo #
--echo # Add clumsy DEFAULT
--echo #
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add b int default(RAND() * 20), add key(b),
algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
select a from t;
--echo # CURRENT_TIMESTAMP
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b),
algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
select a from t;
--echo # CURRENT_TIMESTAMP, mixed key
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b),
algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
select a from t;
--echo # Mixed primary key
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add b int default (a+1), add primary key(b, a),
algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
select a from t;
--echo #
--echo # Normal row, could be used as a key
--echo #
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add b int as (a * 10) unique, algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
--echo #
--echo # Add key for old row
--echo #
create or replace table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add unique(a), algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
--connection default
--reap
# Cleanup
drop table t;
set debug_sync= reset;
set debug_dbug= @old_debug;
--connection default
--echo #
--echo # End of 11.2 tests
--echo #

14
sql/log_event.h
View file

@ -4798,7 +4798,9 @@ protected:
friend class Rows_log_event;
};
int find_key(); // Find a best key to use in find_row()
int find_key(const rpl_group_info *); // Find a best key to use in find_row()
bool is_key_usable(const KEY *key) const;
bool use_pk_position() const;
int find_row(rpl_group_info *);
int write_row(rpl_group_info *, const bool);
int update_sequence();
@ -4864,8 +4866,8 @@ private:
The member function will return 0 if all went OK, or a non-zero
error code otherwise.
*/
virtual
int do_before_row_operations(const Slave_reporting_capability *const log) = 0;
virtual
int do_before_row_operations(const rpl_group_info *) = 0;
/*
Primitive to clean up after a sequence of row executions.
@ -4954,7 +4956,7 @@ private:
#endif
#if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION)
virtual int do_before_row_operations(const Slave_reporting_capability *const);
virtual int do_before_row_operations(const rpl_group_info *);
virtual int do_after_row_operations(const Slave_reporting_capability *const,int);
virtual int do_exec_row(rpl_group_info *);
#endif
@ -5044,7 +5046,7 @@ protected:
#endif
#if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION)
virtual int do_before_row_operations(const Slave_reporting_capability *const);
virtual int do_before_row_operations(const rpl_group_info *);
virtual int do_after_row_operations(const Slave_reporting_capability *const,int);
virtual int do_exec_row(rpl_group_info *);
#endif /* defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) */
@ -5131,7 +5133,7 @@ protected:
#endif
#if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION)
virtual int do_before_row_operations(const Slave_reporting_capability *const);
virtual int do_before_row_operations(const rpl_group_info *const);
virtual int do_after_row_operations(const Slave_reporting_capability *const,int);
virtual int do_exec_row(rpl_group_info *);
#endif

174
sql/log_event_server.cc
View file

@ -5100,7 +5100,7 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
this->slave_exec_mode= slave_exec_mode_options; // fix the mode
// Do event specific preparations
error= do_before_row_operations(rli);
error= do_before_row_operations(rgi);
/*
Bug#56662 Assertion failed: next_insert_id == 0, file handler.cc
@ -6453,7 +6453,7 @@ bool Write_rows_compressed_log_event::write()
#if defined(HAVE_REPLICATION)
int
Write_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const)
Write_rows_log_event::do_before_row_operations(const rpl_group_info *)
{
int error= 0;
@ -7021,16 +7021,19 @@ uint8 Write_rows_log_event::get_trg_event_map()
**************************************************************************/
#if defined(HAVE_REPLICATION)
/*
Compares table->record[0] and table->record[1]
/**
@brief Compares table->record[0] and table->record[1]
Returns TRUE if different.
@returns true if different.
*/
static bool record_compare(TABLE *table, bool vers_from_plain= false)
{
bool result= FALSE;
bool result= false;
bool all_values_set= bitmap_is_set_all(&table->has_value_set);
/**
Compare full record only if:
- all fields were given values
- there are no blob fields (otherwise we would also need
to compare blobs contents as well);
- there are no varchar fields (otherwise we would also need
@ -7041,24 +7044,23 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false)
*/
if ((table->s->blob_fields +
table->s->varchar_fields +
table->s->null_fields) == 0)
table->s->null_fields) == 0
&& all_values_set)
{
result= cmp_record(table,record[1]);
result= cmp_record(table, record[1]);
goto record_compare_exit;
}
/* Compare null bits */
if (memcmp(table->null_flags,
table->null_flags+table->s->rec_buff_length,
table->s->null_bytes))
{
result= TRUE; // Diff in NULL value
goto record_compare_exit;
}
if (all_values_set && memcmp(table->null_flags,
table->null_flags + table->s->rec_buff_length,
table->s->null_bytes))
goto record_compare_differ; // Diff in NULL value
/* Compare fields */
for (Field **ptr=table->field ; *ptr ; ptr++)
{
Field *f= *ptr;
/*
If the table is versioned, don't compare using the version if there is a
primary key. If there isn't a primary key, we need the version to
@ -7068,27 +7070,67 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false)
because the implicit row_end value will be set to the maximum value for
the latest row update (which is what we care about).
*/
if (table->versioned() && (*ptr)->vers_sys_field() &&
if (table->versioned() && f->vers_sys_field() &&
(table->s->primary_key < MAX_KEY ||
(vers_from_plain && table->vers_start_field() == (*ptr))))
(vers_from_plain && table->vers_start_field() == f)))
continue;
/**
We only compare field contents that are not null.
NULL fields (i.e., their null bits) were compared
earlier.
/*
We only compare fields that exist on the master (or in ONLINE
ALTER case, that were in the original table).
*/
if (!(*(ptr))->is_null())
if (!all_values_set)
{
if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length))
{
result= TRUE;
goto record_compare_exit;
}
if (!f->has_explicit_value())
continue;
if (f->is_null() != f->is_null(table->s->rec_buff_length))
goto record_compare_differ;
}
if (!f->is_null() && f->cmp_binary_offset(table->s->rec_buff_length))
goto record_compare_differ;
}
record_compare_exit:
return result;
record_compare_differ:
return true;
}
/**
Newly added fields with non-deterministic defaults (i.e. DEFAULT(RANDOM()),
CURRENT_TIMESTAMP, AUTO_INCREMENT) should be excluded from key search.
Basically we exclude all the default-filled fields based on
has_explicit_value bitmap.
*/
bool Rows_log_event::is_key_usable(const KEY *key) const
{
RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list;
if (!m_table->s->keys_in_use.is_set(uint(key - m_table->key_info)))
return false;
if (!tl->m_online_alter_copy_fields)
{
if (m_cols.n_bits >= m_table->s->fields)
return true;
if (m_table->s->virtual_fields + m_table->s->stored_fields == 0)
{
for (uint p= 0; p < key->user_defined_key_parts; p++)
if (key->key_part[p].fieldnr > m_cols.n_bits)
return false;
return true;
}
}
for (uint p= 0; p < key->user_defined_key_parts; p++)
{
uint field_idx= key->key_part[p].fieldnr - 1;
if (!bitmap_is_set(&m_table->has_value_set, field_idx))
return false;
}
return true;
}
@ -7103,7 +7145,7 @@ record_compare_exit:
@returns Error code on failure, 0 on success.
*/
int Rows_log_event::find_key()
int Rows_log_event::find_key(const rpl_group_info *rgi)
{
DBUG_ASSERT(m_table);
RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list;
@ -7118,6 +7160,35 @@ int Rows_log_event::find_key()
{
best_key_nr= MAX_KEY;
/*
if the source (in the row event) and destination (in m_table) records
don't have the same structure, some keys below might be unusable
for find_row().
If it's a replication and slave table (m_table) has less columns
than the master's - easy, all keys are usable.
If slave's table has more columns, but none of them are generated -
then any column beyond m_cols.n_bits makes an index unusable.
If slave's table has generated columns or it's the online alter table
where arbitrary structure conversion is possible (in the replication case
one table must be a prefix of the other, see table_def::compatible_with)
we cannot deduce what destination columns will be affected by m_cols,
we have to actually unpack one row and examine has_explicit_value()
*/
if (tl->m_online_alter_copy_fields ||
(m_cols.n_bits < m_table->s->fields &&
m_table->s->virtual_fields + m_table->s->stored_fields))
{
const uchar *curr_row_end= m_curr_row_end;
Check_level_instant_set clis(m_table->in_use, CHECK_FIELD_IGNORE);
if (int err= unpack_row(rgi, m_table, m_width, m_curr_row, &m_cols,
&curr_row_end, &m_master_reclength, m_rows_end))
DBUG_RETURN(err);
}
/*
Keys are sorted so that any primary key is first, followed by unique keys,
followed by any other. So we will automatically pick the primary key if
@ -7125,7 +7196,7 @@ int Rows_log_event::find_key()
*/
for (i= 0, key= m_table->key_info; i < m_table->s->keys; i++, key++)
{
if (!m_table->s->keys_in_use.is_set(i))
if (!is_key_usable(key))
continue;
/*
We cannot use a unique key with NULL-able columns to uniquely identify
@ -7163,12 +7234,23 @@ int Rows_log_event::find_key()
else
{
m_key_info= m_table->key_info + best_key_nr;
// Allocate buffer for key searches
m_key= (uchar *) my_malloc(PSI_INSTRUMENT_ME, m_key_info->key_length, MYF(MY_WME));
if (m_key == NULL)
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
if (!use_pk_position())
{
// Allocate buffer for key searches
m_key= (uchar *) my_malloc(PSI_INSTRUMENT_ME, m_key_info->key_length, MYF(MY_WME));
if (m_key == NULL)
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
}
}
DBUG_EXECUTE_IF("rpl_report_chosen_key",
push_warning_printf(m_table->in_use,
Sql_condition::WARN_LEVEL_NOTE,
ER_UNKNOWN_ERROR, "Key chosen: %d",
m_key_nr == MAX_KEY ?
-1 : m_key_nr););
DBUG_RETURN(0);
}
@ -7230,6 +7312,13 @@ static int row_not_found_error(rpl_group_info *rgi)
? HA_ERR_KEY_NOT_FOUND : HA_ERR_RECORD_CHANGED;
}
bool Rows_log_event::use_pk_position() const
{
return m_table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION
&& m_table->s->primary_key < MAX_KEY
&& m_key_nr == m_table->s->primary_key;
}
/**
Locate the current row in event's table.
@ -7302,8 +7391,7 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
DBUG_PRINT("info",("looking for the following record"));
DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) &&
table->s->primary_key < MAX_KEY)
if (use_pk_position())
{
/*
Use a more efficient method to fetch the record given by
@ -7562,7 +7650,7 @@ bool Delete_rows_compressed_log_event::write()
#if defined(HAVE_REPLICATION)
int
Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const)
Delete_rows_log_event::do_before_row_operations(const rpl_group_info *rgi)
{
/*
Increment the global status delete count variable
@ -7570,18 +7658,10 @@ Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability
if (get_flags(STMT_END_F))
status_var_increment(thd->status_var.com_stat[SQLCOM_DELETE]);
if ((m_table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) &&
m_table->s->primary_key < MAX_KEY)
{
/*
We don't need to allocate any memory for m_key since it is not used.
*/
return 0;
}
if (do_invoke_trigger())
m_table->prepare_triggers_for_delete_stmt_or_event();
return find_key();
return find_key(rgi);
}
int
@ -7726,7 +7806,7 @@ void Update_rows_log_event::init(MY_BITMAP const *cols)
#if defined(HAVE_REPLICATION)
int
Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const)
Update_rows_log_event::do_before_row_operations(const rpl_group_info *rgi)
{
/*
Increment the global status update count variable
@ -7735,7 +7815,7 @@ Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability
status_var_increment(thd->status_var.com_stat[SQLCOM_UPDATE]);
int err;
if ((err= find_key()))
if ((err= find_key(rgi)))
return err;
if (do_invoke_trigger())

2
sql/rpl_record.cc
View file

@ -188,7 +188,7 @@ pack_row(TABLE *table, MY_BITMAP const* cols,
@retval HA_ERR_CORRUPT_EVENT
Found error when trying to unpack fields.
*/
int unpack_row(rpl_group_info *rgi, TABLE *table, uint const colcnt,
int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
uchar const *const row_data, MY_BITMAP const *cols,
uchar const **const current_row_end,
ulong *const master_reclength, uchar const *const row_end)

2
sql/rpl_record.h
View file

@ -29,7 +29,7 @@ size_t pack_row(TABLE* table, MY_BITMAP const* cols,
#endif
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
int unpack_row(rpl_group_info *rgi,
int unpack_row(const rpl_group_info *rgi,
TABLE *table, uint const colcnt,
uchar const *const row_data, MY_BITMAP const *cols,
uchar const **const curr_row_end, ulong *const master_reclength,

2
sql/rpl_rli.cc
View file

@ -2473,7 +2473,7 @@ rpl_group_info::mark_start_commit()
If no GTID is available, then NULL is returned.
*/
char *
rpl_group_info::gtid_info()
rpl_group_info::gtid_info() const
{
if (!gtid_sub_id || !current_gtid.seq_no)
return NULL;

4
sql/rpl_rli.h
View file

@ -830,7 +830,7 @@ struct rpl_group_info
longlong row_stmt_start_timestamp;
bool long_find_row_note_printed;
/* Needs room for "Gtid D-S-N\x00". */
char gtid_info_buf[5+10+1+10+1+20+1];
mutable char gtid_info_buf[5+10+1+10+1+20+1];
/*
The timestamp, from the master, of the commit event.
@ -962,7 +962,7 @@ struct rpl_group_info
void slave_close_thread_tables(THD *);
void mark_start_commit_no_lock();
void mark_start_commit();
char *gtid_info();
char *gtid_info() const;
void unmark_start_commit();
longlong get_row_stmt_start_timestamp()