PS-11080 fix: Spoiled logical clock information in rewritten binlog (part 5) (#137)

https://perconadev.atlassian.net/browse/PS-11080

We now require that if we enable "rewrite" in gtid-based replication mode, all the
events coming from the MySQL server should include checksums (should be generated on the server with '@@global.binlog_checksum' set to 'CRC32').
We detect a violation of this rule during the inspection of the
FORMAT_DESCRIPTION event, report an error, and terminate the process with an
error status code.

This limitation is required for the scenario when MySQL server has one binlog file
with checksums enabled and another one with checksums disabled and we need
to combine them on the PBS side into a single one. To do this we need to add /
remove checksums from one group of events to make sure that all the events in
the rewritten binlog file have the same footer (either with or without checksum).
However, this cannot be easily done, because adding / removing footers to events
changes their sizes which in turn should be reflected in the 'transaction_length'
field of the GTID event preceding them. In other words, we cannot recalculate
this 'transaction_length' on the fly without receiving all events in its transaction.

Added new 'binlog_streaming.gtid_rewrite_enforce_checksums' MTR test case
with 2 combinations:
* we start with checksums enabled and rotate binlog to a new file with checksums disabled
* we start with checksums disabled and rotate binlog to a new file with checksums enabled
In both cases, we check that Binlog Server utility identifies the problem, logs an
error and terminates with an error status code.

Author: percona-ysorokin

README.md

@@ -403,7 +403,7 @@ Note: you should specify either `<connection.host>` / `<connection.port>` pair o
 - `<replication.mode>` - the replication mode, can be either `position` for position-based replication or `gtid` for GTID-based replication.
 
 #### \<replication.rewrite\> section
-If this section is present, then the utility will not split binlog events the same way as they were on the original MySQL server. Instead, it will generate its own binlog file name sequence (based on the `<replication.rewrite.base_file_name>`) and will change to a new binary log file when the size of the previous one riches the specified value (`<replication.rewrite.file_size>`). Having this section requires `<replication.mode>` to be set to `gtid`.
+If this section is present, then the utility will not split binlog events the same way as they were on the original MySQL server. Instead, it will generate its own binlog file name sequence (based on the `<replication.rewrite.base_file_name>`) and will change to a new binary log file when the size of the previous one riches the specified value (`<replication.rewrite.file_size>`). Having this section requires `<replication.mode>` to be set to `gtid`. Also, please notice that currently the utility can properly operate in 'rewrite' mode only when all binlog events received from the MySQL server have checksums (were generated on a server that had '@@global.binlog_checksum' set to 'CRC32').
 - `<replication.rewrite.base_file_name>` - the base name of the generated binlog file names in the "rewrite" mode. E.g. `rewritten_binlog` will cause `rewritten_binlog.000001`, `rewritten_binlog.000002`, etc. file names to be generated.
 - `<replication.rewrite.file_size>` - the maximum individual binlog file size after reaching which the utility will switch to a new one. The value is expected to be a string containing an integer followed by an optional suffix 'K' / 'M' / 'G' / 'T' / 'P', e.g. /\d+\[KMGTP\]?/. The minimal allowed value of this parameter is `1024` bytes.
 

mtr/binlog_streaming/r/gtid_rewrite_enforce_checksums.result

@@ -0,0 +1,36 @@
+*** Resetting replication at the very beginning of the test.
+
+*** Generating a configuration file in JSON format for the Binlog
+*** Server utility.
+
+*** Determining binlog file directory from the server.
+
+*** Creating a temporary directory <BINSRV_STORAGE_PATH> for storing
+*** binlog files downloaded via the Binlog Server utility.
+
+*** Creating a simple table and filling it with some data.
+CREATE TABLE t1(id INT UNSIGNED NOT NULL AUTO_INCREMENT, PRIMARY KEY(id)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES(DEFAULT);
+
+*** Inverting the value of the @@global.binlog_checksum system variable.
+*** This must cause server binlog rotation.
+
+*** Filling the table with more data after binlog file rotation.
+INSERT INTO t1 VALUES(DEFAULT);
+
+*** Executing the Binlog Server utility in GTID + rewrite mode.
+*** One of the binlogs generated on the server should not have
+*** checksums in its events, so we should see a dedicated error.
+
+*** Checking that Binlog Server utility reported
+*** "unsupported checksum-less event found" error.
+include/assert_grep.inc [server events must meet the checksum enforcement requirement]
+
+*** Dropping the table.
+DROP TABLE t1;
+
+*** Removing the Binlog Server utility storage directory.
+
+*** Removing the Binlog Server utility log file.
+
+*** Removing the Binlog Server utility configuration file.

mtr/binlog_streaming/t/gtid_rewrite_enforce_checksums.combinations

@@ -0,0 +1,8 @@
+[gtid_on_checksum_on]
+enforce-gtid-consistency = ON
+gtid-mode = ON
+
+[gtid_on_checksum_off]
+enforce-gtid-consistency = ON
+gtid-mode = ON
+binlog-checksum=NONE

mtr/binlog_streaming/t/gtid_rewrite_enforce_checksums.test

@@ -0,0 +1,75 @@
+--source ../include/have_binsrv.inc
+
+--source ../include/v80_v84_compatibility_defines.inc
+
+# in case of --repeat=N, we need to start from a fresh binary log to make
+# this test deterministic
+--echo *** Resetting replication at the very beginning of the test.
+--disable_query_log
+eval $stmt_reset_binary_logs_and_gtids;
+--enable_query_log
+
+# identifying backend storage type ('file' or 's3')
+--source ../include/identify_storage_backend.inc
+
+# creating data directory, configuration file, etc.
+--let $binsrv_connect_timeout = 20
+--let $binsrv_read_timeout = 60
+--let $binsrv_idle_time = 10
+--let $binsrv_verify_checksum = TRUE
+--let $binsrv_replication_mode = gtid
+--let $binsrv_rewrite_file_size = 1G
+--source ../include/set_up_binsrv_environment.inc
+
+--echo
+--echo *** Creating a simple table and filling it with some data.
+CREATE TABLE t1(id INT UNSIGNED NOT NULL AUTO_INCREMENT, PRIMARY KEY(id)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES(DEFAULT);
+
+--echo
+--echo *** Inverting the value of the @@global.binlog_checksum system variable.
+--echo *** This must cause server binlog rotation.
+
+--let $initial_binlog_checksum = `SELECT @@global.binlog_checksum`
+--disable_query_log
+if ($initial_binlog_checksum == CRC32)
+{
+  SET GLOBAL binlog_checksum = NONE;
+}
+if ($initial_binlog_checksum == NONE)
+{
+  SET GLOBAL binlog_checksum = CRC32;
+}
+--enable_query_log
+
+--echo
+--echo *** Filling the table with more data after binlog file rotation.
+INSERT INTO t1 VALUES(DEFAULT);
+
+--echo
+--echo *** Executing the Binlog Server utility in GTID + rewrite mode.
+--echo *** One of the binlogs generated on the server should not have
+--echo *** checksums in its events, so we should see a dedicated error.
+--error 1
+--exec $BINSRV fetch $binsrv_config_file_path > /dev/null
+
+--echo
+--echo *** Checking that Binlog Server utility reported
+--echo *** "unsupported checksum-less event found" error.
+--let $assert_text = server events must meet the checksum enforcement requirement
+--let $assert_file = $binsrv_log_path
+--let $assert_count = 1
+--let $assert_select = rewrite is supported in gtid replication mode only when all events received from the MySQL server have checksums
+--source include/assert_grep.inc
+
+--echo
+--echo *** Dropping the table.
+DROP TABLE t1;
+
+# restoring original state of the @@global.binlog_checksum system variable
+--disable_query_log
+eval SET GLOBAL binlog_checksum = $initial_binlog_checksum;
+--enable_query_log
+
+# cleaning up
+--source ../include/tear_down_binsrv_environment.inc

src/app.cpp

@@ -645,6 +645,7 @@ void rewrite_and_process_binlog_event(
     binsrv::basic_logger &logger, binsrv::events::reader_context &context,
     binsrv::storage &storage, std::uint32_t server_id,
     std::string_view base_file_name, std::uint64_t file_size) {
+  assert(storage.is_in_gtid_replication_mode());
   const auto current_common_header_v = current_event_v.get_common_header_view();
   const auto code = current_common_header_v.get_type_code();
 
@@ -655,6 +656,30 @@ void rewrite_and_process_binlog_event(
       code == binsrv::events::code_type::previous_gtids_log ||
       code == binsrv::events::code_type::rotate ||
       code == binsrv::events::code_type::stop) {
+    // making sure that there will be no events without checksums in the rewrite
+    // mode because when recalculating the value of 'transaction_length' field
+    // in GTID events we rely on the fact that upcoming events from the same
+    // transaction will not change their size after rewriting (currently, to
+    // avoid scenarios when one binlog file with checksums enabled is followed
+    // by another file without checksums, and we have to combine them in the
+    // same storage binlog file, we enforce that all events in the rewrite mode
+    // must have checksums)
+
+    // TODO: this restriction can be lifted if we implement whole transaction
+    //       rewrite logic (we do not add incomplete transaction events into
+    //       the storage buffer unless we receive all of them and perform
+    //       necessary checksum addition/removal)
+    if (code == binsrv::events::code_type::format_description) {
+      const auto format_description_body{binsrv::events::generic_body<
+          binsrv::events::code_type::format_description>{
+          current_event_v.get_body_raw()}};
+      if (!format_description_body.has_checksum_algorithm()) {
+        util::exception_location().raise<std::logic_error>(
+            "rewrite is supported in gtid replication mode only when "
+            "all events received from the MySQL server have checksums");
+      }
+    }
+
     const auto readable_flags{current_common_header_v.get_readable_flags()};
     logger.log(
         binsrv::log_severity::info,