MDEV-19543 Deprecate and ignore innodb_log_checksums

The parameter innodb_log_checksums that was introduced in MariaDB 10.2.2
via mysql/mysql-server@af0acedd885eb7103e319f79d25fda7386ef1506
does not make much sense. The original motivation of introducing this
parameter (initially called innodb_log_checksum_algorithm in
mysql/mysql-server@22ba38218e1d76c24f69b5a5595ad3bf5933acb0)
was that the InnoDB redo log used the slow and insecure innodb algorithm.
With hardware or SIMD accelerated CRC-32C, there should be no reason to
allow checksums to be disabled on the redo log.

The parameter innodb_encrypt_log already implies innodb_log_checksums=ON.

Let us deprecate the parameter innodb_log_checksums and always compute
redo log checksums, even if innodb_log_checksums=OFF is specified.

An upgrade from MariaDB 10.2.2 or later will only be possible after
using the default value innodb_log_checksums=ON. If the non-default
value innodb_log_checksums=OFF was in effect when the server was shut down,
a log block checksum mismatch will be reported and the upgraded server
will fail to start up.

extra/mariabackup/xtrabackup.cc

@@ -1367,11 +1367,6 @@ struct my_option xb_server_options[] =
   &xb_plugin_dir, &xb_plugin_dir,
   0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 },
 
-  {"innodb-log-checksums", OPT_INNODB_LOG_CHECKSUMS,
-   "Whether to require checksums for InnoDB redo log blocks",
-   &innodb_log_checksums, &innodb_log_checksums,
-   0, GET_BOOL, REQUIRED_ARG, 1, 0, 0, 0, 0, 0 },
-
   {"open_files_limit", OPT_OPEN_FILES_LIMIT, "the maximum number of file "
    "descriptors to reserve with setrlimit().",
    (G_PTR*) &xb_open_files_limit, (G_PTR*) &xb_open_files_limit, 0, GET_ULONG,
@@ -1959,10 +1954,6 @@ static bool innodb_init_param()
 		srv_undo_dir = (char*) ".";
 	}
 
-	log_checksum_algorithm_ptr = innodb_log_checksums || srv_encrypt_log
-		? log_block_calc_checksum_crc32
-		: log_block_calc_checksum_none;
-
 #ifdef _WIN32
 	srv_use_native_aio = TRUE;
 #endif

mysql-test/suite/encryption/r/innodb_encrypt_log.result

@@ -7,7 +7,7 @@
 #
 SET GLOBAL innodb_log_checksums=0;
 Warnings:
-Warning	138	innodb_encrypt_log implies innodb_log_checksums
+Warning	138	The parameter innodb_log_checksums is deprecated and has no effect.
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1

mysql-test/suite/sys_vars/r/innodb_log_checksums_basic.result

@@ -28,15 +28,23 @@ SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1
 SET GLOBAL innodb_log_checksums = OFF;
+Warnings:
+Warning	138	The parameter innodb_log_checksums is deprecated and has no effect.
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
-0
+1
 SET GLOBAL innodb_log_checksums = default;
+Warnings:
+Warning	138	The parameter innodb_log_checksums is deprecated and has no effect.
 SET GLOBAL innodb_log_checksums = ON;
+Warnings:
+Warning	138	The parameter innodb_log_checksums is deprecated and has no effect.
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1
 SET GLOBAL innodb_log_checksums = @orig;
+Warnings:
+Warning	138	The parameter innodb_log_checksums is deprecated and has no effect.
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1

mysql-test/suite/sys_vars/r/sysvars_innodb.result

@@ -1401,7 +1401,7 @@ GLOBAL_VALUE_ORIGIN	COMPILE-TIME
 DEFAULT_VALUE	ON
 VARIABLE_SCOPE	GLOBAL
 VARIABLE_TYPE	BOOLEAN
-VARIABLE_COMMENT	Whether to compute and require checksums for InnoDB redo log blocks
+VARIABLE_COMMENT	Deprecated parameter with no effect.
 NUMERIC_MIN_VALUE	NULL
 NUMERIC_MAX_VALUE	NULL
 NUMERIC_BLOCK_SIZE	NULL

