MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:September 21 2009 12:10pm
Subject:Re: bzr commit into mysql-5.4 branch (luis.soares:2813) Bug#47200
View as plain text  
Hi Luis,

Great work.

The patch seems ok.
However, I will formally approve after reviewing the worklog.

Cheers.

Luis Soares wrote:
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/47200/mysql-next-bugfixing/
> based on revid:luis.soares@stripped
>
>  2813 Luis Soares	2009-09-10
>       BUG#47200: RBR: Absence of PK on slave leads to slave stop.
>       
>       If the same table is declared with a PK on the master but without
>       the PK on the slave, row based replication may break. This is
>       noticed for InnoDB engine, which provides HA_PARTIAL_COLUMN_READ,
>       but it may also fail with other engines providing the same
>       flag. The failure happens when searching the row on the slave to
>       be updated.
>       
>       When the slave thread performs a scan (table or index), the
>       search procedure is basically fetching rows from slave engine and
>       comparing them against the row in the event before image. If the
>       engine provides HA_PARTIAL_COLUMN_READ, then a memory comparison
>       is performed for both rows, otherwise, a column by column (for
>       only those marked in the read_set) comparison is done. There can
>       be two problems with this:
>       
>       First, engine does not honor HA_PARTIAL_COLUMN_READ for some
>       cases. When fetching the row from the slave engine, and although
>       it provides HA_PARTIAL_COLUMN_READ, the table->read_set (which
>       marks only the columns as in the master PK) is ignored. Thence,
>       the engine returns the full row instead of only the columns
>       needed for comparison. In this case, the comparison will fail.
>       
>       Second, when the RBR event row is unpacked it is filled with
>       default values for the missing columns. So, it can be the case
>       the event before image is filled with default values, on the
>       slave at unpack time. Consequently, when searching the record,
>       and even if the engine honors HA_PARTIAL_COLUMN_READ, the slave
>       will fail comparison - it will do a memory comparison of row with
>       PK + default values against a row with only the PK.
>       
>       This patch fixes this issue by removing the part of the
>       comparison that takes into account the HA_PARTIAL_COLUMN_READ. In
>       such cases it performs a column by column comparison (for those
>       marked in the read_set).
>
>     modified:
>       mysql-test/suite/rpl/r/rpl_row_record_find_innodb.result
>       mysql-test/suite/rpl/t/rpl_row_record_find_innodb.test
>       sql/log_event.cc
>       sql/log_event_old.cc
> === modified file 'mysql-test/suite/rpl/r/rpl_row_record_find_innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_record_find_innodb.result	2009-04-29 16:48:59
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_record_find_innodb.result	2009-09-10 09:37:43
> +0000
> @@ -826,3 +826,15 @@ DELETE FROM t WHERE c1 = 8;
>  DELETE FROM t;
>  Comparing tables master:test.t and slave:test.t
>  DROP TABLE t;
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (c1 int, c2 char(1), c3 char(1), 
> +c4 char(1), primary key(c1,c3)) engine=
> +InnoDB;
> +SET SQL_LOG_BIN=1;
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (c1 int, c2 char(1), c3 char(1), 
> +c4 char(1)) engine= InnoDB;
> +SET SQL_LOG_BIN=1;
> +INSERT INTO t VALUES (1, '2', '3', '4' );
> +UPDATE t SET c4 = '7';
> +DROP TABLE t;
>
> === modified file 'mysql-test/suite/rpl/t/rpl_row_record_find_innodb.test'
> --- a/mysql-test/suite/rpl/t/rpl_row_record_find_innodb.test	2009-04-29 16:48:59
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_row_record_find_innodb.test	2009-09-10 09:37:43
> +0000
> @@ -44,3 +44,35 @@ let $master_engine= MyISAM;
>  let $slave_engine= InnoDB;
>  
>  -- source extra/rpl_tests/rpl_row_record_find.test
> +
> +#
> +# BUG#47200: RBR: Absence of PK on slave leads to slave stop.
> +# 
> +
> +-- connection master
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (c1 int, c2 char(1), c3 char(1), 
> +                c4 char(1), primary key(c1,c3)) engine=
> +InnoDB;
> +SET SQL_LOG_BIN=1;
> +
> +-- connection slave
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (c1 int, c2 char(1), c3 char(1), 
> +                c4 char(1)) engine= InnoDB;
> +SET SQL_LOG_BIN=1;
> +
> +-- connection master
> +INSERT INTO t VALUES (1, '2', '3', '4' );
> +
> +-- sync_slave_with_master
> +
> +-- connection master
> +UPDATE t SET c4 = '7';
> +
> +-- sync_slave_with_master
> +-- connection master
> +
> +DROP TABLE t;
> +
> +-- sync_slave_with_master
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc	2009-08-13 14:29:55 +0000
> +++ b/sql/log_event.cc	2009-09-10 09:37:43 +0000
> @@ -8735,16 +8735,14 @@ static bool record_compare(TABLE *table,
>       ii) all columns were read or the slave SE provides partial reads
>    */
>    if ((table->s->blob_fields + table->s->varchar_fields == 0)
> &&
> -      (bitmap_is_set_all(cols_in_readset) || 
> -      (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ)))
> +      (bitmap_is_set_all(cols_in_readset)))
>    {
>      result= cmp_record(table,record[1]);
>      goto record_compare_exit;
>    }
>  
>    /* Compare null bits only if all fields were read or slave SE has partial reads
> */
> -  if ((bitmap_is_set_all(cols_in_readset) || 
> -       (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ))
> && 
> +  if ((bitmap_is_set_all(cols_in_readset)) &&
>        memcmp(table->null_flags,
>  	     table->null_flags+table->s->rec_buff_length,
>  	     table->s->null_bytes))
>
> === modified file 'sql/log_event_old.cc'
> --- a/sql/log_event_old.cc	2009-08-13 14:29:55 +0000
> +++ b/sql/log_event_old.cc	2009-09-10 09:37:43 +0000
> @@ -320,16 +320,14 @@ static bool record_compare(TABLE *table,
>        ii) all columns were read or the slave SE provides partial reads
>    */
>    if ((table->s->blob_fields + table->s->varchar_fields == 0)
> &&
> -      (bitmap_is_set_all(cols_in_readset) || 
> -      (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ)))
> +      (bitmap_is_set_all(cols_in_readset)))
>    {
>      result= cmp_record(table,record[1]);
>      goto record_compare_exit;
>    }
>  
>    /* Compare null bits only if all fields were read or slave SE has partial reads
> */
> -  if ((bitmap_is_set_all(cols_in_readset) || 
> -        (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ))
> && 
> +  if ((bitmap_is_set_all(cols_in_readset)) &&
>        memcmp(table->null_flags,
>               table->null_flags+table->s->rec_buff_length,
>               table->s->null_bytes))
>
>   
> ------------------------------------------------------------------------
>
>
>   

Thread
bzr commit into mysql-5.4 branch (luis.soares:2813) Bug#47200Luis Soares10 Sep
  • Re: bzr commit into mysql-5.4 branch (luis.soares:2813) Bug#47200Alfranio Correia21 Sep
    • Re: bzr commit into mysql-5.4 branch (luis.soares:2813) Bug#47200Alfranio Correia21 Sep
  • Re: bzr commit into mysql-5.4 branch (luis.soares:2813) Bug#47200Mats Kindahl22 Sep