MDEV-23474 InnoDB fails to restart after SET GLOBAL innodb_log_checksums=OFF

Regretfully, the parameter innodb_log_checksums was introduced
in MySQL 5.7.9 (the first GA release of that series) by
mysql/mysql-server@af0acedd885eb7103e319f79d25fda7386ef1506
which partly replaced a parameter that had been introduced in 5.7.8
mysql/mysql-server@22ba38218e1d76c24f69b5a5595ad3bf5933acb0
as innodb_log_checksum_algorithm.

Given that the CRC-32C operations are accelerated on many processor
implementations (AMD64 with SSE4.2; since MDEV-22669 also on IA-32
with SSE4.2, POWER 8 and later, ARMv8 with some extensions)
and by lookup tables when only generic SISD instructions are available,
there should be no valid reason to disable checksums.

In MariaDB 10.5.2, as a preparation for MDEV-12353, MDEV-19543 deprecated
and ignored the parameter innodb_log_checksums altogether. This should
imply that after a clean shutdown with innodb_log_checksums=OFF one
cannot upgrade to MariaDB Server 10.5 at all.

Due to these problems, let us deprecate the parameter innodb_log_checksums
and honor it only during server startup.
The command SET GLOBAL innodb_log_checksums will always set the
parameter to ON.

11 files changed, 23 insertions, 104 deletions

diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc
index 02d232b59d6..9b3d9f9ea39 100644
--- a/extra/mariabackup/xtrabackup.cc
+++ b/extra/mariabackup/xtrabackup.cc
@@ -2023,10 +2023,6 @@ innodb_init_param(void)
 		srv_undo_dir = (char*) ".";
 	}
 
-	log_checksum_algorithm_ptr = innodb_log_checksums || srv_encrypt_log
-		? log_block_calc_checksum_crc32
-		: log_block_calc_checksum_none;
-
 	return(FALSE);
 
 error:
diff --git a/mysql-test/suite/encryption/r/innodb_encrypt_log.result b/mysql-test/suite/encryption/r/innodb_encrypt_log.result
index b999e8cb34a..cd37330f2b0 100644
--- a/mysql-test/suite/encryption/r/innodb_encrypt_log.result
+++ b/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	innodb_log_checksums is deprecated and has no effect outside recovery
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1
diff --git a/mysql-test/suite/innodb/r/innodb_xtradb_compat.result b/mysql-test/suite/innodb/r/innodb_xtradb_compat.result
index 20b6ac4c892..5722120356a 100644
--- a/mysql-test/suite/innodb/r/innodb_xtradb_compat.result
+++ b/mysql-test/suite/innodb/r/innodb_xtradb_compat.result
@@ -148,7 +148,7 @@ Warnings:
 Warning	1287	Using innodb_log_checksum_algorithm is deprecated and the parameter may be removed in future releases. Ignoning the parameter.
 select @@innodb_log_checksum_algorithm, @@innodb_log_checksums;
 @@innodb_log_checksum_algorithm	@@innodb_log_checksums
-NONE	0
+STRICT_INNODB	1
 set global innodb_log_checksum_algorithm=STRICT_INNODB;
 Warnings:
 Warning	1287	Using innodb_log_checksum_algorithm is deprecated and the parameter may be removed in future releases. Ignoning the parameter.
diff --git a/mysql-test/suite/sys_vars/r/innodb_log_checksums_basic.result b/mysql-test/suite/sys_vars/r/innodb_log_checksums_basic.result
index 6679ca87249..7724ef9c2ee 100644
--- a/mysql-test/suite/sys_vars/r/innodb_log_checksums_basic.result
+++ b/mysql-test/suite/sys_vars/r/innodb_log_checksums_basic.result
@@ -28,9 +28,11 @@ SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
 1
 SET GLOBAL innodb_log_checksums = OFF;
+Warnings:
+Warning	138	innodb_log_checksums is deprecated and has no effect outside recovery
 SELECT @@global.innodb_log_checksums;
 @@global.innodb_log_checksums
-0
+1
 SET GLOBAL innodb_log_checksums = default;
 SET GLOBAL innodb_log_checksums = ON;
 SELECT @@global.innodb_log_checksums;