storage/innobase/handler/ha_innodb.cc

@@ -3555,46 +3555,6 @@ static int innodb_init_abort()
 	DBUG_RETURN(1);
 }
 
-/** Update log_checksum_algorithm_ptr with a pointer to the function
-corresponding to whether checksums are enabled.
-@param[in,out]	thd	client session, or NULL if at startup
-@param[in]	check	whether redo log block checksums are enabled
-@return whether redo log block checksums are enabled */
-static inline
-bool
-innodb_log_checksums_func_update(THD* thd, bool check)
-{
-	static const char msg[] = "innodb_encrypt_log implies"
-		" innodb_log_checksums";
-
-	ut_ad(!thd == !srv_was_started);
-
-	if (!check) {
-		check = srv_encrypt_log;
-		if (!check) {
-		} else if (thd) {
-			push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-					    HA_ERR_UNSUPPORTED, msg);
-		} else {
-			sql_print_warning(msg);
-		}
-	}
-
-	if (thd) {
-		log_mutex_enter();
-		log_checksum_algorithm_ptr = check
-			? log_block_calc_checksum_crc32
-			: log_block_calc_checksum_none;
-		log_mutex_exit();
-	} else {
-		log_checksum_algorithm_ptr = check
-			? log_block_calc_checksum_crc32
-			: log_block_calc_checksum_none;
-	}
-
-	return(check);
-}
-
 /****************************************************************//**
 Gives the file extension of an InnoDB single-table tablespace. */
 static const char* ha_innobase_exts[] = {
@@ -3692,6 +3652,12 @@ static void innodb_buffer_pool_size_init()
 	innobase_buffer_pool_size = srv_buf_pool_size;
 }
 
+/** Deprecated parameter with no effect */
+static my_bool innodb_log_checksums;
+/** Deprecation message for innodb_log_checksums */
+static const char* innodb_log_checksums_deprecated
+= "The parameter innodb_log_checksums is deprecated and has no effect.";
+
 /** Initialize, validate and normalize the InnoDB startup parameters.
 @return failure code
 @retval 0 on success
@@ -4006,8 +3972,10 @@ static int innodb_init_params()
 
 	srv_buf_pool_size = ulint(innobase_buffer_pool_size);
 
-	innodb_log_checksums = innodb_log_checksums_func_update(
-		NULL, innodb_log_checksums);
+	if (!innodb_log_checksums) {
+		sql_print_warning(innodb_log_checksums_deprecated);
+		innodb_log_checksums = TRUE;
+	}
 
 	row_rollback_on_timeout = (ibool) innobase_rollback_on_timeout;
 
@@ -18586,17 +18554,14 @@ innodb_encrypt_tables_update(THD*, st_mysql_sys_var*, void*, const void* save)
 	fil_crypt_set_encrypt_tables(*static_cast<const ulong*>(save));
 }
 
-/** Update the innodb_log_checksums parameter.
-@param[in,out]	thd	client connection
-@param[out]	var_ptr	current value
-@param[in]	save	immediate result from check function */
-static
-void
-innodb_log_checksums_update(THD* thd, st_mysql_sys_var*, void* var_ptr,
-			    const void* save)
+/** Issue a deprecation warning for SET GLOBAL innodb_log_checksums.
+@param[in,out]	thd	client connection */
+static void
+innodb_log_checksums_warn(THD* thd, st_mysql_sys_var*, void*, const void*)
 {
-	*static_cast<my_bool*>(var_ptr) = innodb_log_checksums_func_update(
-		thd, *static_cast<const my_bool*>(save));
+	push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
+			    HA_ERR_UNSUPPORTED,
+			    innodb_log_checksums_deprecated);
 }
 
 static SHOW_VAR innodb_status_variables_export[]= {
@@ -18795,8 +18760,8 @@ static MYSQL_SYSVAR_ENUM(checksum_algorithm, srv_checksum_algorithm,
 
 static MYSQL_SYSVAR_BOOL(log_checksums, innodb_log_checksums,
   PLUGIN_VAR_RQCMDARG,
-  "Whether to compute and require checksums for InnoDB redo log blocks",
-  NULL, innodb_log_checksums_update, TRUE);
+  "Deprecated parameter with no effect.",
+  NULL, innodb_log_checksums_warn, TRUE);
 
 static MYSQL_SYSVAR_STR(data_home_dir, innobase_data_home_dir,
   PLUGIN_VAR_READONLY,

storage/innobase/include/log0log.h

@@ -54,10 +54,6 @@ step which modifies the database, is started */
 
 typedef ulint (*log_checksum_func_t)(const byte* log_block);
 
-/** Pointer to the log checksum calculation function. Protected with
-log_sys.mutex. */
-extern log_checksum_func_t log_checksum_algorithm_ptr;
-
 /** Append a string to the log.
 @param[in]	str		string
 @param[in]	len		string length
@@ -261,28 +257,10 @@ log_block_set_data_len(
 /*===================*/
 	byte*	log_block,	/*!< in/out: log block */
 	ulint	len);		/*!< in: data length */
-/************************************************************//**
-Calculates the checksum for a log block.
-@return checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum(
-/*====================*/
-	const byte*	block);	/*!< in: log block */
-
-/** Calculates the checksum for a log block using the CRC32 algorithm.
+/** Calculate the CRC-32C checksum of a log block.
 @param[in]	block	log block
 @return checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_crc32(
-	const byte*	block);
-
-/** Calculates the checksum for a log block using the "no-op" algorithm.
-@return		the calculated checksum value */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_none(const byte*);
+inline ulint log_block_calc_checksum_crc32(const byte* block);
 
 /************************************************************//**
 Gets a log block checksum field value.
@@ -360,9 +338,6 @@ void
 log_refresh_stats(void);
 /*===================*/
 
