maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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.

   May be it'd be safer not to backport? What would your fix look like
   if you wouldn't have prepare_for_insert()?

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@xxxxxxxxxxx>
> committer: Sachin <sachin.setiya@xxxxxxxxxxx>
> 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.

> +
> +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?

>    DBUG_RETURN(error);
>  }
 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx