From 109b8582583296c4bb4ee41bc7649f6cd2254124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 17 Aug 2017 07:19:12 +0300 Subject: [PATCH 01/15] MDEV-13432: Assertion failure in buf0rea.cc line 577 Page read could return DB_PAGE_CORRUPTED error that should be reported and passed to upper layer. In case of unknown error code we should print both number and string. --- storage/innobase/buf/buf0rea.cc | 23 +++++++++++++---------- storage/xtradb/buf/buf0rea.cc | 20 ++++++++++++-------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/storage/innobase/buf/buf0rea.cc b/storage/innobase/buf/buf0rea.cc index 5525d8dd534..d2a48975905 100644 --- a/storage/innobase/buf/buf0rea.cc +++ b/storage/innobase/buf/buf0rea.cc @@ -378,13 +378,14 @@ read_ahead: space, i); break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Random readahead failed to decrypt page " - ULINTPF "." ULINTPF " .", + "Random readahead failed to decrypt page or page corrupted " + ULINTPF ":" ULINTPF " .", space, i); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in random readahead", err, ut_strerr(err)); } } } @@ -514,15 +515,15 @@ buf_read_page_async( " in nonexisting or being-dropped tablespace", space, offset); break; - case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Async page read failed to decrypt page " + "Async page read failed to decrypt page or page corrupted " ULINTPF ":" ULINTPF ".", space, offset); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in async page read", err, ut_strerr(err)); } srv_stats.buf_pool_reads.add(count); @@ -798,13 +799,14 @@ buf_read_ahead_linear( space, i); break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Linear readahead failed to decrypt page " + "Linear readahead failed to decrypt page or page corrupted" ULINTPF ":" ULINTPF ".", space, i); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in linear readahead", err, ut_strerr(err)); } } } @@ -901,13 +903,14 @@ tablespace_deleted: zip_size, FALSE); break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Failed to decrypt insert buffer page " + "Failed to decrypt insert buffer page or page corrupted " ULINTPF ":" ULINTPF ".", space_ids[i], page_nos[i]); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in insert buffer read", err, ut_strerr(err)); } } diff --git a/storage/xtradb/buf/buf0rea.cc b/storage/xtradb/buf/buf0rea.cc index b2b737b8d40..76b71550710 100644 --- a/storage/xtradb/buf/buf0rea.cc +++ b/storage/xtradb/buf/buf0rea.cc @@ -428,13 +428,14 @@ read_ahead: space, i); break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Random readahead failed to decrypt page " + "Random readahead failed to decrypt page or page corrupted " ULINTPF ":" ULINTPF ".", i, space); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in random readahead", err, ut_strerr(err)); } } } @@ -570,13 +571,14 @@ buf_read_page_async( break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Async page read failed to decrypt page " + "Async page read failed to decrypt page or page corrupted " ULINTPF ":" ULINTPF ".", space, offset); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in async page read", err, ut_strerr(err)); } srv_stats.buf_pool_reads.add(count); @@ -863,13 +865,14 @@ buf_read_ahead_linear( break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Linear readahead failed to decrypt page " + "Linear readahead failed to decrypt page or page corrupted" ULINTPF ":" ULINTPF ".", i, space); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in linear readahead", err, ut_strerr(err)); } } } @@ -963,13 +966,14 @@ tablespace_deleted: ibuf_delete_for_discarded_space(space_ids[i]); break; case DB_DECRYPTION_FAILED: + case DB_PAGE_CORRUPTED: ib_logf(IB_LOG_LEVEL_ERROR, - "Failed to decrypt insert buffer page " + "Failed to decrypt insert buffer page or page corrupted " ULINTPF ":" ULINTPF ".", space_ids[i], page_nos[i]); break; default: - ut_error; + ib_logf(IB_LOG_LEVEL_FATAL, "Error %u (%s) in insert buffer read", err, ut_strerr(err)); } } From 06106c01481401b6d7ef4309391ca2f66242cc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 17 Aug 2017 16:13:32 +0300 Subject: [PATCH 02/15] MDEV-12988 backup fails if innodb_undo_tablespaces>0 srv_undo_tablespaces_init(): In Mariabackup backup mode, do initialize the array of undo_tablespace_ids[]. --- storage/xtradb/srv/srv0start.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/storage/xtradb/srv/srv0start.cc b/storage/xtradb/srv/srv0start.cc index fdb136ef67e..aab0bc9282b 100644 --- a/storage/xtradb/srv/srv0start.cc +++ b/storage/xtradb/srv/srv0start.cc @@ -1491,6 +1491,18 @@ srv_undo_tablespaces_init( n_undo_tablespaces = n_conf_tablespaces; undo_tablespace_ids[n_conf_tablespaces] = ULINT_UNDEFINED; + + if (backup_mode) { + ut_ad(!create_new_db); + /* MDEV-13561 FIXME: Determine srv_undo_space_id_start + from the undo001 file. */ + srv_undo_space_id_start = 1; + + for (i = 0; i < n_undo_tablespaces; i++) { + undo_tablespace_ids[i] + = i + srv_undo_space_id_start; + } + } } /* Open all the undo tablespaces that are currently in use. If we From ce6c0e584e35b516297f2afdaea5b31e508b7570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 27 Jul 2017 13:17:13 +0300 Subject: [PATCH 03/15] MDEV-8960: Can't refer the same column twice in one ALTER TABLE Problem was that if column was created in alter table when it was refered again it was not tried to find from list of current columns. mysql_prepare_alter_table: There is two cases (1) If alter table adds a new column and then later alter changes the field definition, there was no check from list of new columns, instead an incorrect error was given. (2) If alter table adds a new column and then later alter changes the default, there was no check from list of new columns, instead an incorrect error was given. --- mysql-test/r/alter_table.result | 52 +++++++++++++++++++++++++++++++++ mysql-test/t/alter_table.test | 42 ++++++++++++++++++++++++++ sql/sql_table.cc | 45 ++++++++++++++++++++++++++-- 3 files changed, 136 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/alter_table.result b/mysql-test/r/alter_table.result index cac4c477b5a..5f5e813a302 100644 --- a/mysql-test/r/alter_table.result +++ b/mysql-test/r/alter_table.result @@ -2112,3 +2112,55 @@ t1 CREATE TABLE `t1` ( `b` int(11) DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=utf8 DROP TABLE t1; +# +# MDEV-8960 Can't refer the same column twice in one ALTER TABLE +# +CREATE TABLE t1 ( +`a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL, +ALTER COLUMN `consultant_id` DROP DEFAULT; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `consultant_id` int(11) NOT NULL +) ENGINE=MyISAM DEFAULT CHARSET=utf8 +DROP TABLE t1; +CREATE TABLE t1 ( +`a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL, +ALTER COLUMN `consultant_id` SET DEFAULT 2; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `consultant_id` int(11) NOT NULL DEFAULT '2' +) ENGINE=MyISAM DEFAULT CHARSET=utf8 +DROP TABLE t1; +CREATE TABLE t1 ( +`a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL DEFAULT 2, +ALTER COLUMN `consultant_id` DROP DEFAULT; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `consultant_id` int(11) NOT NULL +) ENGINE=MyISAM DEFAULT CHARSET=utf8 +DROP TABLE t1; +CREATE TABLE t1 ( +`a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL DEFAULT 2, +ALTER COLUMN `consultant_id` DROP DEFAULT, +MODIFY COLUMN `consultant_id` BIGINT; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `consultant_id` bigint(20) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=utf8 +DROP TABLE t1; diff --git a/mysql-test/t/alter_table.test b/mysql-test/t/alter_table.test index 3670e871bb0..e252b606860 100644 --- a/mysql-test/t/alter_table.test +++ b/mysql-test/t/alter_table.test @@ -1767,3 +1767,45 @@ SHOW CREATE TABLE t1; ALTER TABLE t1 CONVERT TO CHARACTER SET utf8; SHOW CREATE TABLE t1; DROP TABLE t1; + +--echo # +--echo # MDEV-8960 Can't refer the same column twice in one ALTER TABLE +--echo # + +CREATE TABLE t1 ( + `a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; + +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL, +ALTER COLUMN `consultant_id` DROP DEFAULT; + +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 ( + `a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; + +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL, +ALTER COLUMN `consultant_id` SET DEFAULT 2; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 ( + `a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; + +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL DEFAULT 2, +ALTER COLUMN `consultant_id` DROP DEFAULT; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 ( + `a` int(11) DEFAULT NULL +) DEFAULT CHARSET=utf8; + +ALTER TABLE t1 ADD COLUMN `consultant_id` integer NOT NULL DEFAULT 2, +ALTER COLUMN `consultant_id` DROP DEFAULT, +MODIFY COLUMN `consultant_id` BIGINT; +SHOW CREATE TABLE t1; +DROP TABLE t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 2eff8fd5e2f..3ba3ec6847e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7503,9 +7503,25 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, { if (def->change && ! def->field) { - my_error(ER_BAD_FIELD_ERROR, MYF(0), def->change, - table->s->table_name.str); - goto err; + /* + Check if there is modify for newly added field. + */ + Create_field *find; + find_it.rewind(); + while((find=find_it++)) + { + if (!my_strcasecmp(system_charset_info,find->field_name, def->field_name)) + break; + } + + if (find && !find->field) + find_it.remove(); + else + { + my_error(ER_BAD_FIELD_ERROR, MYF(0), def->change, + table->s->table_name.str); + goto err; + } } /* Check that the DATE/DATETIME not null field we are going to add is @@ -7571,6 +7587,29 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, find_it.after(def); // Put column after this } } + /* + Check if there is alter for newly added field. + */ + alter_it.rewind(); + Alter_column *alter; + while ((alter=alter_it++)) + { + if (!my_strcasecmp(system_charset_info,def->field_name, alter->name)) + break; + } + if (alter) + { + if (def->sql_type == MYSQL_TYPE_BLOB) + { + my_error(ER_BLOB_CANT_HAVE_DEFAULT, MYF(0), def->change); + goto err; + } + if ((def->def=alter->def)) // Use new default + def->flags&= ~NO_DEFAULT_VALUE_FLAG; + else + def->flags|= NO_DEFAULT_VALUE_FLAG; + alter_it.remove(); + } } if (alter_info->alter_list.elements) { From 9af7561eb44a7465a8a72789678546b37da96eb8 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Mon, 21 Aug 2017 17:16:12 +0000 Subject: [PATCH 04/15] MDEV-13608 : set client plugin directory with mysql_options() if plugin_dir is specified. Also, allow to specify protocol (e.g pipe) --- extra/mariabackup/backup_mysql.cc | 5 +++ extra/mariabackup/xtrabackup.cc | 14 +++++++-- extra/mariabackup/xtrabackup.h | 2 ++ .../suite/mariabackup/auth_plugin_win.opt | 1 + .../suite/mariabackup/auth_plugin_win.result | 5 +++ .../suite/mariabackup/auth_plugin_win.test | 31 +++++++++++++++++++ 6 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 mysql-test/suite/mariabackup/auth_plugin_win.opt create mode 100644 mysql-test/suite/mariabackup/auth_plugin_win.result create mode 100644 mysql-test/suite/mariabackup/auth_plugin_win.test diff --git a/extra/mariabackup/backup_mysql.cc b/extra/mariabackup/backup_mysql.cc index 00fed457304..6f425e9419f 100644 --- a/extra/mariabackup/backup_mysql.cc +++ b/extra/mariabackup/backup_mysql.cc @@ -112,6 +112,11 @@ xb_mysql_connect() (char *) &opt_secure_auth); } + if (xb_plugin_dir && *xb_plugin_dir){ + mysql_options(connection, MYSQL_PLUGIN_DIR, xb_plugin_dir); + } + mysql_options(connection, MYSQL_OPT_PROTOCOL, &opt_protocol); + msg_ts("Connecting to MySQL server host: %s, user: %s, password: %s, " "port: %s, socket: %s\n", opt_host ? opt_host : "localhost", opt_user ? opt_user : "not set", diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index 6a373f0cfd6..c746b469329 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -123,6 +123,7 @@ my_bool xtrabackup_apply_log_only = FALSE; longlong xtrabackup_use_memory = 100*1024*1024L; my_bool xtrabackup_create_ib_logfile = FALSE; +uint opt_protocol; long xtrabackup_throttle = 0; /* 0:unlimited */ lint io_ticket; os_event_t wait_throttle = NULL; @@ -560,6 +561,7 @@ enum options_xtrabackup OPT_XTRA_TABLES_EXCLUDE, OPT_XTRA_DATABASES_EXCLUDE, + OPT_PROTOCOL }; struct my_option xb_client_options[] = @@ -799,6 +801,9 @@ struct my_option xb_client_options[] = 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, + {"protocol", OPT_PROTOCOL, "The protocol to use for connection (tcp, socket, pipe, memory).", + 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, + {"socket", 'S', "This option specifies the socket to use when " "connecting to the local database server with a UNIX domain socket. " "The option accepts a string argument. See mysql --help for details.", @@ -1364,8 +1369,13 @@ xb_get_one_option(int optid, start[1]=0 ; } break; - - + case OPT_PROTOCOL: + if (argument) + { + opt_protocol= find_type_or_exit(argument, &sql_protocol_typelib, + opt->name); + } + break; #include "sslopt-case.h" case '?': diff --git a/extra/mariabackup/xtrabackup.h b/extra/mariabackup/xtrabackup.h index 455b85c4abc..3b2a25d451b 100644 --- a/extra/mariabackup/xtrabackup.h +++ b/extra/mariabackup/xtrabackup.h @@ -55,6 +55,8 @@ extern char *xtrabackup_incremental_dir; extern char *xtrabackup_incremental_basedir; extern char *innobase_data_home_dir; extern char *innobase_buffer_pool_filename; +extern char *xb_plugin_dir; +extern uint opt_protocol; extern ds_ctxt_t *ds_meta; extern ds_ctxt_t *ds_data; diff --git a/mysql-test/suite/mariabackup/auth_plugin_win.opt b/mysql-test/suite/mariabackup/auth_plugin_win.opt new file mode 100644 index 00000000000..9e700db5f6f --- /dev/null +++ b/mysql-test/suite/mariabackup/auth_plugin_win.opt @@ -0,0 +1 @@ +--enable-named-pipe diff --git a/mysql-test/suite/mariabackup/auth_plugin_win.result b/mysql-test/suite/mariabackup/auth_plugin_win.result new file mode 100644 index 00000000000..7a623be147f --- /dev/null +++ b/mysql-test/suite/mariabackup/auth_plugin_win.result @@ -0,0 +1,5 @@ +INSTALL SONAME 'auth_named_pipe'; +CREATE USER 'USERNAME' IDENTIFIED WITH named_pipe; +GRANT ALL PRIVILEGES ON *.* to USERNAME; +DROP USER 'USERNAME'; +UNINSTALL SONAME 'auth_named_pipe'; diff --git a/mysql-test/suite/mariabackup/auth_plugin_win.test b/mysql-test/suite/mariabackup/auth_plugin_win.test new file mode 100644 index 00000000000..9c8cd5ad411 --- /dev/null +++ b/mysql-test/suite/mariabackup/auth_plugin_win.test @@ -0,0 +1,31 @@ +--source include/windows.inc +--source include/not_embedded.inc + +if (!$AUTH_NAMED_PIPE_SO) { + skip No named pipe plugin; +} + +if (!$USERNAME) { + skip USERNAME variable is undefined; +} + +if (`SELECT count(*) <> 0 FROM mysql.user WHERE user = '$USERNAME'`) { + skip \$USER=$USER which exists in mysql.user; +} + +INSTALL SONAME 'auth_named_pipe'; + +--replace_result $USERNAME USERNAME +eval CREATE USER '$USERNAME' IDENTIFIED WITH named_pipe; +--replace_result $USERNAME USERNAME +eval GRANT ALL PRIVILEGES ON *.* to $USERNAME; + +let $targetdir=$MYSQLTEST_VARDIR/tmp/backup; +--disable_result_log +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf -u $USERNAME --backup --protocol=pipe --target-dir=$targetdir; +--enable_result_log +--replace_result $USERNAME USERNAME +eval DROP USER '$USERNAME'; +rmdir $targetdir; +UNINSTALL SONAME 'auth_named_pipe'; + From a00b74d994c43220ff6158c24064dfe90ff40e9a Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Tue, 22 Aug 2017 13:03:40 +0000 Subject: [PATCH 05/15] fix auth_plugin_win test prepend enable-named-pipe (windows-only) option in auth_plugin_win.opt with loose- prefix, to avoid warning on non-Windows. --- mysql-test/suite/mariabackup/auth_plugin_win.opt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/suite/mariabackup/auth_plugin_win.opt b/mysql-test/suite/mariabackup/auth_plugin_win.opt index 9e700db5f6f..e534ae1eae5 100644 --- a/mysql-test/suite/mariabackup/auth_plugin_win.opt +++ b/mysql-test/suite/mariabackup/auth_plugin_win.opt @@ -1 +1 @@ ---enable-named-pipe +--loose-enable-named-pipe From 97f9d3c08056f8b532118c0fb1b988df18db4f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 23 Aug 2017 10:01:48 +0300 Subject: [PATCH 06/15] MDEV-13167 InnoDB key rotation is not skipping unused pages In key rotation, we must initialize unallocated but previously initialized pages, so that if encryption is enabled on a table, all clear-text data for the page will eventually be overwritten. But we should not rotate keys on pages that were never allocated after the data file was created. According to the latching order rules, after acquiring the tablespace latch, no page latches of previously allocated user pages may be acquired. So, key rotation should check the page allocation status after acquiring the page latch, not before. But, the latching order rules also prohibit accessing pages that were not allocated first, and then acquiring the tablespace latch. Such behaviour would indeed result in a deadlock when running the following tests: encryption.innodb_encryption-page-compression encryption.innodb-checksum-algorithm Because the key rotation is accessing potentially unallocated pages, it cannot reliably check if these pages were allocated. It can only check the page header. If the page number is zero, we can assume that the page is unallocated. fil_crypt_rotate_page(): Detect uninitialized pages by FIL_PAGE_OFFSET. Page 0 is never encrypted, and on other pages that are initialized, FIL_PAGE_OFFSET must contain the page number. fil_crypt_is_page_uninitialized(): Remove. It suffices to check the page number field in fil_crypt_rotate_page(). --- storage/innobase/fil/fil0crypt.cc | 68 +++++++++++++++---------------- storage/xtradb/fil/fil0crypt.cc | 68 +++++++++++++++---------------- 2 files changed, 64 insertions(+), 72 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index b5b762c2cd9..683974d8920 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1649,20 +1649,6 @@ fil_crypt_find_page_to_rotate( return found; } -/*********************************************************************** -Check if a page is uninitialized (doesn't need to be rotated) -@param[in] frame Page to check -@param[in] zip_size zip_size or 0 -@return true if page is uninitialized, false if not. */ -static inline -bool -fil_crypt_is_page_uninitialized( - const byte *frame, - uint zip_size) -{ - return (buf_page_is_zeroes(frame, zip_size)); -} - #define fil_crypt_get_page_throttle(state,offset,mtr,sleeptime_ms) \ fil_crypt_get_page_throttle_func(state, offset, mtr, \ sleeptime_ms, __FILE__, __LINE__) @@ -1823,6 +1809,7 @@ fil_crypt_rotate_page( fil_space_crypt_t *crypt_data = space->crypt_data; ut_ad(space->n_pending_ops > 0); + ut_ad(offset > 0); /* In fil_crypt_thread where key rotation is done we have acquired space and checked that this space is not yet @@ -1851,31 +1838,40 @@ fil_crypt_rotate_page( byte* frame = buf_block_get_frame(block); uint kv = mach_read_from_4(frame+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - /* check if tablespace is closing after reading page */ - if (!space->is_stopping()) { + if (space->is_stopping()) { + /* The tablespace is closing (in DROP TABLE or + TRUNCATE TABLE or similar): avoid further access */ + } else if (!*reinterpret_cast(FIL_PAGE_OFFSET + + frame)) { + /* It looks like this page was never + allocated. Because key rotation is accessing + pages in a pattern that is unlike the normal + B-tree and undo log access pattern, we cannot + invoke fseg_page_is_free() here, because that + could result in a deadlock. If we invoked + fseg_page_is_free() and released the + tablespace latch before acquiring block->lock, + then the fseg_page_is_free() information + could be stale already. */ + ut_ad(kv == 0); + ut_ad(page_get_space_id(frame) == 0); + } else if (fil_crypt_needs_rotation( + crypt_data->encryption, + kv, key_state->key_version, + key_state->rotate_key_age)) { - if (kv == 0 && - fil_crypt_is_page_uninitialized(frame, zip_size)) { - ; - } else if (fil_crypt_needs_rotation( - crypt_data->encryption, - kv, key_state->key_version, - key_state->rotate_key_age)) { + modified = true; - modified = true; + /* force rotation by dummy updating page */ + mlog_write_ulint(frame + FIL_PAGE_SPACE_ID, + space_id, MLOG_4BYTES, &mtr); - /* force rotation by dummy updating page */ - mlog_write_ulint(frame + - FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, - space_id, MLOG_4BYTES, &mtr); - - /* statistics */ - state->crypt_stat.pages_modified++; - } else { - if (crypt_data->is_encrypted()) { - if (kv < state->min_key_version_found) { - state->min_key_version_found = kv; - } + /* statistics */ + state->crypt_stat.pages_modified++; + } else { + if (crypt_data->is_encrypted()) { + if (kv < state->min_key_version_found) { + state->min_key_version_found = kv; } } diff --git a/storage/xtradb/fil/fil0crypt.cc b/storage/xtradb/fil/fil0crypt.cc index b5b762c2cd9..683974d8920 100644 --- a/storage/xtradb/fil/fil0crypt.cc +++ b/storage/xtradb/fil/fil0crypt.cc @@ -1649,20 +1649,6 @@ fil_crypt_find_page_to_rotate( return found; } -/*********************************************************************** -Check if a page is uninitialized (doesn't need to be rotated) -@param[in] frame Page to check -@param[in] zip_size zip_size or 0 -@return true if page is uninitialized, false if not. */ -static inline -bool -fil_crypt_is_page_uninitialized( - const byte *frame, - uint zip_size) -{ - return (buf_page_is_zeroes(frame, zip_size)); -} - #define fil_crypt_get_page_throttle(state,offset,mtr,sleeptime_ms) \ fil_crypt_get_page_throttle_func(state, offset, mtr, \ sleeptime_ms, __FILE__, __LINE__) @@ -1823,6 +1809,7 @@ fil_crypt_rotate_page( fil_space_crypt_t *crypt_data = space->crypt_data; ut_ad(space->n_pending_ops > 0); + ut_ad(offset > 0); /* In fil_crypt_thread where key rotation is done we have acquired space and checked that this space is not yet @@ -1851,31 +1838,40 @@ fil_crypt_rotate_page( byte* frame = buf_block_get_frame(block); uint kv = mach_read_from_4(frame+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - /* check if tablespace is closing after reading page */ - if (!space->is_stopping()) { + if (space->is_stopping()) { + /* The tablespace is closing (in DROP TABLE or + TRUNCATE TABLE or similar): avoid further access */ + } else if (!*reinterpret_cast(FIL_PAGE_OFFSET + + frame)) { + /* It looks like this page was never + allocated. Because key rotation is accessing + pages in a pattern that is unlike the normal + B-tree and undo log access pattern, we cannot + invoke fseg_page_is_free() here, because that + could result in a deadlock. If we invoked + fseg_page_is_free() and released the + tablespace latch before acquiring block->lock, + then the fseg_page_is_free() information + could be stale already. */ + ut_ad(kv == 0); + ut_ad(page_get_space_id(frame) == 0); + } else if (fil_crypt_needs_rotation( + crypt_data->encryption, + kv, key_state->key_version, + key_state->rotate_key_age)) { - if (kv == 0 && - fil_crypt_is_page_uninitialized(frame, zip_size)) { - ; - } else if (fil_crypt_needs_rotation( - crypt_data->encryption, - kv, key_state->key_version, - key_state->rotate_key_age)) { + modified = true; - modified = true; + /* force rotation by dummy updating page */ + mlog_write_ulint(frame + FIL_PAGE_SPACE_ID, + space_id, MLOG_4BYTES, &mtr); - /* force rotation by dummy updating page */ - mlog_write_ulint(frame + - FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, - space_id, MLOG_4BYTES, &mtr); - - /* statistics */ - state->crypt_stat.pages_modified++; - } else { - if (crypt_data->is_encrypted()) { - if (kv < state->min_key_version_found) { - state->min_key_version_found = kv; - } + /* statistics */ + state->crypt_stat.pages_modified++; + } else { + if (crypt_data->is_encrypted()) { + if (kv < state->min_key_version_found) { + state->min_key_version_found = kv; } } From b8b3ba632b8596bb483420dc6da46925f1c9b094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 23 Aug 2017 13:03:13 +0300 Subject: [PATCH 07/15] MDEV-13606 XA PREPARE transactions should survive innodb_force_recovery=1 or 2 When MySQL 5.0.3 introduced InnoDB support for two-phase commit, it also introduced the questionable logic to roll back XA PREPARE transactions on startup when innodb_force_recovery is 1 or 2. Remove this logic in order to avoid unwanted side effects when innodb_force_recovery is being set for other reasons. That is, XA PREPARE transactions will always remain in that state until InnoDB receives an explicit XA ROLLBACK or XA COMMIT request from the upper layer. At the time the logic was introduced in MySQL 5.0.3, there already was a startup parameter that is the preferred way of achieving the behaviour: --tc-heuristic-recover=ROLLBACK. --- mysql-test/suite/innodb/r/xa_recovery.result | 2 +- mysql-test/suite/innodb/t/xa_recovery.test | 5 +++ storage/innobase/trx/trx0trx.cc | 36 ++++++-------------- storage/xtradb/trx/trx0trx.cc | 36 ++++++-------------- 4 files changed, 26 insertions(+), 53 deletions(-) diff --git a/mysql-test/suite/innodb/r/xa_recovery.result b/mysql-test/suite/innodb/r/xa_recovery.result index 4441701f961..3b560b79657 100644 --- a/mysql-test/suite/innodb/r/xa_recovery.result +++ b/mysql-test/suite/innodb/r/xa_recovery.result @@ -4,7 +4,7 @@ XA START 'x'; UPDATE t1 set a=2; XA END 'x'; XA PREPARE 'x'; -# Kill and restart +# Kill and restart: --innodb-force-recovery=2 SELECT * FROM t1 LOCK IN SHARE MODE; SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; SELECT * FROM t1; diff --git a/mysql-test/suite/innodb/t/xa_recovery.test b/mysql-test/suite/innodb/t/xa_recovery.test index f5c2b655545..d4f957c7bf4 100644 --- a/mysql-test/suite/innodb/t/xa_recovery.test +++ b/mysql-test/suite/innodb/t/xa_recovery.test @@ -15,7 +15,12 @@ connect (con1,localhost,root); XA START 'x'; UPDATE t1 set a=2; XA END 'x'; XA PREPARE 'x'; connection default; +# innodb_force_recovery=2 prevents the purge and tests that the fix of +# MDEV-13606 XA PREPARE transactions should survive innodb_force_recovery=1 or 2 +# is present. +--let $restart_parameters= --innodb-force-recovery=2 --source include/kill_and_restart_mysqld.inc +--let $restart_parameters= disconnect con1; connect (con1,localhost,root); diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index f06d8c33183..c38c9bf7188 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -536,18 +537,9 @@ trx_resurrect_insert( "InnoDB: Transaction " TRX_ID_FMT " was in the" " XA prepared state.\n", trx->id); - if (srv_force_recovery == 0) { - - trx->state = TRX_STATE_PREPARED; - trx_sys->n_prepared_trx++; - trx_sys->n_prepared_recovered_trx++; - } else { - fprintf(stderr, - "InnoDB: Since innodb_force_recovery" - " > 0, we will rollback it anyway.\n"); - - trx->state = TRX_STATE_ACTIVE; - } + trx->state = TRX_STATE_PREPARED; + trx_sys->n_prepared_trx++; + trx_sys->n_prepared_recovered_trx++; } else { trx->state = TRX_STATE_COMMITTED_IN_MEMORY; } @@ -605,22 +597,14 @@ trx_resurrect_update_in_prepared_state( "InnoDB: Transaction " TRX_ID_FMT " was in the XA prepared state.\n", trx->id); - if (srv_force_recovery == 0) { - if (trx_state_eq(trx, TRX_STATE_NOT_STARTED)) { - trx_sys->n_prepared_trx++; - trx_sys->n_prepared_recovered_trx++; - } else { - ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); - } - - trx->state = TRX_STATE_PREPARED; + if (trx_state_eq(trx, TRX_STATE_NOT_STARTED)) { + trx_sys->n_prepared_trx++; + trx_sys->n_prepared_recovered_trx++; } else { - fprintf(stderr, - "InnoDB: Since innodb_force_recovery" - " > 0, we will rollback it anyway.\n"); - - trx->state = TRX_STATE_ACTIVE; + ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); } + + trx->state = TRX_STATE_PREPARED; } else { trx->state = TRX_STATE_COMMITTED_IN_MEMORY; } diff --git a/storage/xtradb/trx/trx0trx.cc b/storage/xtradb/trx/trx0trx.cc index fd620ae6659..d3b1f1da054 100644 --- a/storage/xtradb/trx/trx0trx.cc +++ b/storage/xtradb/trx/trx0trx.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -714,18 +715,9 @@ trx_resurrect_insert( "InnoDB: Transaction " TRX_ID_FMT " was in the" " XA prepared state.\n", trx->id); - if (srv_force_recovery == 0) { - - trx->state = TRX_STATE_PREPARED; - trx_sys->n_prepared_trx++; - trx_sys->n_prepared_recovered_trx++; - } else { - fprintf(stderr, - "InnoDB: Since innodb_force_recovery" - " > 0, we will rollback it anyway.\n"); - - trx->state = TRX_STATE_ACTIVE; - } + trx->state = TRX_STATE_PREPARED; + trx_sys->n_prepared_trx++; + trx_sys->n_prepared_recovered_trx++; } else { trx->state = TRX_STATE_COMMITTED_IN_MEMORY; } @@ -783,22 +775,14 @@ trx_resurrect_update_in_prepared_state( "InnoDB: Transaction " TRX_ID_FMT " was in the XA prepared state.\n", trx->id); - if (srv_force_recovery == 0) { - if (trx_state_eq(trx, TRX_STATE_NOT_STARTED)) { - trx_sys->n_prepared_trx++; - trx_sys->n_prepared_recovered_trx++; - } else { - ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); - } - - trx->state = TRX_STATE_PREPARED; + if (trx_state_eq(trx, TRX_STATE_NOT_STARTED)) { + trx_sys->n_prepared_trx++; + trx_sys->n_prepared_recovered_trx++; } else { - fprintf(stderr, - "InnoDB: Since innodb_force_recovery" - " > 0, we will rollback it anyway.\n"); - - trx->state = TRX_STATE_ACTIVE; + ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); } + + trx->state = TRX_STATE_PREPARED; } else { trx->state = TRX_STATE_COMMITTED_IN_MEMORY; } From db51ad1e014c3a67b35adcbb90ec6a38d32a6571 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Wed, 23 Aug 2017 18:11:24 +0000 Subject: [PATCH 08/15] Remove workaround for ancient and already fixed CMake bug in MSI creation. The workaround, an extra cmake calls, somehow makes the connect/cmake_install.cmake to lose installation of connect-engine's specific jar files. --- win/packaging/create_msi.cmake.in | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/win/packaging/create_msi.cmake.in b/win/packaging/create_msi.cmake.in index 1f847a39695..62ce5f1edd2 100644 --- a/win/packaging/create_msi.cmake.in +++ b/win/packaging/create_msi.cmake.in @@ -78,13 +78,6 @@ ELSE() ENDIF() SET(ENV{VS_UNICODE_OUTPUT}) -# Workaround for CMake bug#11452 -# Switch off the monolithic install -EXECUTE_PROCESS( - COMMAND ${CMAKE_COMMAND} -DCPACK_MONOLITHIC_INSTALL=0 ${CMAKE_BINARY_DIR} - OUTPUT_QUIET -) - INCLUDE(${CMAKE_BINARY_DIR}/CPackConfig.cmake) @@ -441,11 +434,4 @@ ENDIF() CONFIGURE_FILE(${CPACK_PACKAGE_FILE_NAME}.msi ${CMAKE_BINARY_DIR}/${CPACK_PACKAGE_FILE_NAME}.msi COPYONLY) - -# Workaround for CMake bug#11452 -# Switch monolithic install on again -EXECUTE_PROCESS( - COMMAND ${CMAKE_COMMAND} -DCPACK_MONOLITHIC_INSTALL=1 ${CMAKE_BINARY_DIR} - OUTPUT_QUIET -) From 7b36395ee962e7272d2544c61bf6142df2aba0b4 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Wed, 23 Aug 2017 23:29:59 +0000 Subject: [PATCH 09/15] MDEV-13630 : dont install connect-specific JAR files if connect is not built. --- storage/connect/CMakeLists.txt | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/storage/connect/CMakeLists.txt b/storage/connect/CMakeLists.txt index 81441892215..8df552505d1 100644 --- a/storage/connect/CMakeLists.txt +++ b/storage/connect/CMakeLists.txt @@ -266,13 +266,6 @@ IF(CONNECT_WITH_JDBC) JdbcInterface.java ApacheInterface.java MariadbInterface.java MysqlInterface.java OracleInterface.java PostgresqlInterface.java JavaWrappers.jar) - # TODO: Find how to compile and install the java wrapper classes - # Find required libraries and include directories - SET (JAVA_SOURCES JdbcInterface.java) - add_jar(JdbcInterface ${JAVA_SOURCES}) - INSTALL(FILES ${CMAKE_CURRENT_SOURCE_DIR}/JavaWrappers.jar - ${CMAKE_CURRENT_BINARY_DIR}/JdbcInterface.jar - DESTINATION ${INSTALL_PLUGINDIR} COMPONENT connect-engine) add_definitions(-DJDBC_SUPPORT) ELSE() SET(JDBC_LIBRARY "") @@ -313,3 +306,18 @@ MYSQL_ADD_PLUGIN(connect ${CONNECT_SOURCES} LINK_LIBRARIES ${ZLIB_LIBRARY} ${XML_LIBRARY} ${ICONV_LIBRARY} ${ODBC_LIBRARY} ${JDBC_LIBRARY} ${IPHLPAPI_LIBRARY}) +IF(NOT TARGET connect) + RETURN() +ENDIF() + +IF(CONNECT_WITH_JDBC AND JAVA_FOUND AND JNI_FOUND) + # TODO: Find how to compile and install the java wrapper classes + # Find required libraries and include directories + SET (JAVA_SOURCES JdbcInterface.java) + add_jar(JdbcInterface ${JAVA_SOURCES}) + INSTALL(FILES + ${CMAKE_CURRENT_SOURCE_DIR}/JavaWrappers.jar + ${CMAKE_CURRENT_BINARY_DIR}/JdbcInterface.jar + DESTINATION ${INSTALL_PLUGINDIR} COMPONENT connect-engine) +ENDIF() + From 7aa846e9e30293e0bac7613abfd254405cfe9e09 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Wed, 23 Aug 2017 23:30:51 +0000 Subject: [PATCH 10/15] CONNECT engine: install ha_connect.lib --- storage/connect/CMakeLists.txt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/storage/connect/CMakeLists.txt b/storage/connect/CMakeLists.txt index 8df552505d1..9f42b7a87dd 100644 --- a/storage/connect/CMakeLists.txt +++ b/storage/connect/CMakeLists.txt @@ -310,12 +310,25 @@ IF(NOT TARGET connect) RETURN() ENDIF() +# Install some extra files that belong to connect engine +IF(WIN32) + # install ha_connect.lib + GET_TARGET_PROPERTY(CONNECT_LOCATION connect LOCATION) + STRING(REPLACE "dll" "lib" CONNECT_LIB ${CONNECT_LOCATION}) + IF(CMAKE_CONFIGURATION_TYPES) + STRING(REPLACE "${CMAKE_CFG_INTDIR}" "\${CMAKE_INSTALL_CONFIG_NAME}" + CONNECT_LIB ${CONNECT_LIB}) + ENDIF() + INSTALL(FILES ${CONNECT_LIB} + DESTINATION ${INSTALL_PLUGINDIR} COMPONENT connect-engine) +ENDIF(WIN32) + IF(CONNECT_WITH_JDBC AND JAVA_FOUND AND JNI_FOUND) # TODO: Find how to compile and install the java wrapper classes # Find required libraries and include directories SET (JAVA_SOURCES JdbcInterface.java) add_jar(JdbcInterface ${JAVA_SOURCES}) - INSTALL(FILES + INSTALL(FILES ${CMAKE_CURRENT_SOURCE_DIR}/JavaWrappers.jar ${CMAKE_CURRENT_BINARY_DIR}/JdbcInterface.jar DESTINATION ${INSTALL_PLUGINDIR} COMPONENT connect-engine) From dd229430a977e07b7bd65c1513b266f6cb8333f3 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 24 Aug 2017 08:05:11 +0000 Subject: [PATCH 11/15] Windows compile : make compilation fail on "uninitialized variable used" warning C4700 This is a genuine error, and will crash debug buildd in runtime checks if not fixed. it is better to fail during compile. --- cmake/os/Windows.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/os/Windows.cmake b/cmake/os/Windows.cmake index 84276115723..e4221227d91 100644 --- a/cmake/os/Windows.cmake +++ b/cmake/os/Windows.cmake @@ -115,8 +115,8 @@ IF(MSVC) ENDIF() #TODO: update the code and remove the disabled warnings - SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /wd4800 /wd4805 /wd4996") - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4800 /wd4805 /wd4996 /wd4291 /wd4577 /we4099") + SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /wd4800 /wd4805 /wd4996 /we4700") + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4800 /wd4805 /wd4996 /wd4291 /wd4577 /we4099 /we4700") IF(CMAKE_SIZEOF_VOID_P MATCHES 8) # _WIN64 is defined by the compiler itself. From cd35dd6a0546c31449d1c4c43bc03fe351d0dcf4 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 24 Aug 2017 15:49:50 +0000 Subject: [PATCH 12/15] Windows : Do not use CRT routine to dump memory leaks. Its output is useless,and, in case of large output, it also may prevent with search_pattern_in_file.inc from working. --- mysys/my_init.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mysys/my_init.c b/mysys/my_init.c index dee41e1202a..a0f8e21322e 100644 --- a/mysys/my_init.c +++ b/mysys/my_init.c @@ -200,7 +200,6 @@ Voluntary context switches %ld, Involuntary context switches %ld\n", _CrtSetReportMode( _CRT_ASSERT, _CRTDBG_MODE_FILE ); _CrtSetReportFile( _CRT_ASSERT, _CRTDBG_FILE_STDERR ); _CrtCheckMemory(); - _CrtDumpMemoryLeaks(); #endif } From 582545a38421ea0e43f326db84550773411c3261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 24 Aug 2017 15:38:05 +0300 Subject: [PATCH 13/15] MDEV-13637 InnoDB change buffer housekeeping can cause redo log overrun and possibly deadlocks The function ibuf_remove_free_page() may be called while the caller is holding several mutexes or rw-locks. Because of this, this housekeeping loop may cause performance glitches for operations that involve tables that are stored in the InnoDB system tablespace. Also deadlocks might be possible. The worst impact of all is that due to the mutexes being held, calls to log_free_check() had to be skipped during this housekeeping. This means that the cyclic InnoDB redo log may be overwritten. If the system crashes during this, it would be unable to recover. The entry point to the problematic code is ibuf_free_excess_pages(). It would make sense to call it before acquiring any mutexes or rw-locks, in any 'pessimistic' operation that involves the system tablespace. fseg_create_general(), fseg_alloc_free_page_general(): Do not call ibuf_free_excess_pages() while potentially holding some latches. ibuf_remove_free_page(): Do call log_free_check(), like every operation that is about to generate redo log should do. ibuf_free_excess_pages(): Remove some assertions that are replaced by stricter assertions in the log_free_check() that is now called by ibuf_remove_free_page(). row_ins_sec_index_entry(), row_undo_ins_remove_sec_low(), row_undo_mod_del_mark_or_remove_sec_low(), row_undo_mod_del_unmark_sec_and_undo_update(): Call ibuf_free_excess_pages() if the operation may involve allocating pages and change buffering in the system tablespace. --- storage/innobase/fsp/fsp0fsp.cc | 19 +------------------ storage/innobase/ibuf/ibuf0ibuf.cc | 19 +++---------------- storage/innobase/row/row0ins.cc | 6 ++++++ storage/innobase/row/row0uins.cc | 5 +++++ storage/innobase/row/row0umod.cc | 12 ++++++++++++ storage/xtradb/fsp/fsp0fsp.cc | 19 +------------------ storage/xtradb/ibuf/ibuf0ibuf.cc | 19 +++---------------- storage/xtradb/row/row0ins.cc | 6 ++++++ storage/xtradb/row/row0uins.cc | 5 +++++ storage/xtradb/row/row0umod.cc | 12 ++++++++++++ 10 files changed, 54 insertions(+), 68 deletions(-) diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/fsp0fsp.cc index d04cd7168ed..66f4ada8485 100644 --- a/storage/innobase/fsp/fsp0fsp.cc +++ b/storage/innobase/fsp/fsp0fsp.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -2032,15 +2033,6 @@ fseg_create_general( mtr_x_lock(latch, mtr); - if (rw_lock_get_x_lock_count(latch) == 1) { - /* This thread did not own the latch before this call: free - excess pages from the insert buffer free list */ - - if (space == IBUF_SPACE_ID) { - ibuf_free_excess_pages(); - } - } - if (!has_done_reservation) { success = fsp_reserve_free_extents(&n_reserved, space, 2, FSP_NORMAL, mtr); @@ -2612,15 +2604,6 @@ fseg_alloc_free_page_general( mtr_x_lock(latch, mtr); - if (rw_lock_get_x_lock_count(latch) == 1) { - /* This thread did not own the latch before this call: free - excess pages from the insert buffer free list */ - - if (space == IBUF_SPACE_ID) { - ibuf_free_excess_pages(); - } - } - inode = fseg_inode_get(seg_header, space, zip_size, mtr); if (!has_done_reservation diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc index 49c1d1a4227..99be552c967 100644 --- a/storage/innobase/ibuf/ibuf0ibuf.cc +++ b/storage/innobase/ibuf/ibuf0ibuf.cc @@ -2147,6 +2147,8 @@ ibuf_remove_free_page(void) page_t* root; page_t* bitmap_page; + log_free_check(); + mtr_start(&mtr); /* Acquire the fsp latch before the ibuf header, obeying the latching @@ -2258,22 +2260,7 @@ ibuf_free_excess_pages(void) { ulint i; -#ifdef UNIV_SYNC_DEBUG - ut_ad(rw_lock_own(fil_space_get_latch(IBUF_SPACE_ID, NULL), - RW_LOCK_EX)); -#endif /* UNIV_SYNC_DEBUG */ - - ut_ad(rw_lock_get_x_lock_count( - fil_space_get_latch(IBUF_SPACE_ID, NULL)) == 1); - - /* NOTE: We require that the thread did not own the latch before, - because then we know that we can obey the correct latching order - for ibuf latches */ - - if (!ibuf) { - /* Not yet initialized; not sure if this is possible, but - does no harm to check for it. */ - + if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { return; } diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index 5cfa21c1fa2..d2e9e8f1940 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -38,6 +38,7 @@ Created 4/20/1996 Heikki Tuuri #include "btr0btr.h" #include "btr0cur.h" #include "mach0data.h" +#include "ibuf0ibuf.h" #include "que0que.h" #include "row0upd.h" #include "row0sel.h" @@ -2940,6 +2941,11 @@ row_ins_sec_index_entry( if (err == DB_FAIL) { mem_heap_empty(heap); + if (index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } + /* Try then pessimistic descent to the B-tree */ log_free_check(); diff --git a/storage/innobase/row/row0uins.cc b/storage/innobase/row/row0uins.cc index 651042fb820..a9733f435b7 100644 --- a/storage/innobase/row/row0uins.cc +++ b/storage/innobase/row/row0uins.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -201,6 +202,10 @@ row_undo_ins_remove_sec_low( mtr_s_lock(dict_index_get_lock(index), &mtr); } else { ut_ad(mode == BTR_MODIFY_TREE); + if (index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } mtr_x_lock(dict_index_get_lock(index), &mtr); } diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc index 2e66bc494d4..77fa3cada2b 100644 --- a/storage/innobase/row/row0umod.cc +++ b/storage/innobase/row/row0umod.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -35,6 +36,7 @@ Created 2/27/1997 Heikki Tuuri #include "trx0roll.h" #include "btr0btr.h" #include "mach0data.h" +#include "ibuf0ibuf.h" #include "row0undo.h" #include "row0vers.h" #include "row0log.h" @@ -432,6 +434,11 @@ row_undo_mod_del_mark_or_remove_sec_low( log_free_check(); mtr_start_trx(&mtr, thr_get_trx(thr)); + if (mode == BTR_MODIFY_TREE + && index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } if (*index->name == TEMP_INDEX_PREFIX) { /* The index->online_status may change if the @@ -604,6 +611,11 @@ row_undo_mod_del_unmark_sec_and_undo_update( log_free_check(); mtr_start_trx(&mtr, thr_get_trx(thr)); + if (mode == BTR_MODIFY_TREE + && index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } if (*index->name == TEMP_INDEX_PREFIX) { /* The index->online_status may change if the diff --git a/storage/xtradb/fsp/fsp0fsp.cc b/storage/xtradb/fsp/fsp0fsp.cc index cf6fa4211ca..d441d710671 100644 --- a/storage/xtradb/fsp/fsp0fsp.cc +++ b/storage/xtradb/fsp/fsp0fsp.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -2041,15 +2042,6 @@ fseg_create_general( mtr_x_lock(latch, mtr); - if (rw_lock_get_x_lock_count(latch) == 1) { - /* This thread did not own the latch before this call: free - excess pages from the insert buffer free list */ - - if (space == IBUF_SPACE_ID) { - ibuf_free_excess_pages(); - } - } - if (!has_done_reservation) { success = fsp_reserve_free_extents(&n_reserved, space, 2, FSP_NORMAL, mtr); @@ -2621,15 +2613,6 @@ fseg_alloc_free_page_general( mtr_x_lock(latch, mtr); - if (rw_lock_get_x_lock_count(latch) == 1) { - /* This thread did not own the latch before this call: free - excess pages from the insert buffer free list */ - - if (space == IBUF_SPACE_ID) { - ibuf_free_excess_pages(); - } - } - inode = fseg_inode_get(seg_header, space, zip_size, mtr); if (!has_done_reservation diff --git a/storage/xtradb/ibuf/ibuf0ibuf.cc b/storage/xtradb/ibuf/ibuf0ibuf.cc index 414d7fc39be..d542e488dba 100644 --- a/storage/xtradb/ibuf/ibuf0ibuf.cc +++ b/storage/xtradb/ibuf/ibuf0ibuf.cc @@ -2188,6 +2188,8 @@ ibuf_remove_free_page(void) page_t* root; page_t* bitmap_page; + log_free_check(); + mtr_start(&mtr); /* Acquire the fsp latch before the ibuf header, obeying the latching @@ -2299,22 +2301,7 @@ ibuf_free_excess_pages(void) { ulint i; -#ifdef UNIV_SYNC_DEBUG - ut_ad(rw_lock_own(fil_space_get_latch(IBUF_SPACE_ID, NULL), - RW_LOCK_EX)); -#endif /* UNIV_SYNC_DEBUG */ - - ut_ad(rw_lock_get_x_lock_count( - fil_space_get_latch(IBUF_SPACE_ID, NULL)) == 1); - - /* NOTE: We require that the thread did not own the latch before, - because then we know that we can obey the correct latching order - for ibuf latches */ - - if (!ibuf) { - /* Not yet initialized; not sure if this is possible, but - does no harm to check for it. */ - + if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { return; } diff --git a/storage/xtradb/row/row0ins.cc b/storage/xtradb/row/row0ins.cc index ec9f74cd4ad..20a68e7eca2 100644 --- a/storage/xtradb/row/row0ins.cc +++ b/storage/xtradb/row/row0ins.cc @@ -38,6 +38,7 @@ Created 4/20/1996 Heikki Tuuri #include "btr0btr.h" #include "btr0cur.h" #include "mach0data.h" +#include "ibuf0ibuf.h" #include "que0que.h" #include "row0upd.h" #include "row0sel.h" @@ -3013,6 +3014,11 @@ row_ins_sec_index_entry( if (err == DB_FAIL) { mem_heap_empty(heap); + if (index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } + /* Try then pessimistic descent to the B-tree */ log_free_check(); diff --git a/storage/xtradb/row/row0uins.cc b/storage/xtradb/row/row0uins.cc index 651042fb820..a9733f435b7 100644 --- a/storage/xtradb/row/row0uins.cc +++ b/storage/xtradb/row/row0uins.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -201,6 +202,10 @@ row_undo_ins_remove_sec_low( mtr_s_lock(dict_index_get_lock(index), &mtr); } else { ut_ad(mode == BTR_MODIFY_TREE); + if (index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } mtr_x_lock(dict_index_get_lock(index), &mtr); } diff --git a/storage/xtradb/row/row0umod.cc b/storage/xtradb/row/row0umod.cc index c8a229ea640..bd0a36e2240 100644 --- a/storage/xtradb/row/row0umod.cc +++ b/storage/xtradb/row/row0umod.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -35,6 +36,7 @@ Created 2/27/1997 Heikki Tuuri #include "trx0roll.h" #include "btr0btr.h" #include "mach0data.h" +#include "ibuf0ibuf.h" #include "row0undo.h" #include "row0vers.h" #include "row0log.h" @@ -402,6 +404,11 @@ row_undo_mod_del_mark_or_remove_sec_low( log_free_check(); mtr_start_trx(&mtr, thr_get_trx(thr)); + if (mode == BTR_MODIFY_TREE + && index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } if (*index->name == TEMP_INDEX_PREFIX) { /* The index->online_status may change if the @@ -574,6 +581,11 @@ row_undo_mod_del_unmark_sec_and_undo_update( log_free_check(); mtr_start_trx(&mtr, thr_get_trx(thr)); + if (mode == BTR_MODIFY_TREE + && index->space == IBUF_SPACE_ID + && !dict_index_is_unique(index)) { + ibuf_free_excess_pages(); + } if (*index->name == TEMP_INDEX_PREFIX) { /* The index->online_status may change if the From 882f4566e55694b4f68f61ee723678b49d7e0d8f Mon Sep 17 00:00:00 2001 From: Elena Stepanova Date: Sat, 19 Aug 2017 02:52:35 +0300 Subject: [PATCH 14/15] Combinations with innodb-undo-tablespaces to use in test files --- mysql-test/include/innodb_undo_tablespaces.combinations | 5 +++++ mysql-test/include/innodb_undo_tablespaces.inc | 3 +++ 2 files changed, 8 insertions(+) create mode 100644 mysql-test/include/innodb_undo_tablespaces.combinations create mode 100644 mysql-test/include/innodb_undo_tablespaces.inc diff --git a/mysql-test/include/innodb_undo_tablespaces.combinations b/mysql-test/include/innodb_undo_tablespaces.combinations new file mode 100644 index 00000000000..0ac7a6e2d05 --- /dev/null +++ b/mysql-test/include/innodb_undo_tablespaces.combinations @@ -0,0 +1,5 @@ +[undo0] +innodb-undo-tablespaces=0 + +[undo3] +innodb-undo-tablespaces=3 diff --git a/mysql-test/include/innodb_undo_tablespaces.inc b/mysql-test/include/innodb_undo_tablespaces.inc new file mode 100644 index 00000000000..b109fcc4f0e --- /dev/null +++ b/mysql-test/include/innodb_undo_tablespaces.inc @@ -0,0 +1,3 @@ +# The goal of including this file is to enable innodb_undo_tablespaces combinations +# (see include/innodb_undo_tablespaces.combinations) + From 61096ff214dec68a66efb811aa3a70ef49f06a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Mon, 28 Aug 2017 09:45:54 +0300 Subject: [PATCH 15/15] MDEV-13591: InnoDB: Database page corruption on disk or a failed file read and assertion failure Problem is that page 0 and its possible enrryption information is not read for undo tablespaces. fil_crypt_get_latest_key_version(): Do not send event to encryption threads if event does not yet exists. Seen on regression testing. fil_read_first_page: Add new parameter does page belong to undo tablespace and if it does, we do not read FSP_HEADER. srv_undo_tablespace_open : Read first page of the tablespace to get crypt_data if it exists and pass it to fil_space_create. Tested using innodb_encryption with combinations with innodb-undo-tablespaces. --- .../encryption/r/innodb_encryption.result | 2 + .../suite/encryption/t/innodb_encryption.test | 4 ++ storage/innobase/fil/fil0crypt.cc | 7 ++- storage/innobase/fil/fil0fil.cc | 49 +++++++++++-------- storage/innobase/include/fil0fil.h | 5 +- storage/innobase/srv/srv0start.cc | 29 ++++++++--- storage/xtradb/fil/fil0crypt.cc | 7 ++- storage/xtradb/fil/fil0fil.cc | 49 +++++++++++-------- storage/xtradb/include/fil0fil.h | 9 ++-- storage/xtradb/srv/srv0start.cc | 29 ++++++++--- 10 files changed, 130 insertions(+), 60 deletions(-) diff --git a/mysql-test/suite/encryption/r/innodb_encryption.result b/mysql-test/suite/encryption/r/innodb_encryption.result index 26c77499d25..ce494098d44 100644 --- a/mysql-test/suite/encryption/r/innodb_encryption.result +++ b/mysql-test/suite/encryption/r/innodb_encryption.result @@ -1,3 +1,5 @@ +call mtr.add_suppression("InnoDB: New log files created"); +call mtr.add_suppression("InnoDB: Creating foreign key constraint system tables."); SET @start_global_value = @@global.innodb_encryption_threads; SHOW VARIABLES LIKE 'innodb_encrypt%'; Variable_name Value diff --git a/mysql-test/suite/encryption/t/innodb_encryption.test b/mysql-test/suite/encryption/t/innodb_encryption.test index 50aca2a7260..6e9f80aac0c 100644 --- a/mysql-test/suite/encryption/t/innodb_encryption.test +++ b/mysql-test/suite/encryption/t/innodb_encryption.test @@ -3,10 +3,14 @@ # -- source include/have_innodb.inc -- source include/have_example_key_management_plugin.inc +-- source include/innodb_undo_tablespaces.inc # embedded does not support restart -- source include/not_embedded.inc +call mtr.add_suppression("InnoDB: New log files created"); +call mtr.add_suppression("InnoDB: Creating foreign key constraint system tables."); + SET @start_global_value = @@global.innodb_encryption_threads; SHOW VARIABLES LIKE 'innodb_encrypt%'; diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 683974d8920..ef39aabb8c2 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -191,7 +191,12 @@ fil_crypt_get_latest_key_version( crypt_data->min_key_version, key_version, srv_fil_crypt_rotate_key_age)) { - os_event_set(fil_crypt_threads_event); + /* Below event seen as NULL-pointer at startup + when new database was created and we create a + checkpoint. Only seen when debugging. */ + if (fil_crypt_threads_inited) { + os_event_set(fil_crypt_threads_event); + } } } diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 6abc7f0e1c2..559ba26542f 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -2322,8 +2322,10 @@ the first page of a first data file at database startup. data files @param[out] flushed_lsn flushed lsn value @param[out] crypt_data encryption crypt data -@retval NULL on success, or if innodb_force_recovery is set -@return pointer to an error message string */ +@param[in] check_first_page true if first page contents + should be checked +@return NULL on success, or if innodb_force_recovery is set +@retval pointer to an error message string */ UNIV_INTERN const char* fil_read_first_page( @@ -2336,7 +2338,8 @@ fil_read_first_page( ulint* max_arch_log_no, #endif /* UNIV_LOG_ARCHIVE */ lsn_t* flushed_lsn, - fil_space_crypt_t** crypt_data) + fil_space_crypt_t** crypt_data, + bool check_first_page) { byte* buf; byte* page; @@ -2358,27 +2361,31 @@ fil_read_first_page( *flags and *space_id as they were read from the first file and do not validate the first page. */ if (!one_read_already) { - *space_id = fsp_header_get_space_id(page); - *flags = fsp_header_get_flags(page); + /* Undo tablespace does not contain correct FSP_HEADER, + and actually we really need to read only crypt_data. */ + if (check_first_page) { + *space_id = fsp_header_get_space_id(page); + *flags = fsp_header_get_flags(page); - if (flushed_lsn) { - *flushed_lsn = mach_read_from_8(page + - FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - } - - if (!fsp_flags_is_valid(*flags, *space_id)) { - ulint cflags = fsp_flags_convert_from_101(*flags); - if (cflags == ULINT_UNDEFINED) { - ib_logf(IB_LOG_LEVEL_ERROR, - "Invalid flags 0x%x in tablespace %u", - unsigned(*flags), unsigned(*space_id)); - return "invalid tablespace flags"; - } else { - *flags = cflags; + if (flushed_lsn) { + *flushed_lsn = mach_read_from_8(page + + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); } - } - check_msg = fil_check_first_page(page, *space_id, *flags); + if (!fsp_flags_is_valid(*flags, *space_id)) { + ulint cflags = fsp_flags_convert_from_101(*flags); + if (cflags == ULINT_UNDEFINED) { + ib_logf(IB_LOG_LEVEL_ERROR, + "Invalid flags 0x%x in tablespace %u", + unsigned(*flags), unsigned(*space_id)); + return "invalid tablespace flags"; + } else { + *flags = cflags; + } + } + + check_msg = fil_check_first_page(page, *space_id, *flags); + } /* Possible encryption crypt data is also stored only to first page of the first datafile. */ diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h index e481ea3a8aa..38250949380 100644 --- a/storage/innobase/include/fil0fil.h +++ b/storage/innobase/include/fil0fil.h @@ -802,6 +802,8 @@ the first page of a first data file at database startup. data files @param[out] flushed_lsn flushed lsn value @param[out] crypt_data encryption crypt data +@param[in] check_first_page true if first page contents + should be checked @retval NULL on success, or if innodb_force_recovery is set @return pointer to an error message string */ UNIV_INTERN @@ -816,7 +818,8 @@ fil_read_first_page( ulint* max_arch_log_no, #endif /* UNIV_LOG_ARCHIVE */ lsn_t* flushed_lsn, - fil_space_crypt_t** crypt_data) + fil_space_crypt_t** crypt_data, + bool check_first_page=true) MY_ATTRIBUTE((warn_unused_result)); #endif /* !UNIV_HOTBACKUP */ diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 0880f93c7fa..703ab0c9dca 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -850,7 +850,7 @@ open_or_create_data_files( ibool one_created = FALSE; os_offset_t size; ulint flags; - ulint space; + ulint space=0; ulint rounded_size_pages; char name[10000]; fil_space_crypt_t* crypt_data=NULL; @@ -1332,11 +1332,30 @@ srv_undo_tablespace_open( size = os_file_get_size(fh); ut_a(size != (os_offset_t) -1); + /* Load the tablespace into InnoDB's internal + data structures. */ + + const char* check_msg; + fil_space_crypt_t* crypt_data = NULL; + + /* Set the compressed page size to 0 (non-compressed) */ + flags = FSP_FLAGS_PAGE_SSIZE(); + + /* Read first page to find out does the crypt_info + exists on undo tablespace. */ + check_msg = fil_read_first_page( + fh, FALSE, &flags, &space, + NULL, &crypt_data, false); + ret = os_file_close(fh); ut_a(ret); - /* Load the tablespace into InnoDB's internal - data structures. */ + if (check_msg) { + ib_logf(IB_LOG_LEVEL_ERROR, + "%s in data file %s", + check_msg, name); + return (err); + } /* We set the biggest space id to the undo tablespace because InnoDB hasn't opened any other tablespace apart @@ -1344,10 +1363,8 @@ srv_undo_tablespace_open( fil_set_max_space_id_if_bigger(space); - /* Set the compressed page size to 0 (non-compressed) */ - flags = FSP_FLAGS_PAGE_SSIZE(); fil_space_create(name, space, flags, FIL_TABLESPACE, - NULL /* no encryption */, + crypt_data, true /* create */); ut_a(fil_validate()); diff --git a/storage/xtradb/fil/fil0crypt.cc b/storage/xtradb/fil/fil0crypt.cc index 683974d8920..ef39aabb8c2 100644 --- a/storage/xtradb/fil/fil0crypt.cc +++ b/storage/xtradb/fil/fil0crypt.cc @@ -191,7 +191,12 @@ fil_crypt_get_latest_key_version( crypt_data->min_key_version, key_version, srv_fil_crypt_rotate_key_age)) { - os_event_set(fil_crypt_threads_event); + /* Below event seen as NULL-pointer at startup + when new database was created and we create a + checkpoint. Only seen when debugging. */ + if (fil_crypt_threads_inited) { + os_event_set(fil_crypt_threads_event); + } } } diff --git a/storage/xtradb/fil/fil0fil.cc b/storage/xtradb/fil/fil0fil.cc index dbf6501b183..2c3d26d6302 100644 --- a/storage/xtradb/fil/fil0fil.cc +++ b/storage/xtradb/fil/fil0fil.cc @@ -2372,8 +2372,10 @@ the first page of a first data file at database startup. @param[out] space_id tablepspace ID @param[out] flushed_lsn flushed lsn value @param[out] crypt_data encryption crypt data -@retval NULL on success, or if innodb_force_recovery is set -@return pointer to an error message string */ +@param[in] check_first_page true if first page contents + should be checked +@return NULL on success, or if innodb_force_recovery is set +@retval pointer to an error message string */ UNIV_INTERN const char* fil_read_first_page( @@ -2382,7 +2384,8 @@ fil_read_first_page( ulint* flags, ulint* space_id, lsn_t* flushed_lsn, - fil_space_crypt_t** crypt_data) + fil_space_crypt_t** crypt_data, + bool check_first_page) { byte* buf; byte* page; @@ -2418,28 +2421,32 @@ fil_read_first_page( *flags and *space_id as they were read from the first file and do not validate the first page. */ if (!one_read_already) { - *space_id = fsp_header_get_space_id(page); - *flags = fsp_header_get_flags(page); + /* Undo tablespace does not contain correct FSP_HEADER, + and actually we really need to read only crypt_data. */ + if (check_first_page) { + *space_id = fsp_header_get_space_id(page); + *flags = fsp_header_get_flags(page); - if (flushed_lsn) { - *flushed_lsn = mach_read_from_8(page + + if (flushed_lsn) { + *flushed_lsn = mach_read_from_8(page + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - } - - if (!fsp_flags_is_valid(*flags, *space_id)) { - ulint cflags = fsp_flags_convert_from_101(*flags); - if (cflags == ULINT_UNDEFINED) { - ib_logf(IB_LOG_LEVEL_ERROR, - "Invalid flags 0x%x in tablespace %u", - unsigned(*flags), unsigned(*space_id)); - return "invalid tablespace flags"; - } else { - *flags = cflags; } - } - if (!(IS_XTRABACKUP() && srv_backup_mode)) { - check_msg = fil_check_first_page(page, *space_id, *flags); + if (!fsp_flags_is_valid(*flags, *space_id)) { + ulint cflags = fsp_flags_convert_from_101(*flags); + if (cflags == ULINT_UNDEFINED) { + ib_logf(IB_LOG_LEVEL_ERROR, + "Invalid flags 0x%x in tablespace %u", + unsigned(*flags), unsigned(*space_id)); + return "invalid tablespace flags"; + } else { + *flags = cflags; + } + } + + if (!(IS_XTRABACKUP() && srv_backup_mode)) { + check_msg = fil_check_first_page(page, *space_id, *flags); + } } /* Possible encryption crypt data is also stored only to first page diff --git a/storage/xtradb/include/fil0fil.h b/storage/xtradb/include/fil0fil.h index 6eab5db6883..a33cec65ed5 100644 --- a/storage/xtradb/include/fil0fil.h +++ b/storage/xtradb/include/fil0fil.h @@ -804,8 +804,10 @@ the first page of a first data file at database startup. @param[out] space_id tablepspace ID @param[out] flushed_lsn flushed lsn value @param[out] crypt_data encryption crypt data -@retval NULL on success, or if innodb_force_recovery is set -@return pointer to an error message string */ +@param[in] check_first_page true if first page contents + should be checked +@return NULL on success, or if innodb_force_recovery is set +@retval pointer to an error message string */ UNIV_INTERN const char* fil_read_first_page( @@ -814,7 +816,8 @@ fil_read_first_page( ulint* flags, ulint* space_id, lsn_t* flushed_lsn, - fil_space_crypt_t** crypt_data) + fil_space_crypt_t** crypt_data, + bool check_first_page=true) MY_ATTRIBUTE((warn_unused_result)); #endif /* !UNIV_HOTBACKUP */ diff --git a/storage/xtradb/srv/srv0start.cc b/storage/xtradb/srv/srv0start.cc index aab0bc9282b..7bcb0b1b1ce 100644 --- a/storage/xtradb/srv/srv0start.cc +++ b/storage/xtradb/srv/srv0start.cc @@ -891,7 +891,7 @@ open_or_create_data_files( bool one_created = false; os_offset_t size; ulint flags; - ulint space; + ulint space = 0; ulint rounded_size_pages; char name[10000]; fil_space_crypt_t* crypt_data=NULL; @@ -1369,11 +1369,30 @@ srv_undo_tablespace_open( size = os_file_get_size(fh); ut_a(size != (os_offset_t) -1); + /* Load the tablespace into InnoDB's internal + data structures. */ + + const char* check_msg; + fil_space_crypt_t* crypt_data = NULL; + + /* Set the compressed page size to 0 (non-compressed) */ + flags = FSP_FLAGS_PAGE_SSIZE(); + + /* Read first page to find out does the crypt_info + exists on undo tablespace. */ + check_msg = fil_read_first_page( + fh, FALSE, &flags, &space, + NULL, &crypt_data, false); + ret = os_file_close(fh); ut_a(ret); - /* Load the tablespace into InnoDB's internal - data structures. */ + if (check_msg) { + ib_logf(IB_LOG_LEVEL_ERROR, + "%s in data file %s", + check_msg, name); + return (err); + } /* We set the biggest space id to the undo tablespace because InnoDB hasn't opened any other tablespace apart @@ -1381,10 +1400,8 @@ srv_undo_tablespace_open( fil_set_max_space_id_if_bigger(space); - /* Set the compressed page size to 0 (non-compressed) */ - flags = FSP_FLAGS_PAGE_SSIZE(); fil_space_create(name, space, flags, FIL_TABLESPACE, - NULL /* no encryption */, + crypt_data, true /* create */); ut_a(fil_validate());