-/** Whether to generate and require checksums on the redo log pages */
-extern my_bool	innodb_log_checksums;
-
 /* Values used as flags */
 #define LOG_FLUSH	7652559
 #define LOG_CHECKPOINT	78656949

storage/innobase/include/log0log.ic

@@ -188,18 +188,6 @@ log_block_convert_lsn_to_no(
 	return(((ulint) (lsn / OS_FILE_LOG_BLOCK_SIZE) & 0x3FFFFFFFUL) + 1);
 }
 
-/************************************************************//**
-Calculates the checksum for a log block.
-@return checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum(
-/*====================*/
-	const byte*	block)	/*!< in: log block */
-{
-	return(log_checksum_algorithm_ptr(block));
-}
-
 /** Calculate the checksum for a log block using the pre-5.7.9 algorithm.
 @param[in]	block	log block
 @return		checksum */
@@ -229,26 +217,14 @@ log_block_calc_checksum_format_0(
 	return(sum);
 }
 
-/** Calculate the checksum for a log block using the MySQL 5.7 algorithm.
+/** Calculate the CRC-32C checksum of a log block.
 @param[in]	block	log block
 @return checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_crc32(
-	const byte*	block)
+inline ulint log_block_calc_checksum_crc32(const byte* block)
 {
 	return ut_crc32(block, OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_CHECKSUM);
 }
 
-/** Calculates the checksum for a log block using the "no-op" algorithm.
-@return        checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_none(const byte*)
-{
-	return(LOG_NO_CHECKSUM_MAGIC);
-}
-
 /************************************************************//**
 Gets a log block checksum field value.
 @return checksum */

storage/innobase/log/log0log.cc

@@ -83,12 +83,6 @@ reduce the size of the log.
 /** Redo log system */
 log_t	log_sys;
 
-/** Whether to generate and require checksums on the redo log pages */
-my_bool	innodb_log_checksums;
-
-/** Pointer to the log checksum calculation function */
-log_checksum_func_t log_checksum_algorithm_ptr;
-
 /* Next log block number to do dummy record filling if no log records written
 for a while */
 static ulint		next_lbn_to_pad = 0;
