List:Commits« Previous MessageNext Message »
From:Luís Soares Date:April 19 2010 9:17am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3449)
Bug#49741
View as plain text  
Hi Libing,

  Nice Work, especially because this must have been a tedious
  work. However, I think it's a great job you're doing, as it
  will make tests more robust.

  I have some considerations below. Please look at them and let
  me know what do you think about them.

STATUS
------
 
  Not Approved.

REQUIRED CHANGES
----------------

  RC1. Fix comment in the beginning of get_relay_log_pos:

       "# $master_pos_log: The log event's position in master
          binlog file"

       I think you mean $master_log_pos.
       

  RC2. The temporary file in get_relay_log_pos should be unique
       per get_relay_log_pos usage. Otherwise, we might be in
       trouble when running several tests in
       parallel (--parallel)...

       You can probably resort to UUID() to help you. Have a look
       at include/setup_fake_relay_log.inc for an example.

  RC3. This file needs more comments on its header... Perhaps
       something like:

       "For a given event which at position $master_log_pos in
        the master's binary log, returns its position in the
        slave's relay log file $relay_log_file.

        The position is stored in the variable $relay_log_pos.

        Usage:
          let $relay_log_file= 'relay-log-bin.000001';
          let $master_log_pos= 106;
          source include/get_relay_log_pos.inc;
          # at this point, get_relay_log_pos.inc sets $relay_log_pos.

          echo position in $relay_log_file: $relay_log_pos."

   RC4. I see no parameter checking in get_relay_log_pos. Can you
        please add something like:

        if (`SELECT '$relay_log_file' = ''`)
        {
          -- die ...
        }

        Or something similar.

    
   RC5. I don't get this description:

        "# The relationship between master_log_pos and
         #  relay_log_pos is that events_pos.c1 + 1 = events_at.c1."

        What is the unit for 1 ? 1 byte, 1 event ?


   RC6. There are other suites that were not taken into
        consideration in this patch. Eg, suites/engines or
        suites/rpl_ndb.

        I think these should be fixed as well.

   RC7. There are still tests that depend on explicit
        positions. A simple grep using the 'pos' still returns
        explicit positions, for instance: 

           grep -i pos suite/rpl/r/* | less

   RC8. In some tests, please make master_log_file to use a
        variable as well. For isntance, in
        mysql-test/suite/rpl/t/rpl_slave_skip.test, please change
        so that MASTER_LOG_FILE='master-bin.000001' becomes
        MASTER_LOG_FILE='$mysql_bin'. Something like:

        let $mysql_bin= query_get_value(SHOW MASTER STATUS, File, 1);
        (...)
        eval START SLAVE UNTIL MASTER_LOG_FILE='$mysql_bin',
MASTER_LOG_POS=$master_log_pos;
        

REQUESTS
--------

  R1. This is a request for clarification!

      For some events, the end_log_pos in the relay log seems to
      be the end_log_pos in the master's binary log. However, for
      some events (slave generated events), for instance, Rotate,
      Stop will present the end_log_pos corresponding to the
      relay log. Won't this cause any troubles ?

  R2. This is a request for feedback.

      I don't really like the show_slave_status3.inc... Perhaps
      we could remove the show_slave_status2.inc and have
      show_slave_status.inc serve all purposes... Maybe we could
      make it take some parameters so that it selectively shows
      some fields... Have you thought of that? If not, how do you
      feel about this approach?


SUGGESTIONS
-----------
 n/a

DETAILS 
-------
 n/a

On Tue, 2010-04-13 at 08:56 +0000, Li-Bing.Song@stripped wrote:
> 
> #At
> file:///home/anders/work/bzrwork/worktree1/mysql-5.1-bugteam/ based on
> revid:omer@stripped
> 
>  3449 Li-Bing.Song@stripped       2010-04-13
>       Bug #49741  test files contain explicit references to
> bin/relay-log positions
>       
>       Some of the test cases reference to binlog position and
>       these position numbers are written into result explicitly.
>       It is difficult to maintain if log event format changes. 
>       
>       After this patch, All the binlog position numbers which can be
> effected 
>       by changing binlog format are removed from the result file. 

Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3449) Bug#49741Li-Bing.Song13 Apr
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3449)Bug#49741Luís Soares19 Apr
    • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3449)Bug#49741Libing Song29 Apr