Hi, Sachin!
Two questions:
1. This looks like there was no replication tests for periods of long
uniques at all? Please add them (create an MDEV, like "replication tests
of long unique") and add various relevant replication tests in 10.4.
2. It feels a bit risky to backport my changes to 10.4. At least you
definitely should take the latest code from 10.5. Not cherry-pick one
commit, but copy handler::prepare_for_insert() and other relevant
code from 10.5.
I will look into it.
May be it'd be safer not to backport? What would your fix look like
if you wouldn't have prepare_for_insert()?
No Fix needed, it works without any issue.
Rest of the patch reply are in context of 10.5
Also, a couple of comments to the code, see below.
On Jun 03, Sachin wrote:
> revision-id: 0f91c62d332 (mariadb-10.4.11-225-g0f91c62d332)
> parent(s): c01b3d91690
> author: Sachin <sachin.setiya@mariadb.com>
> committer: Sachin <sachin.setiya@mariadb.com>
> timestamp: 2020-06-02 08:24:24 +0530
> message:
>
> MDEV-22722 Assertion "inited==NONE" failed in handler::ha_index_init on the slave during UPDATE
>
> Add missing call for handler->prepare_for_insert() in rows event.
>
> diff --git a/mysql-test/main/long_unique_bugs_replication.test b/mysql-test/main/long_unique_bugs_replication.test
> new file mode 100644
> index 00000000000..5348be3e7d3
> --- /dev/null
> +++ b/mysql-test/main/long_unique_bugs_replication.test
> @@ -0,0 +1,22 @@
> +#
> +# Long unique bugs related to master slave replication
> +#
> +
> +#
> +# MDEV-22722 Assertion "inited==NONE" failed in handler::ha_index_init on the slave during UPDATE
> +#
> +
> +--source include/master-slave.inc
> +--source include/have_binlog_format_row.inc
master-slave.inc should always be included last, after all have_xxx.inc
files that may cause the test to be skipped.
Done.
> +
> +create table t1 (i1 int, a1 text, unique key i1 (a1)) engine=myisam;
> +insert into t1 values (1,1);
> +update t1 set a1 = 'd' limit 1;
> +
> +sync_slave_with_master;
> +connection slave;
> +
> +connection master;
> +drop table t1;
> +
> +--source include/rpl_end.inc
> diff --git a/sql/log_event.cc b/sql/log_event.cc
> index 3707f73a716..f4c0c546a38 100644
> --- a/sql/log_event.cc
> +++ b/sql/log_event.cc
> @@ -11568,6 +11568,7 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
> {
> master_had_triggers= table->master_had_triggers;
> bool transactional_table= table->file->has_transactions();
> + table->file->prepare_for_insert();
> /*
> table == NULL means that this table should not be replicated
> (this was set up by Table_map_log_event::do_apply_event()
> @@ -14344,6 +14345,7 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
> issue_long_find_row_warning(get_general_type_code(), m_table->alias.c_ptr(),
> is_index_scan, rgi);
> table->default_column_bitmaps();
> + table->file->prepare_for_insert();
Why do you need it here, I'd think that do_apply_event() should be
enough?
So it can be done in Rows_log_event::do_apply_event()
With some logic like this
if (get_general_type_code() == WRITE_ROWS_EVENT)
table->file->prepare_for_insert(0);
else if (get_general_type_code() == UPDATE_ROWS_EVENT ||
get_general_type_code() == DELETE_ROWS_EVENT)
table->file->prepare_for_insert(1);
Because table->file->prepare_for_insert(1); will be inefficient for Write Rows events
Patch is in bb-10.5-sachin branch.
> DBUG_RETURN(error);
> }
Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org