diff --git a/mysql-test/suite/sys_vars/r/sysvars_innodb.result b/mysql-test/suite/sys_vars/r/sysvars_innodb.result
index 98d303ce4bb..bed23a777a8 100644
--- a/mysql-test/suite/sys_vars/r/sysvars_innodb.result
+++ b/mysql-test/suite/sys_vars/r/sysvars_innodb.result
@@ -1499,7 +1499,7 @@ SESSION_VALUE	NULL
 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. Whether to require checksums for InnoDB redo log blocks.
 NUMERIC_MIN_VALUE	NULL
 NUMERIC_MAX_VALUE	NULL
 NUMERIC_BLOCK_SIZE	NULL
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index f9439ee1ce9..8b58ca57a74 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -3599,8 +3599,7 @@ static const char*	deprecated_mtflush_threads
 
 static my_bool innodb_instrument_semaphores;
 
-/** Update log_checksum_algorithm_ptr with a pointer to the function
-corresponding to whether checksums are enabled.
+/** If applicable, emit a message that log checksums cannot be disabled.
 @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 */
@@ -3608,34 +3607,21 @@ static inline
 bool
 innodb_log_checksums_func_update(THD* thd, bool check)
 {
-	static const char msg[] = "innodb_encrypt_log implies"
-		" innodb_log_checksums";
+	static const char msg[] = "innodb_log_checksums is deprecated"
+		" and has no effect outside recovery";
 
 	ut_ad(!thd == !srv_was_started);
 
 	if (!check) {
-		check = srv_encrypt_log;
-		if (!check) {
-		} else if (thd) {
+		if (thd) {
 			push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
 					    HA_ERR_UNSUPPORTED, msg);
+			check = true;
 		} 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);
 }
 
@@ -19905,7 +19891,7 @@ 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",
+  "DEPRECATED. Whether to require checksums for InnoDB redo log blocks.",
   NULL, innodb_log_checksums_update, TRUE);
 
 static MYSQL_SYSVAR_BOOL(checksums, innobase_use_checksums,
diff --git a/storage/innobase/handler/ha_xtradb.h b/storage/innobase/handler/ha_xtradb.h
index 9e898818a01..b049905613c 100644
--- a/storage/innobase/handler/ha_xtradb.h
+++ b/storage/innobase/handler/ha_xtradb.h
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2000, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, 2018, MariaDB Corporation.
+Copyright (c) 2017, 2020, MariaDB Corporation.
 Copyright (c) 2009, Percona Inc.
 
 Portions of this file contain modifications contributed and copyrighted
@@ -507,18 +507,6 @@ set_log_checksum_algorithm(THD* thd, st_mysql_sys_var*, void*, const void* save)
                             ER_WARN_DEPRECATED_SYNTAX,
                             innodb_deprecated_msg,
                             "innodb_log_checksum_algorithm");
