• unknown's avatar
    BUG#32407: Impossible to do point-in-time recovery from older binlog · 8d37a30e
    unknown authored
    Problem: it is unsafe to read base64-printed events without first
    reading the Format_description_log_event (FD).  Currently, mysqlbinlog
    cannot print the FD.
    
    As a side effect, another bug has also been fixed: When mysqlbinlog
    --start-position=X was specified, no ROLLBACK was printed. I changed
    this, so that ROLLBACK is always printed.
    
    This patch does several things:
    
     - Format_description_log_event (FD) now print themselves in base64
       format.
    
     - mysqlbinlog is now able to print FD events.  It has three modes:
        --base64-output=auto    Print row events in base64 output, and print
                                FD event.  The FD event is printed even if
                                it is outside the range specified with
                                --start-position, because it would not be
                                safe to read row events otherwise. This is
                                the default.
    
        --base64-output=always  Like --base64-output=auto, but also print
                                base64 output for query events.  This is
                                like the old --base64-output flag, which
                                is also a shorthand for
                                --base64-output=always
    
        --base64-output=never   Never print base64 output, generate error if
                                row events occur in binlog.  This is
                                useful to suppress the FD event in binlogs
                                known not to contain row events (e.g.,
                                because BINLOG statement is unsafe,
                                requires root privileges, is not SQL, etc)
    
     - the BINLOG statement now handles FD events correctly, by setting
       the thread's rli's relay log's description_event_for_exec to the
       loaded event.
    
       In fact, executing a BINLOG statement is almost the same as reading
       an event from a relay log.  Before my patch, the code for this was
       separated (exec_relay_log_event in slave.cc executes events from
       the relay log, mysql_client_binlog_statement in sql_binlog.cc
       executes BINLOG statements).  I needed to augment
       mysql_client_binlog_statement to do parts of what
       exec_relay_log_event does.  Hence, I did a small refactoring and
       moved parts of exec_relay_log_event to a new function, which I
       named apply_event_and_update_pos.  apply_event_and_update_pos is
       called both from exec_relay_log_event and from
       mysql_client_binlog_statement.
    
     - When a non-FD event is executed in a BINLOG statement, without
       previously executing a FD event in a BINLOG statement, it generates
       an error, because that's unsafe.  I took a new error code for that:
       ER_NO_FORMAT_DESCRIPTION_EVENT_BEFORE_BINLOG_STATEMENTS.
    
       In order to get a decent error message containing the name of the
       event, I added the class method char*
       Log_event::get_type_str(Log_event_type type), which returns a
       string name for the given Log_event_type.  This is just like the
       existing char* Log_event::get_type_str(), except it is a class
       method that takes the log event type as parameter.
    
       I also added PRE_GA_*_ROWS_LOG_EVENT to Log_event::get_type_str(),
       so that names of old rows event are properly printed.
    
     - When reading an event, I added a check that the event type is known
       by the current Format_description_log_event. Without this, it may
       crash on bad input (and I was struck by this several times).
    
     - I patched the following test cases, which all contain BINLOG
       statements for row events which must be preceded by BINLOG
       statements for FD events:
        - rpl_bug31076
    
    While I was here, I fixed some small things in log_event.cc:
    
     - replaced hard-coded 4 by EVENT_TYPE_OFFSET in 3 places
    
     - replaced return by DBUG_VOID_RETURN in one place
    
     - The name of the logfile can be '-' to indicate stdin.  Before my
       patch, the code just checked if the first character is '-'; now it
       does a full strcmp().  Probably, all arguments that begin with a -
       are already handled somewhere else as flags, but I still think it
       is better that the code reflects what it is supposed to do, with as
       little dependencies as possible on other parts of the code.  If we
       one day implement that all command line arguments after -- are
       files (as most unix tools do), then we need this.
    
    I also fixed the following in slave.cc:
    
     - next_event() was declared twice, and queue_event was not static but
       should be static (not used outside the file).
    
    
    client/client_priv.h:
      Declared the new option for base64 output.
    client/mysqlbinlog.cc:
       - Change from using the two-state command line option
        "default/--base64-output" to the three-state
        "--base64-output=[never|auto|always]"
       - Print the FD event even if it is outside the --start-position range.
       - Stop if a row event is about to be printed without a preceding FD
         event.
       - Minor fixes:
          * changed 4 to EVENT_TYPE_OFFSET in some places
          * Added comments
          * before, "mysqlbinlog -xyz" read from stdin; now it does not
            (only "mysqlbinlog -" reads stdin).
    mysql-test/r/mysqlbinlog2.result:
      Updated result file: mysqlbinlog now prints ROLLBACK always.
    mysql-test/suite/binlog/t/disabled.def:
      The test must be disabled since it reveals another bug: see BUG#33247.
    mysql-test/suite/rpl/r/rpl_bug31076.result:
      Updated result file
    mysql-test/suite/rpl/r/rpl_row_mysqlbinlog.result:
      Updated result file
    mysql-test/suite/rpl/t/rpl_bug31076.test:
      Had to add explicit Format_description_log_event before other BINLOG
      statements
    mysql-test/t/mysqlbinlog2.test:
      we must suppress base64 output in result file because it contains a
      timestamp
    sql/log_event.cc:
       - Made FD events able to print themselves
       - Added check that the current FD event knows about the event type, when
         an event is about to be read. (Hint to reviewers: I had to re-indent
         a big block because of this; use diff -b)
          * To get a decent error message, I also added a class method
            const char* Log_event::get_type_str(Log_event_type)
            which converts number to event type string without having a
            Log_event object.
          * Made Log_event::get_type_str aware of PRE_GA_*_ROWS_LOG_EVENT.
       - Minor fixes:
          * Changed return to DBUG_VOID_RETURN
    sql/log_event.h:
       - Declared enum to describe the three base64_output modes
       - Use the enum instead of a flag
       - Declare the new class method get_type_str (see log_event.cc)
    sql/share/errmsg.txt:
      Added error msg.
    sql/slave.cc:
       - Factored out part of exec_relay_log_event to the new function
         apply_event_and_update_pos, because that code is needed when executing
         BINLOG statements. (this is be functionally equivalent to the
         previous code, except: (1) skipping events is now optional, controlled
         by a parameter to the new function (2) the return value of
         exec_relay_log_event has changed; see next item).
       - Changed returned error value to always be 1. Before, it would return
         the error value from apply_log_event, which was unnecessary. This
         change is safe because the exact return value of exec_relay_log_event
         is never examined; it is only tested to be ==0 or !=0.
       - Added comments describing exec_relay_log_event and
         apply_event_and_update_pos.
       - Minor fixes:
          * Removed duplicate declaration of next_event, made queue_event
            static.
          * Added doxygen code to include this file.
    sql/slave.h:
      Declared the new apply_event_and_update_pos
    sql/sql_binlog.cc:
       - Made mysql_binlog_statement set the current FD event when the given
         event is an FD event. This entails using the new function
         apply_event_and_update_pos from slave.cc instead of just calling the
         ev->apply method.
       - Made mysql_binlog_statement fail if the first BINLOG statement is not
         an FD event.
    mysql-test/suite/binlog/r/binlog_base64_flag.result:
      New test file needs new result file
    mysql-test/suite/binlog/t/binlog_base64_flag.test:
      Added test case to verify that:
       - my patch fixes the bug
       - the new --base64-output flag works as expected
       - base64 events not preceded by an FD event give an error
       - an event of a type not known by the current FD event fails cleanly.
    mysql-test/suite/binlog/std_data/binlog-bug32407.000001:
      BitKeeper file /home/sven/bk/b32407-5.1-new-rpl-mysqlbinlog_base64/mysql-test/suite/binlog/std_data/binlog-bug32407.000001
    8d37a30e
mysqlbinlog2.result 29.7 KB