@@ -669,6 +663,12 @@ void log_t::files::create(ulint n_files)
   lsn_offset= LOG_FILE_HDR_SIZE;
 }
 
+/** Update the log block checksum. */
+inline void log_block_store_checksum(byte* block)
+{
+	log_block_set_checksum(block, log_block_calc_checksum_crc32(block));
+}
+
 /******************************************************//**
 Writes a log file header to a log file space. */
 static
@@ -698,7 +698,7 @@ log_file_header_flush(
 	       LOG_HEADER_CREATOR_CURRENT);
 	ut_ad(LOG_HEADER_CREATOR_END - LOG_HEADER_CREATOR
 	      >= sizeof LOG_HEADER_CREATOR_CURRENT);
-	log_block_set_checksum(buf, log_block_calc_checksum_crc32(buf));
+	log_block_store_checksum(buf);
 
 	dest_offset = nth_file * log_sys.log.file_size;
 
@@ -723,19 +723,6 @@ log_file_header_flush(
 	srv_stats.os_log_pending_writes.dec();
 }
 
-/******************************************************//**
-Stores a 4-byte checksum to the trailer checksum field of a log block
-before writing it to a log file. This checksum is used in recovery to
-check the consistency of a log block. */
-static
-void
-log_block_store_checksum(
-/*=====================*/
-	byte*	block)	/*!< in/out: pointer to a log block */
-{
-	log_block_set_checksum(block, log_block_calc_checksum(block));
-}
-
 /******************************************************//**
 Writes a buffer to a log file. */
 static
@@ -1315,7 +1302,7 @@ log_group_checkpoint(lsn_t end_lsn)
 			srv_log_buffer_size);
 	mach_write_to_8(buf + LOG_CHECKPOINT_END_LSN, end_lsn);
 
-	log_block_set_checksum(buf, log_block_calc_checksum_crc32(buf));
+	log_block_store_checksum(buf);
 
 	MONITOR_INC(MONITOR_PENDING_CHECKPOINT_WRITE);
 

storage/innobase/log/log0recv.cc

@@ -959,33 +959,31 @@ bool log_t::files::read_log_seg(lsn_t* start_lsn, lsn_t end_lsn)
 			break;
 		}
 
-		if (innodb_log_checksums || is_encrypted()) {
-			ulint crc = log_block_calc_checksum_crc32(buf);
-			ulint cksum = log_block_get_checksum(buf);
-
-			DBUG_EXECUTE_IF("log_intermittent_checksum_mismatch", {
-					 static int block_counter;
-					 if (block_counter++ == 0) {
-						 cksum = crc + 1;
-					 }
-			 });
-
-			if (crc != cksum) {
-				ib::error() << "Invalid log block checksum."
-					    << " block: " << block_number
-					    << " checkpoint no: "
-					    << log_block_get_checkpoint_no(buf)
-					    << " expected: " << crc
-					    << " found: " << cksum;
-				goto fail;
-			}
+		ulint crc = log_block_calc_checksum_crc32(buf);
+		ulint cksum = log_block_get_checksum(buf);
 
-			if (is_encrypted()
-			    && !log_crypt(buf, *start_lsn,
-					  OS_FILE_LOG_BLOCK_SIZE,
-					  LOG_DECRYPT)) {
-				goto fail;
-			}
+		DBUG_EXECUTE_IF("log_intermittent_checksum_mismatch", {
+				static int block_counter;
+				if (block_counter++ == 0) {
+					cksum = crc + 1;
+				}
+			});
+
+		if (crc != cksum) {
+			ib::error() << "Invalid log block checksum."
+				    << " block: " << block_number
+				    << " checkpoint no: "
+				    << log_block_get_checkpoint_no(buf)
+				    << " expected: " << crc
+				    << " found: " << cksum;
+			goto fail;
+		}
+
+		if (is_encrypted()
+		    && !log_crypt(buf, *start_lsn,
+				  OS_FILE_LOG_BLOCK_SIZE,
+				  LOG_DECRYPT)) {
+			goto fail;
 		}
 
 		ulint dl = log_block_get_data_len(buf);