-	log_mutex_enter();
-	srv_log_checksum_algorithm = *static_cast<const ulong*>(save);
-	if (srv_log_checksum_algorithm == SRV_CHECKSUM_ALGORITHM_NONE) {
-		ib::info() << "Setting innodb_log_checksums = false";
-		innodb_log_checksums = false;
-		log_checksum_algorithm_ptr = log_block_calc_checksum_none;
-	} else {
-		ib::info() << "Setting innodb_log_checksums = true";
-		innodb_log_checksums = true;
-		log_checksum_algorithm_ptr = log_block_calc_checksum_crc32;
-	}
-	log_mutex_exit();
 }
 static MYSQL_SYSVAR_ENUM(log_checksum_algorithm, srv_log_checksum_algorithm,
   PLUGIN_VAR_RQCMDARG,
@@ -869,15 +857,6 @@ innodb_check_deprecated(void)
 
 	if (srv_log_checksum_algorithm != SRV_CHECKSUM_ALGORITHM_DEPRECATED) {
 		innodb_print_deprecation("innodb-log-checksum-algorithm");
-		if (srv_log_checksum_algorithm == SRV_CHECKSUM_ALGORITHM_NONE) {
-			ib::info() << "Setting innodb_log_checksums = false";
-			innodb_log_checksums = false;
-			log_checksum_algorithm_ptr = log_block_calc_checksum_none;
-		} else {
-			ib::info() << "Setting innodb_log_checksums = true";
-			innodb_log_checksums = true;
-			log_checksum_algorithm_ptr = log_block_calc_checksum_crc32;
-		}
 	}
 
 	if (srv_max_changed_pages) {
diff --git a/storage/innobase/include/log0log.h b/storage/innobase/include/log0log.h
index 0fd983a1a10..612a27976e7 100644
--- a/storage/innobase/include/log0log.h
+++ b/storage/innobase/include/log0log.h
@@ -2,7 +2,7 @@
 
 Copyright (c) 1995, 2017, Oracle and/or its affiliates. All rights reserved.
 Copyright (c) 2009, Google Inc.
-Copyright (c) 2017, 2019, MariaDB Corporation.
+Copyright (c) 2017, 2020, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted by
 Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -56,12 +56,6 @@ step which modifies the database, is started */
 #define LOG_CHECKPOINT_FREE_PER_THREAD	(4 * UNIV_PAGE_SIZE)
 #define LOG_CHECKPOINT_EXTRA_FREE	(8 * UNIV_PAGE_SIZE)
 
-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
@@ -295,14 +289,6 @@ 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.
 @param[in]	block	log block
@@ -312,13 +298,6 @@ ulint
 log_block_calc_checksum_crc32(
 	const byte*	block);
 
-/** Calculates the checksum for a log block using the "no-op" algorithm.
-@param[in]	block	the redo log block
-@return		the calculated checksum value */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_none(const byte*	block);
-
 /************************************************************//**
 Gets a log block checksum field value.
 @return checksum */
@@ -403,7 +382,7 @@ log_group_close_all(void);
 void
 log_shutdown();
 
-/** Whether to generate and require checksums on the redo log pages */
+/** Whether to require checksums on the redo log pages */
 extern my_bool	innodb_log_checksums;
 
 /* Values used as flags */
diff --git a/storage/innobase/include/log0log.ic b/storage/innobase/include/log0log.ic
index 36caaedfaa2..c366affcdef 100644
--- a/storage/innobase/include/log0log.ic
+++ b/storage/innobase/include/log0log.ic
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1995, 2015, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, MariaDB Corporation.
+Copyright (c) 2017, 2020, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -190,18 +190,6 @@ log_block_convert_lsn_to_no(
 			0xFUL, 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 */
@@ -242,17 +230,6 @@ log_block_calc_checksum_crc32(
 	return(ut_crc32(block, OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE));
 }
 
-/** Calculates the checksum for a log block using the "no-op" algorithm.
-@param[in]     block   log block
-@return        checksum */
-UNIV_INLINE
-ulint
-log_block_calc_checksum_none(
-	const byte*	block)
-{
-	return(LOG_NO_CHECKSUM_MAGIC);
-}
-
 /************************************************************//**
 Gets a log block checksum field value.
 @return checksum */
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
index 972bbe7c6c0..4c68f3743e9 100644
--- a/storage/innobase/log/log0log.cc
+++ b/storage/innobase/log/log0log.cc
@@ -2,7 +2,7 @@
 
 Copyright (c) 1995, 2017, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2009, Google Inc.
-Copyright (c) 2014, 2019, MariaDB Corporation.
+Copyright (c) 2014, 2020, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted by
 Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -83,12 +83,9 @@ reduce the size of the log.
 /** Redo log system */
 log_t*	log_sys	= NULL;
 
-/** Whether to generate and require checksums on the redo log pages */
+/** Whether to 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;
@@ -857,7 +854,7 @@ log_block_store_checksum(
 /*=====================*/
 	byte*	block)	/*!< in/out: pointer to a log block */
 {
-	log_block_set_checksum(block, log_block_calc_checksum(block));
+	log_block_set_checksum(block, log_block_calc_checksum_crc32(block));
 }
 
 /******************************************************//**
diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc
index 4980e92ea6d..c4fd5f84879 100644
--- a/storage/innobase/log/log0recv.cc
+++ b/storage/innobase/log/log0recv.cc
@@ -1223,7 +1223,10 @@ recv_log_recover_10_3()
 			% univ_page_size.physical()),
 	       OS_FILE_LOG_BLOCK_SIZE, buf, NULL);
 
-	if (log_block_calc_checksum(buf) != log_block_get_checksum(buf)) {
+	const ulint cksum = log_block_get_checksum(buf);
+
+	if (cksum != LOG_NO_CHECKSUM_MAGIC
+	    && cksum != log_block_calc_checksum_crc32(buf)) {
 		return(DB_CORRUPTION);
 	}