From ea70586282d902fe067cee11a5aeb7086ed375f8 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 7 Jun 2018 15:12:26 +0100 Subject: [PATCH 01/15] MDEV-16300 : remove rocksdb checkpoint created by mariabackup. Add variable rocksdb_remove_mariabackup_checkpoint. If set, it will remove $rocksdb_datadir/mariabackup-checkpoint directory. The variable is to be used by exclusively by mariabackup, to remove temporary checkpoints. --- storage/rocksdb/ha_rocksdb.cc | 69 ++++++++++++++++++- .../mysql-test/rocksdb/r/rocksdb.result | 1 + ...remove_mariabackup_checkpoint_basic.result | 4 ++ ...b_remove_mariabackup_checkpoint_basic.test | 5 ++ 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 storage/rocksdb/mysql-test/rocksdb_sys_vars/r/rocksdb_remove_mariabackup_checkpoint_basic.result create mode 100644 storage/rocksdb/mysql-test/rocksdb_sys_vars/t/rocksdb_remove_mariabackup_checkpoint_basic.test diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 96bc7dad3e6..7cddfea33a7 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -479,6 +479,7 @@ static uint32_t rocksdb_access_hint_on_compaction_start; static char *rocksdb_compact_cf_name; static char *rocksdb_checkpoint_name; static my_bool rocksdb_signal_drop_index_thread; +static my_bool rocksdb_signal_remove_mariabackup_checkpoint; static my_bool rocksdb_strict_collation_check = 1; static my_bool rocksdb_ignore_unknown_options = 1; static my_bool rocksdb_enable_2pc = 0; @@ -515,6 +516,67 @@ std::atomic rocksdb_row_lock_wait_timeouts(0); std::atomic rocksdb_snapshot_conflict_errors(0); std::atomic rocksdb_wal_group_syncs(0); + + +/* + Remove directory with files in it. + Used to remove checkpoint created by mariabackup. +*/ +#ifdef _WIN32 +#include /* unlink*/ +#ifndef F_OK +#define F_OK 0 +#endif +#endif + +static int rmdir_force(const char *dir) { + if (access(dir, F_OK)) + return true; + + char path[FN_REFLEN]; + char sep[] = {FN_LIBCHAR, 0}; + int err = 0; + + MY_DIR *dir_info = my_dir(dir, MYF(MY_DONT_SORT | MY_WANT_STAT)); + if (!dir_info) + return 1; + + for (uint i = 0; i < dir_info->number_of_files; i++) { + FILEINFO *file = dir_info->dir_entry + i; + + strxnmov(path, sizeof(path), dir, sep, file->name, NULL); + + err = my_delete(path, 0); + + if (err) { + break; + } + } + + my_dirend(dir_info); + + if (!err) + err = rmdir(dir); + + return (err == 0) ? HA_EXIT_SUCCESS : HA_EXIT_FAILURE; +} + + +static void rocksdb_remove_mariabackup_checkpoint( + my_core::THD *const, + struct st_mysql_sys_var *const , + void *const var_ptr, const void *const) { + std::string mariabackup_checkpoint_dir(rocksdb_datadir); + + mariabackup_checkpoint_dir.append("/mariabackup-checkpoint"); + + if (unlink(mariabackup_checkpoint_dir.c_str()) == 0) + return; + + rmdir_force(mariabackup_checkpoint_dir.c_str()); +} + + static std::unique_ptr rdb_init_rocksdb_db_options(void) { auto o = std::unique_ptr(new rocksdb::DBOptions()); @@ -1312,6 +1374,11 @@ static MYSQL_SYSVAR_STR(create_checkpoint, rocksdb_checkpoint_name, rocksdb_create_checkpoint, rocksdb_create_checkpoint_stub, ""); +static MYSQL_SYSVAR_BOOL(remove_mariabackup_checkpoint, + rocksdb_signal_remove_mariabackup_checkpoint, + PLUGIN_VAR_RQCMDARG, "Remove mariabackup checkpoint", + nullptr, rocksdb_remove_mariabackup_checkpoint, FALSE); + static MYSQL_SYSVAR_BOOL(signal_drop_index_thread, rocksdb_signal_drop_index_thread, PLUGIN_VAR_RQCMDARG, "Wake up drop index thread", nullptr, @@ -1675,7 +1742,7 @@ static struct st_mysql_sys_var *rocksdb_system_variables[] = { MYSQL_SYSVAR(datadir), MYSQL_SYSVAR(supported_compression_types), MYSQL_SYSVAR(create_checkpoint), - + MYSQL_SYSVAR(remove_mariabackup_checkpoint), MYSQL_SYSVAR(checksums_pct), MYSQL_SYSVAR(store_row_debug_checksums), MYSQL_SYSVAR(verify_row_debug_checksums), diff --git a/storage/rocksdb/mysql-test/rocksdb/r/rocksdb.result b/storage/rocksdb/mysql-test/rocksdb/r/rocksdb.result index 6138dac92e5..dc13bb375b8 100644 --- a/storage/rocksdb/mysql-test/rocksdb/r/rocksdb.result +++ b/storage/rocksdb/mysql-test/rocksdb/r/rocksdb.result @@ -959,6 +959,7 @@ rocksdb_print_snapshot_conflict_queries OFF rocksdb_rate_limiter_bytes_per_sec 0 rocksdb_read_free_rpl_tables rocksdb_records_in_range 50 +rocksdb_remove_mariabackup_checkpoint OFF rocksdb_reset_stats OFF rocksdb_seconds_between_stat_computes 3600 rocksdb_signal_drop_index_thread OFF diff --git a/storage/rocksdb/mysql-test/rocksdb_sys_vars/r/rocksdb_remove_mariabackup_checkpoint_basic.result b/storage/rocksdb/mysql-test/rocksdb_sys_vars/r/rocksdb_remove_mariabackup_checkpoint_basic.result new file mode 100644 index 00000000000..01145cd2ab8 --- /dev/null +++ b/storage/rocksdb/mysql-test/rocksdb_sys_vars/r/rocksdb_remove_mariabackup_checkpoint_basic.result @@ -0,0 +1,4 @@ +SET GLOBAL rocksdb_create_checkpoint=CONCAT(@@rocksdb_datadir,'/mariabackup-checkpoint'); +SET GLOBAL rocksdb_remove_mariabackup_checkpoint=ON; +SET GLOBAL rocksdb_create_checkpoint=CONCAT(@@rocksdb_datadir,'/mariabackup-checkpoint'); +SET GLOBAL rocksdb_remove_mariabackup_checkpoint=ON; diff --git a/storage/rocksdb/mysql-test/rocksdb_sys_vars/t/rocksdb_remove_mariabackup_checkpoint_basic.test b/storage/rocksdb/mysql-test/rocksdb_sys_vars/t/rocksdb_remove_mariabackup_checkpoint_basic.test new file mode 100644 index 00000000000..30f38283ba4 --- /dev/null +++ b/storage/rocksdb/mysql-test/rocksdb_sys_vars/t/rocksdb_remove_mariabackup_checkpoint_basic.test @@ -0,0 +1,5 @@ +# Simulate creating and removing mariabackup checkpoint twice +SET GLOBAL rocksdb_create_checkpoint=CONCAT(@@rocksdb_datadir,'/mariabackup-checkpoint'); +SET GLOBAL rocksdb_remove_mariabackup_checkpoint=ON; +SET GLOBAL rocksdb_create_checkpoint=CONCAT(@@rocksdb_datadir,'/mariabackup-checkpoint'); +SET GLOBAL rocksdb_remove_mariabackup_checkpoint=ON; From aba2d7301f30daab9f54e92d21a4a130dda11dc4 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 7 Jun 2018 15:13:54 +0100 Subject: [PATCH 02/15] MDEV-13122 Backup myrocksdb with mariabackup. --- extra/mariabackup/backup_copy.cc | 272 +++++++++++++++++- extra/mariabackup/xtrabackup.cc | 17 +- extra/mariabackup/xtrabackup.h | 3 + .../mariabackup/include/have_rocksdb.inc | 4 + mysql-test/suite/mariabackup/xb_rocksdb.opt | 1 + .../suite/mariabackup/xb_rocksdb.result | 22 ++ mysql-test/suite/mariabackup/xb_rocksdb.test | 52 ++++ .../suite/mariabackup/xb_rocksdb_datadir.opt | 1 + .../mariabackup/xb_rocksdb_datadir.result | 9 + .../suite/mariabackup/xb_rocksdb_datadir.test | 34 +++ .../mariabackup/xb_rocksdb_datadir_debug.opt | 1 + .../xb_rocksdb_datadir_debug.result | 9 + .../mariabackup/xb_rocksdb_datadir_debug.test | 13 + 13 files changed, 435 insertions(+), 3 deletions(-) create mode 100644 mysql-test/suite/mariabackup/include/have_rocksdb.inc create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb.opt create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb.result create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb.test create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir.opt create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir.result create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir.test create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.opt create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.result create mode 100644 mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.test diff --git a/extra/mariabackup/backup_copy.cc b/extra/mariabackup/backup_copy.cc index 1405df9bcc8..16a4042d66e 100644 --- a/extra/mariabackup/backup_copy.cc +++ b/extra/mariabackup/backup_copy.cc @@ -59,6 +59,7 @@ Place, Suite 330, Boston, MA 02111-1307 USA #include #include "xb0xb.h" +#define ROCKSDB_BACKUP_DIR "#rocksdb" /* list of files to sync for --rsync mode */ static std::set rsync_list; @@ -68,6 +69,21 @@ static std::map tablespace_locations; /* Whether LOCK BINLOG FOR BACKUP has been issued during backup */ bool binlog_locked; +static void rocksdb_create_checkpoint(); +static bool has_rocksdb_plugin(); +static void copy_or_move_dir(const char *from, const char *to, bool copy, bool allow_hardlinks); +static void rocksdb_backup_checkpoint(); +static void rocksdb_copy_back(); + +static bool is_abs_path(const char *path) +{ +#ifdef _WIN32 + return path[0] && path[1] == ':' && (path[2] == '/' || path[2] == '\\'); +#else + return path[0] == '/'; +#endif +} + /************************************************************************ Struct represents file or directory. */ struct datadir_node_t { @@ -1140,7 +1156,8 @@ bool copy_or_move_file(const char *src_file_path, const char *dst_file_path, const char *dst_dir, - uint thread_n) + uint thread_n, + bool copy = xtrabackup_copy_back) { ds_ctxt_t *datasink = ds_data; /* copy to datadir by default */ char filedir[FN_REFLEN]; @@ -1188,7 +1205,7 @@ copy_or_move_file(const char *src_file_path, free(link_filepath); } - ret = (xtrabackup_copy_back ? + ret = (copy ? copy_file(datasink, src_file_path, dst_file_path, thread_n) : move_file(datasink, src_file_path, dst_file_path, dst_dir, thread_n)); @@ -1373,6 +1390,10 @@ bool backup_start() return false; } + if (has_rocksdb_plugin()) { + rocksdb_create_checkpoint(); + } + // There is no need to stop slave thread before coping non-Innodb data when // --no-lock option is used because --no-lock option requires that no DDL or // DML to non-transaction tables can occur. @@ -1458,6 +1479,10 @@ bool backup_finish() } } + if (has_rocksdb_plugin()) { + rocksdb_backup_checkpoint(); + } + msg_ts("Backup created in directory '%s'\n", xtrabackup_target_dir); if (mysql_binlog_position != NULL) { msg("MySQL binlog position: %s\n", mysql_binlog_position); @@ -1773,6 +1798,16 @@ copy_back() int i_tmp; bool is_ibdata_file; + if (strstr(node.filepath,"/" ROCKSDB_BACKUP_DIR "/") +#ifdef _WIN32 + || strstr(node.filepath,"\\" ROCKSDB_BACKUP_DIR "\\") +#endif + ) + { + // copied at later step + continue; + } + /* create empty directories */ if (node.is_empty_dir) { char path[FN_REFLEN]; @@ -1857,6 +1892,8 @@ copy_back() } } + rocksdb_copy_back(); + cleanup: if (it != NULL) { datadir_iter_free(it); @@ -2033,3 +2070,234 @@ static bool backup_files_from_datadir(const char *dir_path) os_file_closedir(dir); return ret; } + + +static int rocksdb_remove_checkpoint_directory() +{ + xb_mysql_query(mysql_connection, "set global rocksdb_remove_mariabackup_checkpoint=ON", false); + return 0; +} + +static bool has_rocksdb_plugin() +{ + static bool first_time = true; + static bool has_plugin= false; + if (!first_time || !xb_backup_rocksdb) + return has_plugin; + + const char *query = "SELECT COUNT(*) FROM information_schema.plugins WHERE plugin_name='rocksdb'"; + MYSQL_RES* result = xb_mysql_query(mysql_connection, query, true); + MYSQL_ROW row = mysql_fetch_row(result); + if (row) + has_plugin = !strcmp(row[0], "1"); + mysql_free_result(result); + first_time = false; + return has_plugin; +} + +static char *trim_trailing_dir_sep(char *path) +{ + size_t path_len = strlen(path); + while (path_len) + { + char c = path[path_len - 1]; + if (c == '/' IF_WIN(|| c == '\\', )) + path_len--; + else + break; + } + path[path_len] = 0; + return path; +} + +/* +Create a file hardlink. +@return true on success, false on error. +*/ +static bool make_hardlink(const char *from_path, const char *to_path) +{ + DBUG_EXECUTE_IF("no_hardlinks", return false;); + char to_path_full[FN_REFLEN]; + if (!is_abs_path(to_path)) + { + fn_format(to_path_full, to_path, ds_data->root, "", MYF(MY_RELATIVE_PATH)); + } + else + { + strncpy(to_path_full, to_path, sizeof(to_path_full)); + } +#ifdef _WIN32 + return CreateHardLink(to_path_full, from_path, NULL); +#else + return !link(from_path, to_path_full); +#endif +} + +/* + Copies or moves a directory (non-recursively so far). + Helper function used to backup rocksdb checkpoint, or copy-back the + rocksdb files. + + Has optimization that allows to use hardlinks when possible + (source and destination are directories on the same device) +*/ +static void copy_or_move_dir(const char *from, const char *to, bool do_copy, bool allow_hardlinks) +{ + datadir_node_t node; + datadir_node_init(&node); + datadir_iter_t *it = datadir_iter_new(from, false); + + while (datadir_iter_next(it, &node)) + { + char to_path[FN_REFLEN]; + const char *from_path = node.filepath; + snprintf(to_path, sizeof(to_path), "%s/%s", to, base_name(from_path)); + bool rc = false; + if (do_copy && allow_hardlinks) + { + rc = make_hardlink(from_path, to_path); + if (rc) + { + msg_ts("[%02u] Creating hardlink from %s to %s\n", + 1, from_path, to_path); + } + else + { + allow_hardlinks = false; + } + } + + if (!rc) + { + rc = (do_copy ? + copy_file(ds_data, from_path, to_path, 1) : + move_file(ds_data, from_path, node.filepath_rel, + to, 1)); + } + if (!rc) + exit(EXIT_FAILURE); + } + datadir_iter_free(it); + datadir_node_free(&node); + +} + +/* + Obtain user level lock , to protect the checkpoint directory of the server + from being user/overwritten by different backup processes, if backups are + running in parallel. + + This lock will be acquired before rocksdb checkpoint is created, held + while all files from it are being copied to their final backup destination, + and finally released after the checkpoint is removed. +*/ +static void rocksdb_lock_checkpoint() +{ + msg_ts("Obtaining rocksdb checkpoint lock.\n"); + MYSQL_RES *res = + xb_mysql_query(mysql_connection, "SELECT GET_LOCK('mariabackup_rocksdb_checkpoint',3600)", true, true); + + MYSQL_ROW r = mysql_fetch_row(res); + if (r && r[0] && strcmp(r[0], "1")) + { + msg_ts("Could not obtain rocksdb checkpont lock\n"); + exit(EXIT_FAILURE); + } +} + +static void rocksdb_unlock_checkpoint() +{ + xb_mysql_query(mysql_connection, + "SELECT RELEASE_LOCK('mariabackup_rocksdb_checkpoint')", false, true); +} + + +/* + Create temporary checkpoint in $rocksdb_datadir/mariabackup-checkpoint + directory. + A (user-level) lock named 'mariabackup_rocksdb_checkpoint' will also be + acquired be this function. +*/ +#define MARIADB_CHECKPOINT_DIR "mariabackup-checkpoint" +static char rocksdb_checkpoint_dir[FN_REFLEN]; + +static void rocksdb_create_checkpoint() +{ + MYSQL_RES *result = xb_mysql_query(mysql_connection, "SELECT @@rocksdb_datadir,@@datadir", true, true); + MYSQL_ROW row = mysql_fetch_row(result); + + DBUG_ASSERT(row && row[0] && row[1]); + + char *rocksdbdir = row[0]; + char *datadir = row[1]; + + if (is_abs_path(rocksdbdir)) + { + snprintf(rocksdb_checkpoint_dir, sizeof(rocksdb_checkpoint_dir), + "%s/" MARIADB_CHECKPOINT_DIR, trim_trailing_dir_sep(rocksdbdir)); + } + else + { + snprintf(rocksdb_checkpoint_dir, sizeof(rocksdb_checkpoint_dir), + "%s/%s/" MARIADB_CHECKPOINT_DIR, trim_trailing_dir_sep(datadir), + trim_dotslash(rocksdbdir)); + } + mysql_free_result(result); + +#ifdef _WIN32 + for (char *p = rocksdb_checkpoint_dir; *p; p++) + if (*p == '\\') *p = '/'; +#endif + + rocksdb_lock_checkpoint(); + + if (!access(rocksdb_checkpoint_dir, 0)) + { + msg_ts("Removing rocksdb checkpoint from previous backup attempt.\n"); + rocksdb_remove_checkpoint_directory(); + } + + char query[FN_REFLEN + 32]; + snprintf(query, sizeof(query), "SET GLOBAL rocksdb_create_checkpoint='%s'", rocksdb_checkpoint_dir); + xb_mysql_query(mysql_connection, query, false, true); +} + +/* + Copy files from rocksdb temporary checkpoint to final destination. + remove temp.checkpoint directory (in server's datadir) + and release user level lock acquired inside rocksdb_create_checkpoint(). +*/ +static void rocksdb_backup_checkpoint() +{ + msg_ts("Backing up rocksdb files.\n"); + char rocksdb_backup_dir[FN_REFLEN]; + snprintf(rocksdb_backup_dir, sizeof(rocksdb_backup_dir), "%s/" ROCKSDB_BACKUP_DIR , xtrabackup_target_dir); + bool backup_to_directory = xtrabackup_backup && xtrabackup_stream_fmt == XB_STREAM_FMT_NONE; + if (backup_to_directory) + { + if (my_mkdir(rocksdb_backup_dir, 0777, MYF(0))){ + msg_ts("Can't create rocksdb backup directory %s\n", rocksdb_backup_dir); + exit(EXIT_FAILURE); + } + } + copy_or_move_dir(rocksdb_checkpoint_dir, ROCKSDB_BACKUP_DIR, true, backup_to_directory); + rocksdb_remove_checkpoint_directory(); + rocksdb_unlock_checkpoint(); +} + +/* + Copies #rocksdb directory to the $rockdb_data_dir, on copy-back +*/ +static void rocksdb_copy_back() { + if (access(ROCKSDB_BACKUP_DIR, 0)) + return; + char rocksdb_home_dir[FN_REFLEN]; + if (xb_rocksdb_datadir && is_abs_path(xb_rocksdb_datadir)) { + strncpy(rocksdb_home_dir, xb_rocksdb_datadir, sizeof(rocksdb_home_dir)); + } else { + snprintf(rocksdb_home_dir, sizeof(rocksdb_home_dir), "%s/%s", mysql_data_home, + xb_rocksdb_datadir?trim_dotslash(xb_rocksdb_datadir): ROCKSDB_BACKUP_DIR); + } + mkdirp(rocksdb_home_dir, 0777, MYF(0)); + copy_or_move_dir(ROCKSDB_BACKUP_DIR, rocksdb_home_dir, xtrabackup_copy_back, xtrabackup_copy_back); +} diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index 65fddedc61a..4d88778f020 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -146,6 +146,8 @@ char *xtrabackup_tmpdir; char *xtrabackup_tables; char *xtrabackup_tables_file; char *xtrabackup_tables_exclude; +char *xb_rocksdb_datadir; +my_bool xb_backup_rocksdb = 1; typedef std::list regex_list_t; static regex_list_t regex_include_list; @@ -687,7 +689,9 @@ enum options_xtrabackup OPT_XTRA_TABLES_EXCLUDE, OPT_XTRA_DATABASES_EXCLUDE, OPT_PROTOCOL, - OPT_LOCK_DDL_PER_TABLE + OPT_LOCK_DDL_PER_TABLE, + OPT_ROCKSDB_DATADIR, + OPT_BACKUP_ROCKSDB }; struct my_option xb_client_options[] = @@ -1234,6 +1238,17 @@ struct my_option xb_server_options[] = (uchar*) &opt_lock_ddl_per_table, (uchar*) &opt_lock_ddl_per_table, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, + {"rocksdb-datadir", OPT_ROCKSDB_DATADIR, "RocksDB data directory." + "This option is only used with --copy-back or --move-back option", + &xb_rocksdb_datadir, &xb_rocksdb_datadir, + 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 }, + + { "backup-rocksdb", OPT_BACKUP_ROCKSDB, "Backup rocksdb data, if rocksdb plugin is installed." + "Used only with --backup option. Can be useful for partial backups, to exclude all rocksdb data", + &xb_backup_rocksdb, &xb_backup_rocksdb, + 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0 }, + + { 0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} }; diff --git a/extra/mariabackup/xtrabackup.h b/extra/mariabackup/xtrabackup.h index 2db5bc29b51..857c253f803 100644 --- a/extra/mariabackup/xtrabackup.h +++ b/extra/mariabackup/xtrabackup.h @@ -44,6 +44,9 @@ extern char *xtrabackup_incremental_basedir; extern char *innobase_data_home_dir; extern char *innobase_buffer_pool_filename; extern char *xb_plugin_dir; +extern char *xb_rocksdb_datadir; +extern my_bool xb_backup_rocksdb; + extern uint opt_protocol; extern ds_ctxt_t *ds_meta; extern ds_ctxt_t *ds_data; diff --git a/mysql-test/suite/mariabackup/include/have_rocksdb.inc b/mysql-test/suite/mariabackup/include/have_rocksdb.inc new file mode 100644 index 00000000000..d59f76f6cf3 --- /dev/null +++ b/mysql-test/suite/mariabackup/include/have_rocksdb.inc @@ -0,0 +1,4 @@ +if (`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.PLUGINS WHERE PLUGIN_NAME = 'rocksdb'`) +{ + --skip Requires rocksdb +} \ No newline at end of file diff --git a/mysql-test/suite/mariabackup/xb_rocksdb.opt b/mysql-test/suite/mariabackup/xb_rocksdb.opt new file mode 100644 index 00000000000..e582413e5b5 --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb.opt @@ -0,0 +1 @@ +--plugin-load=$HA_ROCKSDB_SO \ No newline at end of file diff --git a/mysql-test/suite/mariabackup/xb_rocksdb.result b/mysql-test/suite/mariabackup/xb_rocksdb.result new file mode 100644 index 00000000000..84476eeaba0 --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb.result @@ -0,0 +1,22 @@ +CREATE TABLE t(i INT) ENGINE ROCKSDB; +INSERT INTO t VALUES(1); +# xtrabackup backup +INSERT INTO t VALUES(2); +# xtrabackup prepare +# shutdown server +# remove datadir +# xtrabackup move back +# restart server +SELECT * FROM t; +i +1 +# xbstream extract +# xtrabackup prepare +# shutdown server +# remove datadir +# xtrabackup move back +# restart server +SELECT * FROM t; +i +1 +DROP TABLE t; diff --git a/mysql-test/suite/mariabackup/xb_rocksdb.test b/mysql-test/suite/mariabackup/xb_rocksdb.test new file mode 100644 index 00000000000..e41f3b2bf7e --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb.test @@ -0,0 +1,52 @@ +--source include/have_rocksdb.inc + +CREATE TABLE t(i INT) ENGINE ROCKSDB; +INSERT INTO t VALUES(1); +echo # xtrabackup backup; +# we'll backup to both directory and to stream to restore that later + +let $targetdir=$MYSQLTEST_VARDIR/tmp/backup; +let $stream=$MYSQLTEST_VARDIR/tmp/backup.xb; +--disable_result_log +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --backup --target-dir=$targetdir $backup_extra_param; +--enable_result_log +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --backup --stream=xbstream > $stream 2>$MYSQLTEST_VARDIR/tmp/backup_stream.log; + +INSERT INTO t VALUES(2); + + +echo # xtrabackup prepare; +--disable_result_log +exec $XTRABACKUP --prepare --target-dir=$targetdir; +-- source include/restart_and_restore.inc +--enable_result_log + +SELECT * FROM t; + +rmdir $targetdir; +mkdir $targetdir; + + +echo # xbstream extract; + +exec $XBSTREAM -x -C $targetdir < $stream; + +echo # xtrabackup prepare; +--disable_result_log +exec $XTRABACKUP --prepare --target-dir=$targetdir; + +let $_datadir= `SELECT @@datadir`; +echo # shutdown server; +--source include/shutdown_mysqld.inc +echo # remove datadir; +rmdir $_datadir; +echo # xtrabackup move back; +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --move-back --datadir=$_datadir --target-dir=$targetdir $copy_back_extra_param; +echo # restart server; +--source include/start_mysqld.inc + +--enable_result_log +SELECT * FROM t; + +DROP TABLE t; +rmdir $targetdir; diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir.opt b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.opt new file mode 100644 index 00000000000..0f069018e15 --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.opt @@ -0,0 +1 @@ +--plugin-load=$HA_ROCKSDB_SO --loose-rocksdb-datadir=$MYSQLTEST_VARDIR/tmp/rocksdb_datadir \ No newline at end of file diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir.result b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.result new file mode 100644 index 00000000000..9227198cbec --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.result @@ -0,0 +1,9 @@ +CREATE TABLE t(i INT) ENGINE ROCKSDB; +INSERT INTO t VALUES(1); +# xtrabackup backup +INSERT INTO t VALUES(2); +# xtrabackup prepare +SELECT * FROM t; +i +1 +DROP TABLE t; diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir.test b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.test new file mode 100644 index 00000000000..c2e90d9075b --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir.test @@ -0,0 +1,34 @@ +if (`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.PLUGINS WHERE PLUGIN_NAME = 'rocksdb'`) +{ + --skip Requires rocksdb +} + + +CREATE TABLE t(i INT) ENGINE ROCKSDB; +INSERT INTO t VALUES(1); +echo # xtrabackup backup; +let $targetdir=$MYSQLTEST_VARDIR/tmp/backup; +--disable_result_log +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --backup --target-dir=$targetdir; +--enable_result_log + +INSERT INTO t VALUES(2); + + +echo # xtrabackup prepare; +--disable_result_log +exec $XTRABACKUP --prepare --target-dir=$targetdir; +let $_datadir= `SELECT @@datadir`; +let $_rocksdb_datadir=`SELECT @@rocksdb_datadir`; +--source include/shutdown_mysqld.inc +rmdir $_datadir; +rmdir $_rocksdb_datadir; +exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --move-back --target-dir=$targetdir --datadir=$_datadir --rocksdb_datadir=$_rocksdb_datadir; +--enable_result_log +--source include/start_mysqld.inc + + +SELECT * FROM t; +DROP TABLE t; +rmdir $targetdir; + diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.opt b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.opt new file mode 100644 index 00000000000..0f069018e15 --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.opt @@ -0,0 +1 @@ +--plugin-load=$HA_ROCKSDB_SO --loose-rocksdb-datadir=$MYSQLTEST_VARDIR/tmp/rocksdb_datadir \ No newline at end of file diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.result b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.result new file mode 100644 index 00000000000..9227198cbec --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.result @@ -0,0 +1,9 @@ +CREATE TABLE t(i INT) ENGINE ROCKSDB; +INSERT INTO t VALUES(1); +# xtrabackup backup +INSERT INTO t VALUES(2); +# xtrabackup prepare +SELECT * FROM t; +i +1 +DROP TABLE t; diff --git a/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.test b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.test new file mode 100644 index 00000000000..a71c63b98cc --- /dev/null +++ b/mysql-test/suite/mariabackup/xb_rocksdb_datadir_debug.test @@ -0,0 +1,13 @@ +--source include/have_debug.inc +--source include/have_rocksdb.inc + +# Check how rocksdb backup works without hardlinks +let $backup_extra_param='--dbug=+d,no_hardlinks'; +let $copy_back_extra_param='--dbug=+d,no_hardlinks'; + +# Pretend that previous backup crashes, and left checkpoint directory +let $rocksdb_datadir= `SELECT @@rocksdb_datadir`; +mkdir $rocksdb_datadir/mariadb-checkpoint; + +--source xb_rocksdb_datadir.test + From ecd4c2b4a953596e15b90c1108aaa74a87d9b945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Mon, 11 Jun 2018 20:24:41 +0300 Subject: [PATCH 03/15] Add missed change from 7ffa82b03c8da12062223d5e332e972d6f828d44 --- client/mysqltest.cc | 2 +- mysql-test/include/wait_until_connected_again.inc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index b962813f558..803550ba7be 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -1507,7 +1507,7 @@ void free_used_memory() dynstr_free(&(*q)->content); my_free((*q)); } - for (i= 0; i < 10; i++) + for (i= 0; i < 12; i++) { if (var_reg[i].alloced_len) my_free(var_reg[i].str_val); diff --git a/mysql-test/include/wait_until_connected_again.inc b/mysql-test/include/wait_until_connected_again.inc index 2e80ea2ed7d..26168d10558 100644 --- a/mysql-test/include/wait_until_connected_again.inc +++ b/mysql-test/include/wait_until_connected_again.inc @@ -11,7 +11,7 @@ let $counter= 5000; let $mysql_errno= 9999; while ($mysql_errno) { - --error 0,ER_SERVER_SHUTDOWN,ER_CONNECTION_KILLED,2002,2006,2013 + --error 0,ER_SERVER_SHUTDOWN,ER_CONNECTION_KILLED,ER_LOCK_WAIT_TIMEOUT,2002,2006,2013 show status; dec $counter; From d54d06760195ed0117882a7b8dacfb6cfc23e823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicen=C8=9Biu=20Ciorbaru?= Date: Mon, 11 Jun 2018 21:16:37 +0300 Subject: [PATCH 04/15] Undo wrong my_free overflow --- client/mysqltest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 803550ba7be..b962813f558 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -1507,7 +1507,7 @@ void free_used_memory() dynstr_free(&(*q)->content); my_free((*q)); } - for (i= 0; i < 12; i++) + for (i= 0; i < 10; i++) { if (var_reg[i].alloced_len) my_free(var_reg[i].str_val); From 8f5f0575ab41bd03369ab1dc31425ef7ab1c6b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 11 Jun 2018 13:02:47 +0300 Subject: [PATCH 05/15] MDEV-16456 InnoDB error "returned OS error 71" complains about wrong path When attempting to rename a table to a non-existing database, InnoDB would misleadingly report "OS error 71" when in fact the error code is InnoDB's own (OS_FILE_NOT_FOUND), and not report both pathnames. Errors on rename could occur due to reasons connected to either pathname. os_file_handle_rename_error(): New function, to report errors in renaming files. --- mysql-test/suite/innodb/r/rename_table.result | 6 +++++ .../suite/innodb/t/innodb-mdev7046.test | 6 ++--- mysql-test/suite/innodb/t/rename_table.test | 14 +++++++++++ storage/innobase/os/os0file.cc | 24 +++++++++++++++---- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/mysql-test/suite/innodb/r/rename_table.result b/mysql-test/suite/innodb/r/rename_table.result index 3d7c3ff1b0e..9c45117cf10 100644 --- a/mysql-test/suite/innodb/r/rename_table.result +++ b/mysql-test/suite/innodb/r/rename_table.result @@ -18,3 +18,9 @@ path ./abc_def2/test1.ibd DROP DATABASE abc_def; DROP DATABASE abc_def2; +call mtr.add_suppression("InnoDB: (Operating system error|The error means|Cannot rename file)"); +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +RENAME TABLE t1 TO non_existing_db.t1; +ERROR HY000: Error on rename of './test/t1' to './non_existing_db/t1' (errno: 168 "Unknown (generic) error from engine") +FOUND 1 /\[ERROR\] InnoDB: Cannot rename file '.*t1\.ibd' to '.*non_existing_db/ in mysqld.1.err +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/innodb-mdev7046.test b/mysql-test/suite/innodb/t/innodb-mdev7046.test index 208dcd52f35..85a257a1739 100644 --- a/mysql-test/suite/innodb/t/innodb-mdev7046.test +++ b/mysql-test/suite/innodb/t/innodb-mdev7046.test @@ -9,9 +9,9 @@ # Ignore OS errors -call mtr.add_suppression("InnoDB: File ./test/t1*"); -call mtr.add_suppression("InnoDB: Error number*"); -call mtr.add_suppression("InnoDB: File ./test/t1#p#p1#sp#p1sp0.ibd: 'rename' returned OS error*"); +call mtr.add_suppression("InnoDB: File ./test/t1"); +call mtr.add_suppression("InnoDB: Error number"); +call mtr.add_suppression("InnoDB: Cannot rename file '.*/test/t1#[Pp]#p1#[Ss][Pp]#p1sp0\\.ibd' to"); call mtr.add_suppression("InnoDB: Operating system error number .* in a file operation."); # MDEV-7046: MySQL#74480 - Failing assertion: os_file_status(newpath, &exists, &type) diff --git a/mysql-test/suite/innodb/t/rename_table.test b/mysql-test/suite/innodb/t/rename_table.test index ea9f70bacb0..0191a94def2 100644 --- a/mysql-test/suite/innodb/t/rename_table.test +++ b/mysql-test/suite/innodb/t/rename_table.test @@ -29,3 +29,17 @@ DROP DATABASE abc_def; --source include/restart_mysqld.inc DROP DATABASE abc_def2; + +call mtr.add_suppression("InnoDB: (Operating system error|The error means|Cannot rename file)"); + +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +--replace_result "\\" "/" +--error ER_ERROR_ON_RENAME +RENAME TABLE t1 TO non_existing_db.t1; + +--let SEARCH_PATTERN= \[ERROR\] InnoDB: Cannot rename file '.*t1\.ibd' to '.*non_existing_db +let SEARCH_FILE= $MYSQLTEST_VARDIR/log/mysqld.1.err; +--source include/search_pattern_in_file.inc + +# Cleanup +DROP TABLE t1; diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 9fb1c398e6e..613a3cad0e9 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -753,6 +753,23 @@ os_file_handle_error_no_exit( name, operation, false, on_error_silent)); } +/** Handle RENAME error. +@param name old name of the file +@param new_name new name of the file */ +static void os_file_handle_rename_error(const char* name, const char* new_name) +{ + if (os_file_get_last_error(true) != OS_FILE_DISK_FULL) { + ib::error() << "Cannot rename file '" << name << "' to '" + << new_name << "'"; + } else if (!os_has_said_disk_full) { + os_has_said_disk_full = true; + /* Disk full error is reported irrespective of the + on_error_silent setting. */ + ib::error() << "Full disk prevents renaming file '" + << name << "' to '" << new_name << "'"; + } +} + /** Does simulated AIO. This function should be called by an i/o-handler thread. @@ -777,9 +794,7 @@ os_aio_simulated_handler( #ifdef _WIN32 static HANDLE win_get_syncio_event(); -#endif -#ifdef _WIN32 /** Wrapper around Windows DeviceIoControl() function. @@ -3224,7 +3239,7 @@ os_file_rename_func( ret = rename(oldpath, newpath); if (ret != 0) { - os_file_handle_error_no_exit(oldpath, "rename", FALSE); + os_file_handle_rename_error(oldpath, newpath); return(false); } @@ -4555,8 +4570,7 @@ os_file_rename_func( return(true); } - os_file_handle_error_no_exit(oldpath, "rename", false); - + os_file_handle_rename_error(oldpath, newpath); return(false); } From ae0aefb1c522455b785c2e43636c482cd161e3de Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 12 Jun 2018 14:12:36 +0400 Subject: [PATCH 06/15] MDEV-12060 Crash in EXECUTE IMMEDIATE with an expression returning a GRANT command This problem was earlier fixed by MDEV-14603. Only adding 10.2 specific tests. --- mysql-test/r/ps.result | 39 ++++++++++++++++++++++++++--- mysql-test/t/ps.test | 56 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 6f86b330251..ac96dd05a2a 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -4918,9 +4918,6 @@ DROP TABLE t1; # End of MDEV-10866 Extend PREPARE and EXECUTE IMMEDIATE to understand expressions # # -# End of 10.2 tests -# -# # MDEV-11360 Dynamic SQL: DEFAULT as a bind parameter # CREATE TABLE t1 (a INT DEFAULT 10, b INT DEFAULT NULL); @@ -5174,3 +5171,39 @@ execute stmt; execute stmt; execute stmt; drop table t1; +# +# MDEV-12060 Crash in EXECUTE IMMEDIATE with an expression returning a GRANT command +# +CREATE ROLE testrole; +CREATE OR REPLACE PROCEDURE p1() +BEGIN +END; +/ +CREATE PROCEDURE p2 (wgrp VARCHAR(10)) +BEGIN +EXECUTE IMMEDIATE concat('GRANT EXECUTE ON PROCEDURE p1 TO ',wgrp); +END; +/ +CALL p2('testrole'); +DROP PROCEDURE p2; +CREATE PROCEDURE p2 () +BEGIN +EXECUTE IMMEDIATE concat(_utf8'GRANT EXECUTE ON PROCEDURE p1 TO ',_latin1'testrole'); +END; +/ +CALL p2(); +DROP PROCEDURE p2; +CREATE PROCEDURE p2 () +BEGIN +PREPARE stmt FROM concat(_utf8'GRANT EXECUTE ON PROCEDURE p1 TO ',_latin1' testrole'); +EXECUTE stmt; +DEALLOCATE PREPARE stmt; +END; +/ +CALL p2(); +DROP PROCEDURE p2; +DROP PROCEDURE p1; +DROP ROLE testrole; +# +# End of 10.2 tests +# diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index e051cdce179..1c8595936cd 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -4386,9 +4386,6 @@ DROP TABLE t1; --echo # End of MDEV-10866 Extend PREPARE and EXECUTE IMMEDIATE to understand expressions --echo # ---echo # ---echo # End of 10.2 tests ---echo # --echo # @@ -4650,3 +4647,56 @@ execute stmt; execute stmt; execute stmt; drop table t1; + + +--echo # +--echo # MDEV-12060 Crash in EXECUTE IMMEDIATE with an expression returning a GRANT command +--echo # + +CREATE ROLE testrole; +DELIMITER /; +CREATE OR REPLACE PROCEDURE p1() +BEGIN +END; +/ +DELIMITER ;/ + +DELIMITER /; +CREATE PROCEDURE p2 (wgrp VARCHAR(10)) +BEGIN + EXECUTE IMMEDIATE concat('GRANT EXECUTE ON PROCEDURE p1 TO ',wgrp); +END; +/ +DELIMITER ;/ +CALL p2('testrole'); +DROP PROCEDURE p2; + +DELIMITER /; +CREATE PROCEDURE p2 () +BEGIN + EXECUTE IMMEDIATE concat(_utf8'GRANT EXECUTE ON PROCEDURE p1 TO ',_latin1'testrole'); +END; +/ +DELIMITER ;/ +CALL p2(); +DROP PROCEDURE p2; + +DELIMITER /; +CREATE PROCEDURE p2 () +BEGIN + PREPARE stmt FROM concat(_utf8'GRANT EXECUTE ON PROCEDURE p1 TO ',_latin1' testrole'); + EXECUTE stmt; + DEALLOCATE PREPARE stmt; +END; +/ +DELIMITER ;/ +CALL p2(); +DROP PROCEDURE p2; + +DROP PROCEDURE p1; +DROP ROLE testrole; + + +--echo # +--echo # End of 10.2 tests +--echo # From 2412c151916dc65660644a0cd2fe5f34816ea901 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 13 Jun 2018 11:56:56 +0400 Subject: [PATCH 07/15] MDEV-15870 Using aggregate and window function in unexpected places can crash the server --- mysql-test/r/sp.result | 11 +++++++++++ mysql-test/t/sp.test | 14 ++++++++++++++ sql/item_sum.cc | 4 ++-- sql/item_windowfunc.cc | 6 +++--- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index ad8dc15318e..59387b37585 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -8354,3 +8354,14 @@ drop procedure p3; CREATE PROCEDURE foo ( IN i INT UNSIGNED ) BEGIN END; CALL foo( LAST_INSERT_ID() ); DROP PROCEDURE foo; +# +# MDEV-15870 Using aggregate and window function in unexpected places can crash the server +# +CREATE PROCEDURE p1 (a TEXT) BEGIN END; +CALL p1(RANK() OVER (ORDER BY 1)); +ERROR HY000: Window function is allowed only in SELECT list and ORDER BY clause +CALL p1(ROW_NUMBER() OVER ()); +ERROR HY000: Window function is allowed only in SELECT list and ORDER BY clause +CALL p1(SUM(1)); +ERROR HY000: Invalid use of group function +DROP PROCEDURE p1; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 549d97ad72b..e8b63c4d791 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -9865,3 +9865,17 @@ drop procedure p3; CREATE PROCEDURE foo ( IN i INT UNSIGNED ) BEGIN END; CALL foo( LAST_INSERT_ID() ); DROP PROCEDURE foo; + + +--echo # +--echo # MDEV-15870 Using aggregate and window function in unexpected places can crash the server +--echo # + +CREATE PROCEDURE p1 (a TEXT) BEGIN END; +--error ER_WRONG_PLACEMENT_OF_WINDOW_FUNCTION +CALL p1(RANK() OVER (ORDER BY 1)); +--error ER_WRONG_PLACEMENT_OF_WINDOW_FUNCTION +CALL p1(ROW_NUMBER() OVER ()); +--error ER_INVALID_GROUP_FUNC_USE +CALL p1(SUM(1)); +DROP PROCEDURE p1; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 4cf11e81d3d..cb150db3031 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -68,14 +68,14 @@ size_t Item_sum::ram_limitation(THD *thd) bool Item_sum::init_sum_func_check(THD *thd) { SELECT_LEX *curr_sel= thd->lex->current_select; - if (!curr_sel->name_visibility_map) + if (curr_sel && !curr_sel->name_visibility_map) { for (SELECT_LEX *sl= curr_sel; sl; sl= sl->context.outer_select()) { curr_sel->name_visibility_map|= (1 << sl-> nest_level); } } - if (!(thd->lex->allow_sum_func & curr_sel->name_visibility_map)) + if (!curr_sel || !(thd->lex->allow_sum_func & curr_sel->name_visibility_map)) { my_message(ER_INVALID_GROUP_FUNC_USE, ER_THD(thd, ER_INVALID_GROUP_FUNC_USE), MYF(0)); diff --git a/sql/item_windowfunc.cc b/sql/item_windowfunc.cc index 5fbfb2651af..52738bfab87 100644 --- a/sql/item_windowfunc.cc +++ b/sql/item_windowfunc.cc @@ -71,9 +71,9 @@ Item_window_func::fix_fields(THD *thd, Item **ref) { DBUG_ASSERT(fixed == 0); - enum_parsing_place place= thd->lex->current_select->context_analysis_place; - - if (!(place == SELECT_LIST || place == IN_ORDER_BY)) + if (!thd->lex->current_select || + (thd->lex->current_select->context_analysis_place != SELECT_LIST && + thd->lex->current_select->context_analysis_place != IN_ORDER_BY)) { my_error(ER_WRONG_PLACEMENT_OF_WINDOW_FUNCTION, MYF(0)); return true; From 931daaf79b9bea447a28abff01f04cdf2f3a07f8 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 13 Jun 2018 14:50:25 +0300 Subject: [PATCH 08/15] MDEV-15319: [SQL Layer] Server crashes in Field::set_null / myrocks::ha_rocksdb ... Added a testcase --- .../mysql-test/rocksdb/r/mariadb_port_fixes.result | 9 +++++++++ .../mysql-test/rocksdb/t/mariadb_port_fixes.test | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/storage/rocksdb/mysql-test/rocksdb/r/mariadb_port_fixes.result b/storage/rocksdb/mysql-test/rocksdb/r/mariadb_port_fixes.result index d9e2bf5eea5..aab6e82ae67 100644 --- a/storage/rocksdb/mysql-test/rocksdb/r/mariadb_port_fixes.result +++ b/storage/rocksdb/mysql-test/rocksdb/r/mariadb_port_fixes.result @@ -107,3 +107,12 @@ a b 101 foo 102 bar DROP TABLE t1; +# +# MDEV-15319: [SQL Layer] Server crashes in Field::set_null / myrocks::ha_rocksdb::convert_record_from_storage_format +# (just a testcase) +# +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1); +CREATE TABLE t2 ENGINE=RocksDB AS SELECT VALUES(i) AS a FROM t1; +DELETE FROM t2; +DROP TABLE t1,t2; diff --git a/storage/rocksdb/mysql-test/rocksdb/t/mariadb_port_fixes.test b/storage/rocksdb/mysql-test/rocksdb/t/mariadb_port_fixes.test index 7282ec237c2..569f33c94b2 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/mariadb_port_fixes.test +++ b/storage/rocksdb/mysql-test/rocksdb/t/mariadb_port_fixes.test @@ -101,3 +101,14 @@ UPDATE t1 SET a=a+100; SELECT * FROM t1; DROP TABLE t1; + +--echo # +--echo # MDEV-15319: [SQL Layer] Server crashes in Field::set_null / myrocks::ha_rocksdb::convert_record_from_storage_format +--echo # (just a testcase) +--echo # + +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1); +CREATE TABLE t2 ENGINE=RocksDB AS SELECT VALUES(i) AS a FROM t1; +DELETE FROM t2; +DROP TABLE t1,t2; From 8662015c90718501d504f4c7aeb94b8626902a9c Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 13 Jun 2018 15:26:50 +0300 Subject: [PATCH 09/15] MDEV-15304: Server crash in print_keydup_error / key_unpack or unexpected ER_DUP_KEY Adjust the patch to match the variant accepted into the upstream: undo the changes in ha_rocksdb::load_hidden_pk_value(). --- storage/rocksdb/ha_rocksdb.cc | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 7cddfea33a7..c1c3191514f 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -5302,17 +5302,8 @@ int ha_rocksdb::load_hidden_pk_value() { active_index = m_tbl_def->m_key_count - 1; const uint8 save_table_status = table->status; - /* - We should read the latest committed value in the database. - That is, if we have an open transaction with a snapshot, we should not use - it as we may get old data. Start a new transaction to read the latest - value. - */ - Rdb_transaction *const temp_tx = new Rdb_transaction_impl(table->in_use); - temp_tx->start_tx(); - Rdb_transaction *&tx = get_tx_from_thd(table->in_use); - Rdb_transaction *save_tx= tx; - tx= temp_tx; + Rdb_transaction *const tx = get_or_create_tx(table->in_use); + const bool is_new_snapshot = !tx->has_snapshot(); longlong hidden_pk_id = 1; // Do a lookup. @@ -5322,8 +5313,9 @@ int ha_rocksdb::load_hidden_pk_value() { */ auto err = read_hidden_pk_id_from_rowkey(&hidden_pk_id); if (err) { - delete tx; - tx= save_tx; + if (is_new_snapshot) { + tx->release_snapshot(); + } return err; } @@ -5335,8 +5327,9 @@ int ha_rocksdb::load_hidden_pk_value() { !m_tbl_def->m_hidden_pk_val.compare_exchange_weak(old, hidden_pk_id)) { } - delete tx; - tx= save_tx; + if (is_new_snapshot) { + tx->release_snapshot(); + } table->status = save_table_status; active_index = save_active_index; From 23ced2f846c6a8b9b303c1365780999888fa438f Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 13 Jun 2018 23:37:09 +0400 Subject: [PATCH 10/15] MDEV-16311 Server crash when using a NAME_CONST() with a CURSOR Problem: The problem was most likely introduced by a fix for MDEV-11597 (commit 5f0c31f928338e8a6ffde098b7ffd3d1a8b02903) which removed the assignment "killed= KILL_BAD_DATA" from THD::raise_condition(). Before MDEV-11597, sp_head::execute() tested thd->killed after looping through the SP instructions and exited with an error if thd->killed is set. After MDEV-11597, sp_head::execute() stopped to notice errors and set the OK status on top of the error status, which crashed on assert. Fix: Making sp_cursor::fetch() return -1 if server_side_cursor->fetch(1) left an error in the diagnostics area. This makes the statement "err_status= i->execute(thd, &ip)" in sp_head::execute() set the error code and correctly break the SP instruction loop and return on error without setting the OK status. --- mysql-test/r/sp.result | 17 +++++++++++++++++ mysql-test/t/sp.test | 22 ++++++++++++++++++++++ sql/sp_rcontext.cc | 6 ++++++ 3 files changed, 45 insertions(+) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 59387b37585..7e466f23c72 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -8365,3 +8365,20 @@ ERROR HY000: Window function is allowed only in SELECT list and ORDER BY clause CALL p1(SUM(1)); ERROR HY000: Invalid use of group function DROP PROCEDURE p1; +# +# MDEV-16311 Server crash when using a NAME_CONST() with a CURSOR +# +SET sql_mode=STRICT_ALL_TABLES; +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (10); +BEGIN NOT ATOMIC +DECLARE a INT; +DECLARE c CURSOR FOR SELECT NAME_CONST('x','y') FROM t1; +OPEN c; +FETCH c INTO a; +CLOSE c; +END; +$$ +ERROR 22007: Incorrect integer value: 'y' for column 'a' at row 1 +DROP TABLE t1; +SET sql_mode=DEFAULT; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index e8b63c4d791..a2155d3fe79 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -9879,3 +9879,25 @@ CALL p1(ROW_NUMBER() OVER ()); --error ER_INVALID_GROUP_FUNC_USE CALL p1(SUM(1)); DROP PROCEDURE p1; + + +--echo # +--echo # MDEV-16311 Server crash when using a NAME_CONST() with a CURSOR +--echo # + +SET sql_mode=STRICT_ALL_TABLES; +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (10); +DELIMITER $$; +--error ER_TRUNCATED_WRONG_VALUE_FOR_FIELD +BEGIN NOT ATOMIC + DECLARE a INT; + DECLARE c CURSOR FOR SELECT NAME_CONST('x','y') FROM t1; + OPEN c; + FETCH c INTO a; + CLOSE c; +END; +$$ +DELIMITER ;$$ +DROP TABLE t1; +SET sql_mode=DEFAULT; diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc index 396f5b448fc..d612e15c000 100644 --- a/sql/sp_rcontext.cc +++ b/sql/sp_rcontext.cc @@ -509,9 +509,15 @@ int sp_cursor::fetch(THD *thd, List *vars) result.set_spvar_list(vars); + DBUG_ASSERT(!thd->is_error()); + /* Attempt to fetch one row */ if (server_side_cursor->is_open()) + { server_side_cursor->fetch(1); + if (thd->is_error()) + return -1; // e.g. data type conversion failed + } /* If the cursor was pointing after the last row, the fetch will From 2cdb483bc4ed5bf816ff609cce91745babc9657a Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 14 Jun 2018 13:13:23 +0400 Subject: [PATCH 11/15] MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number --- mysql-test/include/autoinc_mdev15353.inc | 29 +++++ mysql-test/r/auto_increment.result | 120 +++++++++++++++++ .../suite/heap/heap_auto_increment.result | 120 +++++++++++++++++ .../suite/heap/heap_auto_increment.test | 7 + .../suite/innodb/r/innodb-autoinc.result | 121 ++++++++++++++++++ mysql-test/suite/innodb/t/innodb-autoinc.test | 10 ++ mysql-test/suite/maria/maria-autoinc.result | 120 +++++++++++++++++ mysql-test/suite/maria/maria-autoinc.test | 8 ++ mysql-test/t/auto_increment.test | 8 ++ sql/field.cc | 10 ++ sql/field.h | 117 ++++++++++------- 11 files changed, 624 insertions(+), 46 deletions(-) create mode 100644 mysql-test/include/autoinc_mdev15353.inc create mode 100644 mysql-test/suite/maria/maria-autoinc.result create mode 100644 mysql-test/suite/maria/maria-autoinc.test diff --git a/mysql-test/include/autoinc_mdev15353.inc b/mysql-test/include/autoinc_mdev15353.inc new file mode 100644 index 00000000000..9085cb29f20 --- /dev/null +++ b/mysql-test/include/autoinc_mdev15353.inc @@ -0,0 +1,29 @@ +DELIMITER $$; +CREATE PROCEDURE autoinc_mdev15353_one(engine VARCHAR(64), t VARCHAR(64)) +BEGIN + DECLARE query TEXT DEFAULT 'CREATE TABLE t1 (' + ' id TTT NOT NULL AUTO_INCREMENT,' + ' name CHAR(30) NOT NULL,' + ' PRIMARY KEY (id)) ENGINE=EEE'; + EXECUTE IMMEDIATE REPLACE(REPLACE(query,'TTT', t), 'EEE', engine); + SHOW CREATE TABLE t1; + INSERT INTO t1 (name) VALUES ('dog'); + SELECT * FROM t1; + UPDATE t1 SET id=-1 WHERE id=1; + SELECT * FROM t1; + INSERT INTO t1 (name) VALUES ('cat'); + SELECT * FROM t1; + DROP TABLE t1; +END; +$$ +DELIMITER ;$$ + +CALL autoinc_mdev15353_one(@engine, 'tinyint'); +CALL autoinc_mdev15353_one(@engine, 'smallint'); +CALL autoinc_mdev15353_one(@engine, 'mediumint'); +CALL autoinc_mdev15353_one(@engine, 'int'); +CALL autoinc_mdev15353_one(@engine, 'bigint'); +CALL autoinc_mdev15353_one(@engine, 'float'); +CALL autoinc_mdev15353_one(@engine, 'double'); + +DROP PROCEDURE autoinc_mdev15353_one; diff --git a/mysql-test/r/auto_increment.result b/mysql-test/r/auto_increment.result index 12cbf294b69..660a93b1b30 100644 --- a/mysql-test/r/auto_increment.result +++ b/mysql-test/r/auto_increment.result @@ -537,3 +537,123 @@ pk -5 1 drop table t1; +# +# MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +# +SET @engine='MyISAM'; +CREATE PROCEDURE autoinc_mdev15353_one(engine VARCHAR(64), t VARCHAR(64)) +BEGIN +DECLARE query TEXT DEFAULT 'CREATE TABLE t1 (' + ' id TTT NOT NULL AUTO_INCREMENT,' + ' name CHAR(30) NOT NULL,' + ' PRIMARY KEY (id)) ENGINE=EEE'; +EXECUTE IMMEDIATE REPLACE(REPLACE(query,'TTT', t), 'EEE', engine); +SHOW CREATE TABLE t1; +INSERT INTO t1 (name) VALUES ('dog'); +SELECT * FROM t1; +UPDATE t1 SET id=-1 WHERE id=1; +SELECT * FROM t1; +INSERT INTO t1 (name) VALUES ('cat'); +SELECT * FROM t1; +DROP TABLE t1; +END; +$$ +CALL autoinc_mdev15353_one(@engine, 'tinyint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` tinyint(4) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'smallint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` smallint(6) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'mediumint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` mediumint(9) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'int'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'bigint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` bigint(20) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'float'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` float NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'double'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` double NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +DROP PROCEDURE autoinc_mdev15353_one; diff --git a/mysql-test/suite/heap/heap_auto_increment.result b/mysql-test/suite/heap/heap_auto_increment.result index 5b04a77e9eb..d6504349d92 100644 --- a/mysql-test/suite/heap/heap_auto_increment.result +++ b/mysql-test/suite/heap/heap_auto_increment.result @@ -39,3 +39,123 @@ _rowid _rowid skey sval 1 1 1 hello 2 2 2 hey drop table t1; +# +# MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +# +SET @engine='MEMORY'; +CREATE PROCEDURE autoinc_mdev15353_one(engine VARCHAR(64), t VARCHAR(64)) +BEGIN +DECLARE query TEXT DEFAULT 'CREATE TABLE t1 (' + ' id TTT NOT NULL AUTO_INCREMENT,' + ' name CHAR(30) NOT NULL,' + ' PRIMARY KEY (id)) ENGINE=EEE'; +EXECUTE IMMEDIATE REPLACE(REPLACE(query,'TTT', t), 'EEE', engine); +SHOW CREATE TABLE t1; +INSERT INTO t1 (name) VALUES ('dog'); +SELECT * FROM t1; +UPDATE t1 SET id=-1 WHERE id=1; +SELECT * FROM t1; +INSERT INTO t1 (name) VALUES ('cat'); +SELECT * FROM t1; +DROP TABLE t1; +END; +$$ +CALL autoinc_mdev15353_one(@engine, 'tinyint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` tinyint(4) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'smallint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` smallint(6) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'mediumint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` mediumint(9) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'int'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'bigint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` bigint(20) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'float'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` float NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'double'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` double NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MEMORY DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +DROP PROCEDURE autoinc_mdev15353_one; diff --git a/mysql-test/suite/heap/heap_auto_increment.test b/mysql-test/suite/heap/heap_auto_increment.test index 016bc946209..6ec150f48da 100644 --- a/mysql-test/suite/heap/heap_auto_increment.test +++ b/mysql-test/suite/heap/heap_auto_increment.test @@ -33,3 +33,10 @@ select _rowid,t1._rowid,skey,sval from t1; drop table t1; # End of 4.1 tests + +--echo # +--echo # MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +--echo # + +SET @engine='MEMORY'; +--source include/autoinc_mdev15353.inc diff --git a/mysql-test/suite/innodb/r/innodb-autoinc.result b/mysql-test/suite/innodb/r/innodb-autoinc.result index ee1adc07661..eb7ff026b1a 100644 --- a/mysql-test/suite/innodb/r/innodb-autoinc.result +++ b/mysql-test/suite/innodb/r/innodb-autoinc.result @@ -1351,6 +1351,7 @@ t CREATE TABLE `t` ( KEY `i` (`i`) ) ENGINE=InnoDB AUTO_INCREMENT=401 DEFAULT CHARSET=latin1 DROP TABLE t; +SET auto_increment_increment = DEFAULT; # # MDEV-14008 Assertion failing: `!is_set() || (m_status == DA_OK_BULK && is_bulk_op()) # @@ -1369,3 +1370,123 @@ SELECT * FROM t1; a -1 DROP TABLE t1; +# +# MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +# +SET @engine='INNODB'; +CREATE PROCEDURE autoinc_mdev15353_one(engine VARCHAR(64), t VARCHAR(64)) +BEGIN +DECLARE query TEXT DEFAULT 'CREATE TABLE t1 (' + ' id TTT NOT NULL AUTO_INCREMENT,' + ' name CHAR(30) NOT NULL,' + ' PRIMARY KEY (id)) ENGINE=EEE'; +EXECUTE IMMEDIATE REPLACE(REPLACE(query,'TTT', t), 'EEE', engine); +SHOW CREATE TABLE t1; +INSERT INTO t1 (name) VALUES ('dog'); +SELECT * FROM t1; +UPDATE t1 SET id=-1 WHERE id=1; +SELECT * FROM t1; +INSERT INTO t1 (name) VALUES ('cat'); +SELECT * FROM t1; +DROP TABLE t1; +END; +$$ +CALL autoinc_mdev15353_one(@engine, 'tinyint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` tinyint(4) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'smallint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` smallint(6) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'mediumint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` mediumint(9) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'int'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'bigint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` bigint(20) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'float'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` float NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'double'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` double NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +DROP PROCEDURE autoinc_mdev15353_one; diff --git a/mysql-test/suite/innodb/t/innodb-autoinc.test b/mysql-test/suite/innodb/t/innodb-autoinc.test index db265ff8f67..b8f2d75c876 100644 --- a/mysql-test/suite/innodb/t/innodb-autoinc.test +++ b/mysql-test/suite/innodb/t/innodb-autoinc.test @@ -683,6 +683,8 @@ INSERT INTO t VALUES (NULL); SELECT * FROM t; SHOW CREATE TABLE t; DROP TABLE t; +SET auto_increment_increment = DEFAULT; + --echo # --echo # MDEV-14008 Assertion failing: `!is_set() || (m_status == DA_OK_BULK && is_bulk_op()) @@ -700,3 +702,11 @@ CREATE TABLE t1 (a DOUBLE PRIMARY KEY AUTO_INCREMENT) ENGINE=InnoDB; INSERT INTO t1 VALUES (-1); SELECT * FROM t1; DROP TABLE t1; + + +--echo # +--echo # MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +--echo # + +SET @engine='INNODB'; +--source include/autoinc_mdev15353.inc diff --git a/mysql-test/suite/maria/maria-autoinc.result b/mysql-test/suite/maria/maria-autoinc.result new file mode 100644 index 00000000000..a4ae1f72c1e --- /dev/null +++ b/mysql-test/suite/maria/maria-autoinc.result @@ -0,0 +1,120 @@ +# +# MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +# +SET @engine='ARIA'; +CREATE PROCEDURE autoinc_mdev15353_one(engine VARCHAR(64), t VARCHAR(64)) +BEGIN +DECLARE query TEXT DEFAULT 'CREATE TABLE t1 (' + ' id TTT NOT NULL AUTO_INCREMENT,' + ' name CHAR(30) NOT NULL,' + ' PRIMARY KEY (id)) ENGINE=EEE'; +EXECUTE IMMEDIATE REPLACE(REPLACE(query,'TTT', t), 'EEE', engine); +SHOW CREATE TABLE t1; +INSERT INTO t1 (name) VALUES ('dog'); +SELECT * FROM t1; +UPDATE t1 SET id=-1 WHERE id=1; +SELECT * FROM t1; +INSERT INTO t1 (name) VALUES ('cat'); +SELECT * FROM t1; +DROP TABLE t1; +END; +$$ +CALL autoinc_mdev15353_one(@engine, 'tinyint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` tinyint(4) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'smallint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` smallint(6) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'mediumint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` mediumint(9) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'int'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'bigint'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` bigint(20) NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'float'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` float NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +CALL autoinc_mdev15353_one(@engine, 'double'); +Table Create Table +t1 CREATE TABLE `t1` ( + `id` double NOT NULL AUTO_INCREMENT, + `name` char(30) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 +id name +1 dog +id name +-1 dog +id name +-1 dog +2 cat +DROP PROCEDURE autoinc_mdev15353_one; diff --git a/mysql-test/suite/maria/maria-autoinc.test b/mysql-test/suite/maria/maria-autoinc.test new file mode 100644 index 00000000000..e7dc10b503b --- /dev/null +++ b/mysql-test/suite/maria/maria-autoinc.test @@ -0,0 +1,8 @@ +-- source include/have_maria.inc + +--echo # +--echo # MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +--echo # + +SET @engine='ARIA'; +--source include/autoinc_mdev15353.inc diff --git a/mysql-test/t/auto_increment.test b/mysql-test/t/auto_increment.test index 7f0ab5dc169..6f678ed309f 100644 --- a/mysql-test/t/auto_increment.test +++ b/mysql-test/t/auto_increment.test @@ -397,3 +397,11 @@ insert into t1 values(null); select last_insert_id(); select * from t1; drop table t1; + + +--echo # +--echo # MDEV-15352 AUTO_INCREMENT breaks after updating a column value to a negative number +--echo # + +SET @engine='MyISAM'; +--source include/autoinc_mdev15353.inc diff --git a/sql/field.cc b/sql/field.cc index a8d82170d52..56948acd8ba 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -3328,6 +3328,16 @@ longlong Field_new_decimal::val_int(void) } +ulonglong Field_new_decimal::val_uint(void) +{ + ASSERT_COLUMN_MARKED_FOR_READ; + longlong i; + my_decimal decimal_value; + my_decimal2int(E_DEC_FATAL_ERROR, val_decimal(&decimal_value), true, &i); + return i; +} + + my_decimal* Field_new_decimal::val_decimal(my_decimal *decimal_value) { ASSERT_COLUMN_MARKED_FOR_READ; diff --git a/sql/field.h b/sql/field.h index 4bf92faecdd..ca85a7c276f 100644 --- a/sql/field.h +++ b/sql/field.h @@ -851,9 +851,14 @@ public: { return store(ls->str, (uint32) ls->length, cs); } virtual double val_real(void)=0; virtual longlong val_int(void)=0; + /* + Get ulonglong representation. + Negative values are truncated to 0. + */ virtual ulonglong val_uint(void) { - return (ulonglong) val_int(); + longlong nr= val_int(); + return nr < 0 ? 0 : (ulonglong) nr; } virtual bool val_bool(void)= 0; virtual my_decimal *val_decimal(my_decimal *); @@ -1898,6 +1903,7 @@ public: int store_decimal(const my_decimal *); double val_real(void); longlong val_int(void); + ulonglong val_uint(void); my_decimal *val_decimal(my_decimal *); String *val_str(String*, String *); bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); @@ -1925,15 +1931,34 @@ public: }; -class Field_tiny :public Field_num { +class Field_integer: public Field_num +{ +public: + Field_integer(uchar *ptr_arg, uint32 len_arg, + uchar *null_ptr_arg, uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, 0, + zero_arg, unsigned_arg) + { } + ulonglong val_uint() + { + longlong nr= val_int(); + return nr < 0 && !unsigned_flag ? 0 : (ulonglong) nr; + } +}; + + +class Field_tiny :public Field_integer { public: Field_tiny(uchar *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, - uchar null_bit_arg, - enum utype unireg_check_arg, const char *field_name_arg, - bool zero_arg, bool unsigned_arg) - :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, - unireg_check_arg, field_name_arg, - 0, zero_arg,unsigned_arg) + uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_integer(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, + zero_arg, unsigned_arg) {} enum_field_types type() const { return MYSQL_TYPE_TINY;} enum ha_base_keytype key_type() const @@ -1969,20 +1994,20 @@ public: }; -class Field_short :public Field_num { +class Field_short :public Field_integer { public: Field_short(uchar *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, - uchar null_bit_arg, - enum utype unireg_check_arg, const char *field_name_arg, - bool zero_arg, bool unsigned_arg) - :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, - unireg_check_arg, field_name_arg, - 0, zero_arg,unsigned_arg) + uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_integer(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, + zero_arg, unsigned_arg) {} Field_short(uint32 len_arg,bool maybe_null_arg, const char *field_name_arg, - bool unsigned_arg) - :Field_num((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "": 0,0, - NONE, field_name_arg, 0, 0, unsigned_arg) + bool unsigned_arg) + :Field_integer((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "" : 0, 0, + NONE, field_name_arg, 0, unsigned_arg) {} enum_field_types type() const { return MYSQL_TYPE_SHORT;} enum ha_base_keytype key_type() const @@ -2009,15 +2034,15 @@ public: { return unpack_int16(to, from, from_end); } }; -class Field_medium :public Field_num { +class Field_medium :public Field_integer { public: Field_medium(uchar *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, - uchar null_bit_arg, - enum utype unireg_check_arg, const char *field_name_arg, - bool zero_arg, bool unsigned_arg) - :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, - unireg_check_arg, field_name_arg, - 0, zero_arg,unsigned_arg) + uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_integer(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, + zero_arg, unsigned_arg) {} enum_field_types type() const { return MYSQL_TYPE_INT24;} enum ha_base_keytype key_type() const @@ -2043,20 +2068,20 @@ public: }; -class Field_long :public Field_num { +class Field_long :public Field_integer { public: Field_long(uchar *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, - uchar null_bit_arg, - enum utype unireg_check_arg, const char *field_name_arg, - bool zero_arg, bool unsigned_arg) - :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, - unireg_check_arg, field_name_arg, - 0, zero_arg,unsigned_arg) + uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_integer(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, + zero_arg, unsigned_arg) {} Field_long(uint32 len_arg,bool maybe_null_arg, const char *field_name_arg, - bool unsigned_arg) - :Field_num((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "": 0,0, - NONE, field_name_arg,0,0,unsigned_arg) + bool unsigned_arg) + :Field_integer((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "" : 0, 0, + NONE, field_name_arg, 0, unsigned_arg) {} enum_field_types type() const { return MYSQL_TYPE_LONG;} enum ha_base_keytype key_type() const @@ -2088,21 +2113,21 @@ public: }; -class Field_longlong :public Field_num { +class Field_longlong :public Field_integer { public: Field_longlong(uchar *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, - uchar null_bit_arg, - enum utype unireg_check_arg, const char *field_name_arg, - bool zero_arg, bool unsigned_arg) - :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, - unireg_check_arg, field_name_arg, - 0, zero_arg,unsigned_arg) + uchar null_bit_arg, + enum utype unireg_check_arg, const char *field_name_arg, + bool zero_arg, bool unsigned_arg) + :Field_integer(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, + unireg_check_arg, field_name_arg, + zero_arg, unsigned_arg) {} Field_longlong(uint32 len_arg,bool maybe_null_arg, - const char *field_name_arg, - bool unsigned_arg) - :Field_num((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "": 0,0, - NONE, field_name_arg,0,0,unsigned_arg) + const char *field_name_arg, + bool unsigned_arg) + :Field_integer((uchar*) 0, len_arg, maybe_null_arg ? (uchar*) "" : 0, 0, + NONE, field_name_arg,0, unsigned_arg) {} enum_field_types type() const { return MYSQL_TYPE_LONGLONG;} enum ha_base_keytype key_type() const From 2ca904f0ca981666b90151ff3fcdad4307b96a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 13 Jun 2018 16:15:21 +0300 Subject: [PATCH 12/15] MDEV-13103 Deal with page_compressed page corruption fil_page_decompress(): Replaces fil_decompress_page(). Allow the caller detect errors. Remove duplicated code. Use the "safe" instead of "fast" variants of decompression routines. fil_page_compress(): Replaces fil_compress_page(). The length of the input buffer always was srv_page_size (innodb_page_size). Remove printouts, and remove the fil_space_t* parameter. buf_tmp_buffer_t::reserved: Make private; the accessors acquire() and release() will use atomic memory access. buf_pool_reserve_tmp_slot(): Make static. Remove the second parameter. Do not acquire any mutex. Remove the allocation of the buffers. buf_tmp_reserve_crypt_buf(), buf_tmp_reserve_compression_buf(): Refactored away from buf_pool_reserve_tmp_slot(). buf_page_decrypt_after_read(): Make static, and simplify the logic. Use the encryption buffer also for decompressing. buf_page_io_complete(), buf_dblwr_process(): Check more failures. fil_space_encrypt(): Simplify the debug checks. fil_space_t::printed_compression_failure: Remove. fil_get_compression_alg_name(): Remove. fil_iterate(): Allocate a buffer for compression and decompression only once, instead of allocating and freeing it for every page that uses compression, during IMPORT TABLESPACE. Also, validate the page checksum before decryption, and reduce the scope of some variables. fil_page_is_index_page(), fil_page_is_lzo_compressed(): Remove (unused). AbstractCallback::operator()(): Remove the parameter 'offset'. The check for it in FetchIndexRootPages::operator() was basically redundant and dead code since the previous refactoring. --- .../r/innodb-page_compression_default.result | 1 - .../r/innodb-page_compression_snappy.result | 1 - .../t/innodb-page_compression_default.test | 2 - .../t/innodb-page_compression_snappy.test | 2 - storage/innobase/buf/buf0buf.cc | 352 +++++------ storage/innobase/buf/buf0dblwr.cc | 34 +- storage/innobase/fil/fil0crypt.cc | 59 +- storage/innobase/fil/fil0pagecompress.cc | 575 +++++------------- storage/innobase/ibuf/ibuf0ibuf.cc | 5 + storage/innobase/include/buf0buf.h | 20 +- storage/innobase/include/fil0fil.h | 3 - storage/innobase/include/fil0pagecompress.h | 56 +- storage/innobase/include/fsp0pagecompress.ic | 70 +-- storage/innobase/row/row0import.cc | 180 +++--- 14 files changed, 503 insertions(+), 857 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb-page_compression_default.result b/mysql-test/suite/innodb/r/innodb-page_compression_default.result index 413450e1a6d..ca7129fd3eb 100644 --- a/mysql-test/suite/innodb/r/innodb-page_compression_default.result +++ b/mysql-test/suite/innodb/r/innodb-page_compression_default.result @@ -1,4 +1,3 @@ -call mtr.add_suppression("InnoDB: Compression failed for space [0-9]+ name test/innodb_page_compressed[0-9] len [0-9]+ err 2 write_size [0-9]+."); set global innodb_file_format = `Barracuda`; set global innodb_file_per_table = on; create table innodb_normal (c1 int not null auto_increment primary key, b char(200)) engine=innodb; diff --git a/mysql-test/suite/innodb/r/innodb-page_compression_snappy.result b/mysql-test/suite/innodb/r/innodb-page_compression_snappy.result index 83a17f678e4..6e0ee94e69e 100644 --- a/mysql-test/suite/innodb/r/innodb-page_compression_snappy.result +++ b/mysql-test/suite/innodb/r/innodb-page_compression_snappy.result @@ -1,4 +1,3 @@ -call mtr.add_suppression("InnoDB: Compression failed for space [0-9]+ name test/innodb_page_compressed[0-9] len [0-9]+ err 2 write_size [0-9]+."); set global innodb_compression_algorithm = snappy; set global innodb_file_format = `Barracuda`; set global innodb_file_per_table = on; diff --git a/mysql-test/suite/innodb/t/innodb-page_compression_default.test b/mysql-test/suite/innodb/t/innodb-page_compression_default.test index 1cc6c917548..34b1829485e 100644 --- a/mysql-test/suite/innodb/t/innodb-page_compression_default.test +++ b/mysql-test/suite/innodb/t/innodb-page_compression_default.test @@ -1,8 +1,6 @@ --source include/have_innodb.inc --source include/not_embedded.inc -call mtr.add_suppression("InnoDB: Compression failed for space [0-9]+ name test/innodb_page_compressed[0-9] len [0-9]+ err 2 write_size [0-9]+."); - # All page compression test use the same --source include/innodb-page-compression.inc diff --git a/mysql-test/suite/innodb/t/innodb-page_compression_snappy.test b/mysql-test/suite/innodb/t/innodb-page_compression_snappy.test index 532ec294d28..0186c24ef2e 100644 --- a/mysql-test/suite/innodb/t/innodb-page_compression_snappy.test +++ b/mysql-test/suite/innodb/t/innodb-page_compression_snappy.test @@ -2,8 +2,6 @@ -- source include/have_innodb_snappy.inc --source include/not_embedded.inc -call mtr.add_suppression("InnoDB: Compression failed for space [0-9]+ name test/innodb_page_compressed[0-9] len [0-9]+ err 2 write_size [0-9]+."); - # snappy set global innodb_compression_algorithm = snappy; diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 845bb17bfa5..30fe1f8f616 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -423,6 +423,53 @@ on the io_type */ ? (counter##_READ) \ : (counter##_WRITTEN)) + +/** Reserve a buffer slot for encryption, decryption or page compression. +@param[in,out] buf_pool buffer pool +@return reserved buffer slot */ +static buf_tmp_buffer_t* buf_pool_reserve_tmp_slot(buf_pool_t* buf_pool) +{ + for (ulint i = 0; i < buf_pool->tmp_arr->n_slots; i++) { + buf_tmp_buffer_t* slot = &buf_pool->tmp_arr->slots[i]; + if (slot->acquire()) { + return slot; + } + } + + /* We assume that free slot is found */ + ut_error; + return NULL; +} + +/** Reserve a buffer for encryption, decryption or decompression. +@param[in,out] slot reserved slot */ +static void buf_tmp_reserve_crypt_buf(buf_tmp_buffer_t* slot) +{ + if (!slot->crypt_buf) { + slot->crypt_buf = static_cast( + aligned_malloc(srv_page_size, srv_page_size)); + } +} + +/** Reserve a buffer for compression. +@param[in,out] slot reserved slot */ +static void buf_tmp_reserve_compression_buf(buf_tmp_buffer_t* slot) +{ + if (!slot->comp_buf) { + /* Both snappy and lzo compression methods require that + output buffer used for compression is bigger than input + buffer. Increase the allocated buffer size accordingly. */ + ulint size = srv_page_size; +#ifdef HAVE_LZO + size += LZO1X_1_15_MEM_COMPRESS; +#elif defined HAVE_SNAPPY + size = snappy_max_compressed_length(size); +#endif + slot->comp_buf = static_cast( + aligned_malloc(size, srv_page_size)); + } +} + /** Registers a chunk to buf_pool_chunk_map @param[in] chunk chunk of buffers */ static @@ -438,10 +485,91 @@ buf_pool_register_chunk( @param[in,out] bpage Page control block @param[in,out] space tablespace @return whether the operation was successful */ -static -bool -buf_page_decrypt_after_read(buf_page_t* bpage, fil_space_t* space) - MY_ATTRIBUTE((nonnull)); +static bool buf_page_decrypt_after_read(buf_page_t* bpage, fil_space_t* space) +{ + ut_ad(space->n_pending_ios > 0); + ut_ad(space->id == bpage->id.space()); + + byte* dst_frame = bpage->zip.data ? bpage->zip.data : + ((buf_block_t*) bpage)->frame; + bool page_compressed = fil_page_is_compressed(dst_frame); + buf_pool_t* buf_pool = buf_pool_from_bpage(bpage); + + if (bpage->id.page_no() == 0) { + /* File header pages are not encrypted/compressed */ + return (true); + } + + /* Page is encrypted if encryption information is found from + tablespace and page contains used key_version. This is true + also for pages first compressed and then encrypted. */ + + buf_tmp_buffer_t* slot; + + if (page_compressed) { + /* the page we read is unencrypted */ + /* Find free slot from temporary memory array */ +decompress: + slot = buf_pool_reserve_tmp_slot(buf_pool); + /* For decompression, use crypt_buf. */ + buf_tmp_reserve_crypt_buf(slot); +decompress_with_slot: + ut_d(fil_page_type_validate(dst_frame)); + + bpage->write_size = fil_page_decompress(slot->crypt_buf, + dst_frame); + slot->release(); + + ut_ad(!bpage->write_size || fil_page_type_validate(dst_frame)); + ut_ad(space->n_pending_ios > 0); + return bpage->write_size != 0; + } + + if (space->crypt_data + && mach_read_from_4(FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION + + dst_frame)) { + /* Verify encryption checksum before we even try to + decrypt. */ + if (!fil_space_verify_crypt_checksum( + dst_frame, bpage->size, bpage->id.space(), + bpage->id.page_no())) { +decrypt_failed: + /* Mark page encrypted in case it should be. */ + if (space->crypt_data->type + != CRYPT_SCHEME_UNENCRYPTED) { + bpage->encrypted = true; + } + + return false; + } + + /* Find free slot from temporary memory array */ + slot = buf_pool_reserve_tmp_slot(buf_pool); + buf_tmp_reserve_crypt_buf(slot); + + ut_d(fil_page_type_validate(dst_frame)); + + /* decrypt using crypt_buf to dst_frame */ + if (!fil_space_decrypt(space, slot->crypt_buf, + dst_frame, &bpage->encrypted)) { + slot->release(); + goto decrypt_failed; + } + + ut_d(fil_page_type_validate(dst_frame)); + + if (fil_page_is_compressed_encrypted(dst_frame)) { + goto decompress_with_slot; + } + + slot->release(); + } else if (fil_page_is_compressed_encrypted(dst_frame)) { + goto decompress; + } + + ut_ad(space->n_pending_ios > 0); + return true; +} /* prototypes for new functions added to ha_innodb.cc */ trx_t* innobase_get_trx(); @@ -5931,21 +6059,23 @@ buf_page_io_complete(buf_page_t* bpage, bool dblwr, bool evict) ulint read_page_no = 0; ulint read_space_id = 0; uint key_version = 0; - - ut_ad(bpage->zip.data != NULL || ((buf_block_t*)bpage)->frame != NULL); + byte* frame = bpage->zip.data + ? bpage->zip.data + : reinterpret_cast(bpage)->frame; + ut_ad(frame); fil_space_t* space = fil_space_acquire_for_io( bpage->id.space()); if (!space) { return DB_TABLESPACE_DELETED; } - buf_page_decrypt_after_read(bpage, space); - - byte* frame = bpage->zip.data - ? bpage->zip.data - : reinterpret_cast(bpage)->frame; dberr_t err; + if (!buf_page_decrypt_after_read(bpage, space)) { + err = DB_DECRYPTION_FAILED; + goto database_corrupted; + } + if (bpage->zip.data && uncompressed) { my_atomic_addlint(&buf_pool->n_pend_unzip, 1); ibool ok = buf_zip_decompress((buf_block_t*) bpage, @@ -6097,7 +6227,7 @@ database_corrupted: /* io_type == BUF_IO_WRITE */ if (bpage->slot) { /* Mark slot free */ - bpage->slot->reserved = false; + bpage->slot->release(); bpage->slot = NULL; } } @@ -7268,66 +7398,6 @@ operator<<( return(out); } -/********************************************************************//** -Reserve unused slot from temporary memory array and allocate necessary -temporary memory if not yet allocated. -@return reserved slot */ -UNIV_INTERN -buf_tmp_buffer_t* -buf_pool_reserve_tmp_slot( -/*======================*/ - buf_pool_t* buf_pool, /*!< in: buffer pool where to - reserve */ - bool compressed) /*!< in: is file space compressed */ -{ - buf_tmp_buffer_t *free_slot=NULL; - - /* Array is protected by buf_pool mutex */ - buf_pool_mutex_enter(buf_pool); - - for(ulint i = 0; i < buf_pool->tmp_arr->n_slots; i++) { - buf_tmp_buffer_t *slot = &buf_pool->tmp_arr->slots[i]; - - if(slot->reserved == false) { - free_slot = slot; - break; - } - } - - /* We assume that free slot is found */ - ut_a(free_slot != NULL); - free_slot->reserved = true; - /* Now that we have reserved this slot we can release - buf_pool mutex */ - buf_pool_mutex_exit(buf_pool); - - /* Allocate temporary memory for encryption/decryption */ - if (free_slot->crypt_buf == NULL) { - free_slot->crypt_buf = static_cast(aligned_malloc(UNIV_PAGE_SIZE, UNIV_PAGE_SIZE)); - memset(free_slot->crypt_buf, 0, UNIV_PAGE_SIZE); - } - - /* For page compressed tables allocate temporary memory for - compression/decompression */ - if (compressed && free_slot->comp_buf == NULL) { - ulint size = UNIV_PAGE_SIZE; - - /* Both snappy and lzo compression methods require that - output buffer used for compression is bigger than input - buffer. Increase the allocated buffer size accordingly. */ -#if HAVE_SNAPPY - size = snappy_max_compressed_length(size); -#endif -#if HAVE_LZO - size += LZO1X_1_15_MEM_COMPRESS; -#endif - free_slot->comp_buf = static_cast(aligned_malloc(size, UNIV_PAGE_SIZE)); - memset(free_slot->comp_buf, 0, size); - } - - return (free_slot); -} - /** Encryption and page_compression hook that is called just before a page is written to disk. @param[in,out] space tablespace @@ -7376,15 +7446,18 @@ buf_page_encrypt_before_write( return src_frame; } + ut_ad(!bpage->size.is_compressed() || !page_compressed); buf_pool_t* buf_pool = buf_pool_from_bpage(bpage); /* Find free slot from temporary memory array */ - buf_tmp_buffer_t* slot = buf_pool_reserve_tmp_slot(buf_pool, page_compressed); + buf_tmp_buffer_t* slot = buf_pool_reserve_tmp_slot(buf_pool); slot->out_buf = NULL; bpage->slot = slot; + buf_tmp_reserve_crypt_buf(slot); byte *dst_frame = slot->crypt_buf; if (!page_compressed) { +not_compressed: /* Encrypt page content */ byte* tmp = fil_space_encrypt(space, bpage->id.page_no(), @@ -7392,33 +7465,30 @@ buf_page_encrypt_before_write( src_frame, dst_frame); + bpage->real_size = UNIV_PAGE_SIZE; slot->out_buf = dst_frame = tmp; ut_d(fil_page_type_validate(tmp)); } else { /* First we compress the page content */ - ulint out_len = 0; - - byte *tmp = fil_compress_page( - space, - (byte *)src_frame, - slot->comp_buf, - srv_page_size, + buf_tmp_reserve_compression_buf(slot); + byte* tmp = slot->comp_buf; + ulint out_len = fil_page_compress( + src_frame, tmp, fsp_flags_get_page_compression_level(space->flags), fil_space_get_block_size(space, bpage->id.page_no()), - encrypted, - &out_len); + encrypted); + if (!out_len) { + goto not_compressed; + } bpage->real_size = out_len; /* Workaround for MDEV-15527. */ memset(tmp + out_len, 0 , srv_page_size - out_len); -#ifdef UNIV_DEBUG - fil_page_type_validate(tmp); -#endif - - if(encrypted) { + ut_d(fil_page_type_validate(tmp)); + if (encrypted) { /* And then we encrypt the page content */ tmp = fil_space_encrypt(space, bpage->id.page_no(), @@ -7436,112 +7506,6 @@ buf_page_encrypt_before_write( return dst_frame; } -/** Decrypt a page. -@param[in,out] bpage Page control block -@param[in,out] space tablespace -@return whether the operation was successful */ -static -bool -buf_page_decrypt_after_read(buf_page_t* bpage, fil_space_t* space) -{ - ut_ad(space->n_pending_ios > 0); - ut_ad(space->id == bpage->id.space()); - - bool compressed = bpage->size.is_compressed(); - const page_size_t& size = bpage->size; - byte* dst_frame = compressed ? bpage->zip.data : - ((buf_block_t*) bpage)->frame; - unsigned key_version = - mach_read_from_4(dst_frame + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - bool page_compressed = fil_page_is_compressed(dst_frame); - bool page_compressed_encrypted = fil_page_is_compressed_encrypted(dst_frame); - buf_pool_t* buf_pool = buf_pool_from_bpage(bpage); - bool success = true; - - if (bpage->id.page_no() == 0) { - /* File header pages are not encrypted/compressed */ - return (true); - } - - /* Page is encrypted if encryption information is found from - tablespace and page contains used key_version. This is true - also for pages first compressed and then encrypted. */ - if (!space->crypt_data) { - key_version = 0; - } - - if (page_compressed) { - /* the page we read is unencrypted */ - /* Find free slot from temporary memory array */ - buf_tmp_buffer_t* slot = buf_pool_reserve_tmp_slot(buf_pool, page_compressed); - - ut_d(fil_page_type_validate(dst_frame)); - - /* decompress using comp_buf to dst_frame */ - fil_decompress_page(slot->comp_buf, - dst_frame, - ulong(size.logical()), - &bpage->write_size); - - /* Mark this slot as free */ - slot->reserved = false; - key_version = 0; - - ut_d(fil_page_type_validate(dst_frame)); - } else { - buf_tmp_buffer_t* slot = NULL; - - if (key_version) { - /* Verify encryption checksum before we even try to - decrypt. */ - if (!fil_space_verify_crypt_checksum( - dst_frame, size, - bpage->id.space(), bpage->id.page_no())) { - if (space->crypt_data->type - != CRYPT_SCHEME_UNENCRYPTED) { - bpage->encrypted = true; - } - return (false); - } - - /* Find free slot from temporary memory array */ - slot = buf_pool_reserve_tmp_slot(buf_pool, page_compressed); - - ut_d(fil_page_type_validate(dst_frame)); - - /* decrypt using crypt_buf to dst_frame */ - if (!fil_space_decrypt(space, slot->crypt_buf, - dst_frame, &bpage->encrypted)) { - success = false; - } - - ut_d(fil_page_type_validate(dst_frame)); - } - - if (page_compressed_encrypted && success) { - if (!slot) { - slot = buf_pool_reserve_tmp_slot(buf_pool, page_compressed); - } - - ut_d(fil_page_type_validate(dst_frame)); - /* decompress using comp_buf to dst_frame */ - fil_decompress_page(slot->comp_buf, - dst_frame, - ulong(size.logical()), - &bpage->write_size); - ut_d(fil_page_type_validate(dst_frame)); - } - - /* Mark this slot as free */ - if (slot) { - slot->reserved = false; - } - } - - ut_ad(space->n_pending_ios > 0); - return (success); -} - /** Should we punch hole to deallocate unused portion of the page. @param[in] bpage Page control block @@ -7565,6 +7529,4 @@ buf_page_get_trim_length( { return (bpage->size.physical() - write_length); } - - #endif /* !UNIV_INNOCHECKSUM */ diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc index 96000c4eb92..f6a325a15fb 100644 --- a/storage/innobase/buf/buf0dblwr.cc +++ b/storage/innobase/buf/buf0dblwr.cc @@ -533,10 +533,11 @@ buf_dblwr_process() } unaligned_read_buf = static_cast( - ut_malloc_nokey(2 * UNIV_PAGE_SIZE)); + ut_malloc_nokey(3 * UNIV_PAGE_SIZE)); read_buf = static_cast( ut_align(unaligned_read_buf, UNIV_PAGE_SIZE)); + byte* const buf = read_buf + UNIV_PAGE_SIZE; for (recv_dblwr_t::list::iterator i = recv_dblwr.pages.begin(); i != recv_dblwr.pages.end(); @@ -604,24 +605,24 @@ buf_dblwr_process() ignore this page (there should be redo log records to initialize it). */ } else { - if (fil_page_is_compressed_encrypted(read_buf) || - fil_page_is_compressed(read_buf)) { - /* Decompress the page before - validating the checksum. */ - fil_decompress_page( - NULL, read_buf, srv_page_size, - NULL, true); + /* Decompress the page before + validating the checksum. */ + ulint decomp = fil_page_decompress(buf, read_buf); + if (!decomp || (decomp != srv_page_size + && page_size.is_compressed())) { + goto bad; } if (fil_space_verify_crypt_checksum( read_buf, page_size, space_id, page_no) - || !buf_page_is_corrupted( - true, read_buf, page_size, space)) { + || !buf_page_is_corrupted( + true, read_buf, page_size, space)) { /* The page is good; there is no need to consult the doublewrite buffer. */ continue; } +bad: /* We intentionally skip this message for is_all_zero pages. */ ib::info() @@ -629,19 +630,16 @@ buf_dblwr_process() << " from the doublewrite buffer."; } - /* Next, validate the doublewrite page. */ - if (fil_page_is_compressed_encrypted(page) || - fil_page_is_compressed(page)) { - /* Decompress the page before - validating the checksum. */ - fil_decompress_page( - NULL, page, srv_page_size, NULL, true); + ulint decomp = fil_page_decompress(buf, page); + if (!decomp || (decomp != srv_page_size + && page_size.is_compressed())) { + goto bad_doublewrite; } - if (!fil_space_verify_crypt_checksum(page, page_size, space_id, page_no) && buf_page_is_corrupted(true, page, page_size, space)) { if (!is_all_zero) { +bad_doublewrite: ib::warn() << "A doublewrite copy of page " << page_id << " is corrupted."; } diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 4c53441df58..3c9640c8e15 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -665,57 +665,38 @@ fil_space_encrypt( #ifdef UNIV_DEBUG if (tmp) { /* Verify that encrypted buffer is not corrupted */ - byte* tmp_mem = (byte *)malloc(UNIV_PAGE_SIZE); dberr_t err = DB_SUCCESS; byte* src = src_frame; bool page_compressed_encrypted = (mach_read_from_2(tmp+FIL_PAGE_TYPE) == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED); - byte* comp_mem = NULL; - byte* uncomp_mem = NULL; + byte uncomp_mem[UNIV_PAGE_SIZE_MAX]; + byte tmp_mem[UNIV_PAGE_SIZE_MAX]; if (page_compressed_encrypted) { - comp_mem = (byte *)malloc(UNIV_PAGE_SIZE); - uncomp_mem = (byte *)malloc(UNIV_PAGE_SIZE); - memcpy(comp_mem, src_frame, UNIV_PAGE_SIZE); - fil_decompress_page(uncomp_mem, comp_mem, - srv_page_size, NULL); - src = uncomp_mem; + memcpy(uncomp_mem, src, srv_page_size); + ulint unzipped1 = fil_page_decompress( + tmp_mem, uncomp_mem); + ut_ad(unzipped1); + if (unzipped1 != srv_page_size) { + src = uncomp_mem; + } } - bool corrupted1 = buf_page_is_corrupted(true, src, page_size, space); - bool ok = fil_space_decrypt(crypt_data, tmp_mem, page_size, tmp, &err); + ut_ad(!buf_page_is_corrupted(true, src, page_size, space)); + ut_ad(fil_space_decrypt(crypt_data, tmp_mem, page_size, tmp, + &err)); + ut_ad(err == DB_SUCCESS); /* Need to decompress the page if it was also compressed */ if (page_compressed_encrypted) { - memcpy(comp_mem, tmp_mem, UNIV_PAGE_SIZE); - fil_decompress_page(tmp_mem, comp_mem, - srv_page_size, NULL); + byte buf[UNIV_PAGE_SIZE_MAX]; + memcpy(buf, tmp_mem, srv_page_size); + ulint unzipped2 = fil_page_decompress(tmp_mem, buf); + ut_ad(unzipped2); } - bool corrupted = buf_page_is_corrupted(true, tmp_mem, page_size, space); - memcpy(tmp_mem+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION, src+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION, 8); - bool different = memcmp(src, tmp_mem, page_size.physical()); - - if (!ok || corrupted || corrupted1 || err != DB_SUCCESS || different) { - fprintf(stderr, "ok %d corrupted %d corrupted1 %d err %d different %d\n", - ok , corrupted, corrupted1, err, different); - fprintf(stderr, "src_frame\n"); - buf_page_print(src_frame, page_size); - fprintf(stderr, "encrypted_frame\n"); - buf_page_print(tmp, page_size); - fprintf(stderr, "decrypted_frame\n"); - buf_page_print(tmp_mem, page_size); - ut_ad(0); - } - - free(tmp_mem); - - if (comp_mem) { - free(comp_mem); - } - - if (uncomp_mem) { - free(uncomp_mem); - } + memcpy(tmp_mem + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION, + src + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION, 8); + ut_ad(!memcmp(src, tmp_mem, page_size.physical())); } #endif /* UNIV_DEBUG */ diff --git a/storage/innobase/fil/fil0pagecompress.cc b/storage/innobase/fil/fil0pagecompress.cc index cddeb6a2d28..491de512156 100644 --- a/storage/innobase/fil/fil0pagecompress.cc +++ b/storage/innobase/fil/fil0pagecompress.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (C) 2013, 2017, MariaDB Corporation. +Copyright (C) 2013, 2018, 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 @@ -74,73 +74,26 @@ Updated 14/02/2015 #include "snappy-c.h" #endif -/* Used for debugging */ -//#define UNIV_PAGECOMPRESS_DEBUG 1 - -/****************************************************************//** -For page compressed pages compress the page before actual write -operation. -@return compressed page to be written*/ -UNIV_INTERN -byte* -fil_compress_page( -/*==============*/ - fil_space_t* space, /*!< in,out: tablespace (NULL during IMPORT) */ - byte* buf, /*!< in: buffer from which to write; in aio - this must be appropriately aligned */ - byte* out_buf, /*!< out: compressed buffer */ - ulint len, /*!< in: length of input buffer.*/ - ulint level, /* in: compression level */ - ulint block_size, /*!< in: block size */ - bool encrypted, /*!< in: is page also encrypted */ - ulint* out_len) /*!< out: actual length of compressed - page */ +/** Compress a page_compressed page before writing to a data file. +@param[in] buf page to be compressed +@param[out] out_buf compressed page +@param[in] level compression level +@param[in] block_size file system block size +@param[in] encrypted whether the page will be subsequently encrypted +@return actual length of compressed page +@retval 0 if the page was not compressed */ +ulint fil_page_compress(const byte* buf, byte* out_buf, ulint level, + ulint block_size, bool encrypted) { - int err = Z_OK; int comp_level = int(level); ulint header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE; - ulint write_size = 0; -#if HAVE_LZO - lzo_uint write_size_lzo = write_size; -#endif /* Cache to avoid change during function execution */ ulint comp_method = innodb_compression_algorithm; - bool allocated = false; - - /* page_compression does not apply to tables or tablespaces - that use ROW_FORMAT=COMPRESSED */ - ut_ad(!space || !FSP_FLAGS_GET_ZIP_SSIZE(space->flags)); if (encrypted) { header_len += FIL_PAGE_COMPRESSION_METHOD_SIZE; } - if (!out_buf) { - allocated = true; - ulint size = UNIV_PAGE_SIZE; - - /* Both snappy and lzo compression methods require that - output buffer used for compression is bigger than input - buffer. Increase the allocated buffer size accordingly. */ -#if HAVE_SNAPPY - if (comp_method == PAGE_SNAPPY_ALGORITHM) { - size = snappy_max_compressed_length(size); - } -#endif -#if HAVE_LZO - if (comp_method == PAGE_LZO_ALGORITHM) { - size += LZO1X_1_15_MEM_COMPRESS; - } -#endif - - out_buf = static_cast(ut_malloc_nokey(size)); - } - - ut_ad(buf); - ut_ad(out_buf); - ut_ad(len); - ut_ad(out_len); - /* Let's not compress file space header or extent descriptor */ switch (fil_page_get_type(buf)) { @@ -148,8 +101,7 @@ fil_compress_page( case FIL_PAGE_TYPE_FSP_HDR: case FIL_PAGE_TYPE_XDES: case FIL_PAGE_PAGE_COMPRESSED: - *out_len = len; - goto err_exit; + return 0; } /* If no compression level was provided to this table, use system @@ -158,125 +110,113 @@ fil_compress_page( comp_level = page_zip_level; } - DBUG_LOG("compress", "Preparing for space " - << (space ? space->id : 0) << " '" - << (space ? space->name : "(import)") << "' len " << len); + ulint write_size = srv_page_size - header_len; - write_size = UNIV_PAGE_SIZE - header_len; - - switch(comp_method) { + switch (comp_method) { + default: + ut_ad(!"unknown compression method"); + /* fall through */ + case PAGE_UNCOMPRESSED: + return 0; + case PAGE_ZLIB_ALGORITHM: + { + ulong len = uLong(write_size); + if (Z_OK == compress2( + out_buf + header_len, &len, + buf, uLong(srv_page_size), comp_level)) { + write_size = len; + goto success; + } + } + break; #ifdef HAVE_LZ4 case PAGE_LZ4_ALGORITHM: +# ifdef HAVE_LZ4_COMPRESS_DEFAULT + write_size = LZ4_compress_default( + reinterpret_cast(buf), + reinterpret_cast(out_buf) + header_len, + int(srv_page_size), int(write_size)); +# else + write_size = LZ4_compress_limitedOutput( + reinterpret_cast(buf), + reinterpret_cast(out_buf) + header_len, + int(srv_page_size), int(write_size)); +# endif -#ifdef HAVE_LZ4_COMPRESS_DEFAULT - err = LZ4_compress_default((const char *)buf, - (char *)out_buf+header_len, len, write_size); -#else - err = LZ4_compress_limitedOutput((const char *)buf, - (char *)out_buf+header_len, len, write_size); -#endif /* HAVE_LZ4_COMPRESS_DEFAULT */ - write_size = err; - - if (err == 0) { - goto err_exit; + if (write_size) { + goto success; } break; #endif /* HAVE_LZ4 */ #ifdef HAVE_LZO - case PAGE_LZO_ALGORITHM: - err = lzo1x_1_15_compress( - buf, len, out_buf+header_len, &write_size_lzo, out_buf+UNIV_PAGE_SIZE); + case PAGE_LZO_ALGORITHM: { + lzo_uint len = write_size; - write_size = write_size_lzo; - - if (err != LZO_E_OK || write_size > UNIV_PAGE_SIZE-header_len) { - goto err_exit; + if (LZO_E_OK == lzo1x_1_15_compress( + buf, srv_page_size, + out_buf + header_len, &len, + out_buf + srv_page_size) + && len <= write_size) { + write_size = len; + goto success; } - break; + } #endif /* HAVE_LZO */ #ifdef HAVE_LZMA case PAGE_LZMA_ALGORITHM: { - size_t out_pos=0; + size_t out_pos = 0; - err = lzma_easy_buffer_encode( - comp_level, - LZMA_CHECK_NONE, - NULL, /* No custom allocator, use malloc/free */ - reinterpret_cast(buf), - len, - reinterpret_cast(out_buf + header_len), - &out_pos, - (size_t)write_size); - - if (err != LZMA_OK || out_pos > UNIV_PAGE_SIZE-header_len) { + if (LZMA_OK == lzma_easy_buffer_encode( + comp_level, LZMA_CHECK_NONE, NULL, + buf, srv_page_size, out_buf + header_len, + &out_pos, write_size) + && out_pos <= write_size) { write_size = out_pos; - goto err_exit; + goto success; } - - write_size = out_pos; - break; } #endif /* HAVE_LZMA */ #ifdef HAVE_BZIP2 case PAGE_BZIP2_ALGORITHM: { - - err = BZ2_bzBuffToBuffCompress( - (char *)(out_buf + header_len), - (unsigned int *)&write_size, - (char *)buf, - len, - 1, - 0, - 0); - - if (err != BZ_OK || write_size > UNIV_PAGE_SIZE-header_len) { - goto err_exit; + unsigned len = unsigned(write_size); + if (BZ_OK == BZ2_bzBuffToBuffCompress( + reinterpret_cast(out_buf + header_len), + &len, + const_cast( + reinterpret_cast(buf)), + unsigned(srv_page_size), 1, 0, 0) + && len <= write_size) { + write_size = len; + goto success; } break; } #endif /* HAVE_BZIP2 */ #ifdef HAVE_SNAPPY - case PAGE_SNAPPY_ALGORITHM: - { - snappy_status cstatus; - write_size = snappy_max_compressed_length(UNIV_PAGE_SIZE); + case PAGE_SNAPPY_ALGORITHM: { + size_t len = snappy_max_compressed_length(srv_page_size); - cstatus = snappy_compress( - (const char *)buf, - (size_t)len, - (char *)(out_buf+header_len), - (size_t*)&write_size); - - if (cstatus != SNAPPY_OK || write_size > UNIV_PAGE_SIZE-header_len) { - err = (int)cstatus; - goto err_exit; + if (SNAPPY_OK == snappy_compress( + reinterpret_cast(buf), + srv_page_size, + reinterpret_cast(out_buf) + header_len, + &len) + && len <= write_size) { + write_size = len; + goto success; } break; } #endif /* HAVE_SNAPPY */ - - case PAGE_ZLIB_ALGORITHM: - err = compress2(out_buf+header_len, (ulong*)&write_size, buf, - uLong(len), comp_level); - - if (err != Z_OK) { - goto err_exit; - } - break; - - case PAGE_UNCOMPRESSED: - *out_len = len; - return (buf); - break; - default: - ut_error; - break; } + srv_stats.pages_page_compression_error.inc(); + return 0; +success: /* Set up the page header */ memcpy(out_buf, buf, FIL_PAGE_DATA); /* Set up the checksum */ @@ -307,23 +247,12 @@ fil_compress_page( /* Verify that page can be decompressed */ { - byte *comp_page; - byte *uncomp_page; - - comp_page = static_cast(ut_malloc_nokey(UNIV_PAGE_SIZE)); - uncomp_page = static_cast(ut_malloc_nokey(UNIV_PAGE_SIZE)); - memcpy(comp_page, out_buf, UNIV_PAGE_SIZE); - - fil_decompress_page(uncomp_page, comp_page, ulong(len), NULL); - - if (buf_page_is_corrupted(false, uncomp_page, univ_page_size, - space)) { - buf_page_print(uncomp_page, univ_page_size); - ut_ad(0); - } - - ut_free(comp_page); - ut_free(uncomp_page); + page_t tmp_buf[UNIV_PAGE_SIZE_MAX]; + page_t page[UNIV_PAGE_SIZE_MAX]; + memcpy(page, out_buf, srv_page_size); + ut_ad(fil_page_decompress(tmp_buf, page)); + ut_ad(!buf_page_is_corrupted(false, page, univ_page_size, + NULL)); } #endif /* UNIV_DEBUG */ @@ -347,317 +276,143 @@ fil_compress_page( #endif } - DBUG_LOG("compress", "Succeeded for space " - << (space ? space->id : 0) << " '" - << (space ? space->name : "(import)") - << "' len " << len << " out_len " << write_size); - - srv_stats.page_compression_saved.add((len - write_size)); + srv_stats.page_compression_saved.add(srv_page_size - write_size); srv_stats.pages_page_compressed.inc(); /* If we do not persistently trim rest of page, we need to write it all */ if (!srv_use_trim) { - memset(out_buf+write_size,0,len-write_size); - write_size = len; + memset(out_buf + write_size, 0, srv_page_size - write_size); } - *out_len = write_size; - - if (allocated) { - /* TODO: reduce number of memcpy's */ - memcpy(buf, out_buf, len); - goto exit_free; - } else { - return(out_buf); - } - -err_exit: - /* If error we leave the actual page as it was */ - -#ifndef UNIV_PAGECOMPRESS_DEBUG - if (space && !space->printed_compression_failure) { - space->printed_compression_failure = true; -#endif - ib::warn() << "Compression failed for space: " - << space->id << " name: " - << space->name << " len: " - << len << " err: " << err << " write_size: " - << write_size - << " compression method: " - << fil_get_compression_alg_name(comp_method) - << "."; -#ifndef UNIV_PAGECOMPRESS_DEBUG - } -#endif - srv_stats.pages_page_compression_error.inc(); - *out_len = len; - -exit_free: - if (allocated) { - ut_free(out_buf); - } - - return (buf); - + return write_size; } -/****************************************************************//** -For page compressed pages decompress the page after actual read -operation. */ -UNIV_INTERN -void -fil_decompress_page( -/*================*/ - byte* page_buf, /*!< in: preallocated buffer or NULL */ - byte* buf, /*!< out: buffer from which to read; in aio - this must be appropriately aligned */ - ulong len, /*!< in: length of output buffer.*/ - ulint* write_size, /*!< in/out: Actual payload size of - the compressed data. */ - bool return_error) /*!< in: true if only an error should - be produced when decompression fails. - By default this parameter is false. */ +/** Decompress a page that may be subject to page_compressed compression. +@param[in,out] tmp_buf temporary buffer (of innodb_page_size) +@param[in,out] buf possibly compressed page buffer +@return size of the compressed data +@retval 0 if decompression failed +@retval srv_page_size if the page was not compressed */ +ulint fil_page_decompress(byte* tmp_buf, byte* buf) { - int err = 0; - ulint actual_size = 0; - ib_uint64_t compression_alg = 0; - byte *in_buf; - ulint ptype; + const unsigned ptype = mach_read_from_2(buf+FIL_PAGE_TYPE); ulint header_len; - - ut_ad(buf); - ut_ad(len); - - ptype = mach_read_from_2(buf+FIL_PAGE_TYPE); - + uint64_t compression_alg; switch (ptype) { case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED: header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE + FIL_PAGE_COMPRESSION_METHOD_SIZE; + compression_alg = mach_read_from_2( + FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE + buf); break; case FIL_PAGE_PAGE_COMPRESSED: header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE; + compression_alg = mach_read_from_8( + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION + buf); break; default: - /* The page is not in our format. */ - return; + return srv_page_size; } - // If no buffer was given, we need to allocate temporal buffer - if (page_buf == NULL) { - in_buf = static_cast(ut_malloc_nokey(UNIV_PAGE_SIZE)); - memset(in_buf, 0, UNIV_PAGE_SIZE); - } else { - in_buf = page_buf; + if (mach_read_from_4(buf + FIL_PAGE_SPACE_OR_CHKSUM) + != BUF_NO_CHECKSUM_MAGIC) { + return 0; } - /* Before actual decompress, make sure that page type is correct */ + ulint actual_size = mach_read_from_2(buf + FIL_PAGE_DATA); - if (mach_read_from_4(buf+FIL_PAGE_SPACE_OR_CHKSUM) - != BUF_NO_CHECKSUM_MAGIC - || (ptype != FIL_PAGE_PAGE_COMPRESSED - && ptype != FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED)) { - ib::error() << "Corruption: We try to uncompress corrupted " - "page CRC " - << mach_read_from_4(buf+FIL_PAGE_SPACE_OR_CHKSUM) - << " type " << ptype << " len " << len << "."; - - if (return_error) { - goto error_return; - } - ut_error; - } - - /* Get compression algorithm */ - if (ptype == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) { - compression_alg = static_cast(mach_read_from_2(buf+FIL_PAGE_DATA+FIL_PAGE_COMPRESSED_SIZE)); - } else { - compression_alg = mach_read_from_8(buf+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); - } - - /* Get the actual size of compressed page */ - actual_size = mach_read_from_2(buf+FIL_PAGE_DATA); /* Check if payload size is corrupted */ - if (actual_size == 0 || actual_size > UNIV_PAGE_SIZE) { - ib::error() << "Corruption: We try to uncompress corrupted page" - << " actual size: " << actual_size - << " compression method: " - << fil_get_compression_alg_name(compression_alg) - << "."; - if (return_error) { - goto error_return; - } - ut_error; + if (actual_size == 0 || actual_size > srv_page_size - header_len) { + return 0; } - /* Store actual payload size of the compressed data. This pointer - points to buffer pool. */ - if (write_size) { - *write_size = actual_size; - } - - DBUG_LOG("compress", "Preparing for decompress for len " - << actual_size << "."); - - switch(compression_alg) { + switch (compression_alg) { + default: + ib::error() << "Unknown compression algorithm " + << compression_alg; + return 0; case PAGE_ZLIB_ALGORITHM: - err= uncompress(in_buf, &len, buf+header_len, (unsigned long)actual_size); - - /* If uncompress fails it means that page is corrupted */ - if (err != Z_OK) { - goto err_exit; - if (return_error) { - goto error_return; + { + uLong len = srv_page_size; + if (Z_OK != uncompress(tmp_buf, &len, + buf + header_len, + uLong(actual_size)) + && len != srv_page_size) { + return 0; } } break; - #ifdef HAVE_LZ4 case PAGE_LZ4_ALGORITHM: - err = LZ4_decompress_fast((const char *)buf+header_len, (char *)in_buf, len); - - if (err != (int)actual_size) { - goto err_exit; - if (return_error) { - goto error_return; - } + if (LZ4_decompress_safe(reinterpret_cast(buf) + + header_len, + reinterpret_cast(tmp_buf), + actual_size, srv_page_size) + == int(srv_page_size)) { + break; } - break; + return 0; #endif /* HAVE_LZ4 */ #ifdef HAVE_LZO case PAGE_LZO_ALGORITHM: { - ulint olen = 0; - lzo_uint olen_lzo = olen; - err = lzo1x_decompress((const unsigned char *)buf+header_len, - actual_size,(unsigned char *)in_buf, &olen_lzo, NULL); - - olen = olen_lzo; - - if (err != LZO_E_OK || (olen == 0 || olen > UNIV_PAGE_SIZE)) { - len = olen; - goto err_exit; - if (return_error) { - goto error_return; - } + lzo_uint len_lzo = srv_page_size; + if (LZO_E_OK == lzo1x_decompress_safe( + buf + header_len, + actual_size, tmp_buf, &len_lzo, NULL) + && len_lzo == srv_page_size) { + break; } - break; + return 0; } #endif /* HAVE_LZO */ #ifdef HAVE_LZMA case PAGE_LZMA_ALGORITHM: { - - lzma_ret ret; size_t src_pos = 0; size_t dst_pos = 0; uint64_t memlimit = UINT64_MAX; - ret = lzma_stream_buffer_decode( - &memlimit, - 0, - NULL, - buf+header_len, - &src_pos, - actual_size, - in_buf, - &dst_pos, - len); - - - if (ret != LZMA_OK || (dst_pos == 0 || dst_pos > UNIV_PAGE_SIZE)) { - len = dst_pos; - goto err_exit; - if (return_error) { - goto error_return; - } + if (LZMA_OK == lzma_stream_buffer_decode( + &memlimit, 0, NULL, buf + header_len, + &src_pos, actual_size, tmp_buf, &dst_pos, + srv_page_size) + && dst_pos == srv_page_size) { + break; } - - break; + return 0; } #endif /* HAVE_LZMA */ #ifdef HAVE_BZIP2 case PAGE_BZIP2_ALGORITHM: { - unsigned int dst_pos = UNIV_PAGE_SIZE; - - err = BZ2_bzBuffToBuffDecompress( - (char *)in_buf, - &dst_pos, - (char *)(buf+header_len), - actual_size, - 1, - 0); - - if (err != BZ_OK || (dst_pos == 0 || dst_pos > UNIV_PAGE_SIZE)) { - len = dst_pos; - goto err_exit; - if (return_error) { - goto error_return; - } + unsigned int dst_pos = srv_page_size; + if (BZ_OK == BZ2_bzBuffToBuffDecompress( + reinterpret_cast(tmp_buf), + &dst_pos, + reinterpret_cast(buf) + header_len, + actual_size, 1, 0) + && dst_pos == srv_page_size) { + break; } - break; + return 0; } #endif /* HAVE_BZIP2 */ #ifdef HAVE_SNAPPY - case PAGE_SNAPPY_ALGORITHM: - { - snappy_status cstatus; - ulint olen = UNIV_PAGE_SIZE; + case PAGE_SNAPPY_ALGORITHM: { + size_t olen = srv_page_size; - cstatus = snappy_uncompress( - (const char *)(buf+header_len), - (size_t)actual_size, - (char *)in_buf, - (size_t*)&olen); - - if (cstatus != SNAPPY_OK || (olen == 0 || olen > UNIV_PAGE_SIZE)) { - err = (int)cstatus; - len = olen; - goto err_exit; - if (return_error) { - goto error_return; - } + if (SNAPPY_OK == snappy_uncompress( + reinterpret_cast(buf) + header_len, + actual_size, + reinterpret_cast(tmp_buf), &olen) + && olen == srv_page_size) { + break; } - - break; + return 0; } #endif /* HAVE_SNAPPY */ - default: - goto err_exit; - if (return_error) { - goto error_return; - } - break; } srv_stats.pages_page_decompressed.inc(); - - /* Copy the uncompressed page to the buffer pool, not - really any other options. */ - memcpy(buf, in_buf, len); - -error_return: - if (page_buf != in_buf) { - ut_free(in_buf); - } - - return; - -err_exit: - /* Note that as we have found the page is corrupted, so - all this could be incorrect. */ - ulint space_id = mach_read_from_4(buf+FIL_PAGE_SPACE_ID); - fil_space_t* space = fil_space_acquire_for_io(space_id); - - ib::error() << "Corruption: Page is marked as compressed" - << " space: " << space_id << " name: " - << (space ? space->name : "NULL") - << " but uncompress failed with error: " << err - << " size: " << actual_size - << " len: " << len - << " compression method: " - << fil_get_compression_alg_name(compression_alg) << "."; - - buf_page_print(buf, univ_page_size); - fil_space_release_for_io(space); - ut_ad(0); + memcpy(buf, tmp_buf, srv_page_size); + return actual_size; } diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc index f20354e7588..fe0b4556fa2 100644 --- a/storage/innobase/ibuf/ibuf0ibuf.cc +++ b/storage/innobase/ibuf/ibuf0ibuf.cc @@ -5040,6 +5040,11 @@ ibuf_check_bitmap_on_import( continue; } + if (!bitmap_page) { + mutex_exit(&ibuf_mutex); + return DB_CORRUPTION; + } + for (i = FSP_IBUF_BITMAP_OFFSET + 1; i < page_size.physical(); i++) { diff --git a/storage/innobase/include/buf0buf.h b/storage/innobase/include/buf0buf.h index 1001f2ca807..a79b39235f3 100644 --- a/storage/innobase/include/buf0buf.h +++ b/storage/innobase/include/buf0buf.h @@ -41,6 +41,7 @@ Created 11/5/1995 Heikki Tuuri #include "os0proc.h" #include "log0log.h" #include "srv0srv.h" +#include "my_atomic.h" #include // Forward declaration @@ -1516,8 +1517,10 @@ NOTE! The definition appears here only for other modules of this directory (buf) to see it. Do not use from outside! */ typedef struct { - bool reserved; /*!< true if this slot is reserved +private: + int32 reserved; /*!< true if this slot is reserved */ +public: byte* crypt_buf; /*!< for encryption the data needs to be copied to a separate buffer before it's encrypted&written. this as a page can be @@ -1528,6 +1531,21 @@ typedef struct { byte* out_buf; /*!< resulting buffer after encryption/compression. This is a pointer and not allocated. */ + + /** Release the slot */ + void release() + { + my_atomic_store32_explicit(&reserved, false, + MY_MEMORY_ORDER_RELAXED); + } + + /** Acquire the slot + @return whether the slot was acquired */ + bool acquire() + { + return !my_atomic_fas32_explicit(&reserved, true, + MY_MEMORY_ORDER_RELAXED); + } } buf_tmp_buffer_t; /** The common buffer control block structure diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h index 9797de6df0b..8aa8a746ce1 100644 --- a/storage/innobase/include/fil0fil.h +++ b/storage/innobase/include/fil0fil.h @@ -175,9 +175,6 @@ struct fil_space_t { /** MariaDB encryption data */ fil_space_crypt_t* crypt_data; - /** True if we have already printed compression failure */ - bool printed_compression_failure; - /** True if the device this filespace is on supports atomic writes */ bool atomic_write_supported; diff --git a/storage/innobase/include/fil0pagecompress.h b/storage/innobase/include/fil0pagecompress.h index be10f99d0f0..1046d720102 100644 --- a/storage/innobase/include/fil0pagecompress.h +++ b/storage/innobase/include/fil0pagecompress.h @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (C) 2013, 2017 MariaDB Corporation. All Rights Reserved. +Copyright (C) 2013, 2018 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 @@ -30,40 +30,24 @@ atomic writes information to table space. Created 11/12/2013 Jan Lindström jan.lindstrom@skysql.com ***********************************************************************/ -/****************************************************************//** -For page compressed pages compress the page before actual write -operation. -@return compressed page to be written*/ -UNIV_INTERN -byte* -fil_compress_page( -/*==============*/ - fil_space_t* space, /*!< in,out: tablespace (NULL during IMPORT) */ - byte* buf, /*!< in: buffer from which to write; in aio - this must be appropriately aligned */ - byte* out_buf, /*!< out: compressed buffer */ - ulint len, /*!< in: length of input buffer.*/ - ulint level, /* in: compression level */ - ulint block_size, /*!< in: block size */ - bool encrypted, /*!< in: is page also encrypted */ - ulint* out_len); /*!< out: actual length of compressed - page */ +/** Compress a page_compressed page before writing to a data file. +@param[in] buf page to be compressed +@param[out] out_buf compressed page +@param[in] level compression level +@param[in] block_size file system block size +@param[in] encrypted whether the page will be subsequently encrypted +@return actual length of compressed page +@retval 0 if the page was not compressed */ +ulint fil_page_compress(const byte* buf, byte* out_buf, ulint level, + ulint block_size, bool encrypted) + MY_ATTRIBUTE((nonnull, warn_unused_result)); -/****************************************************************//** -For page compressed pages decompress the page after actual read -operation. */ -UNIV_INTERN -void -fil_decompress_page( -/*================*/ - byte* page_buf, /*!< in: preallocated buffer or NULL */ - byte* buf, /*!< out: buffer from which to read; in aio - this must be appropriately aligned */ - ulong len, /*!< in: length of output buffer.*/ - ulint* write_size, /*!< in/out: Actual payload size of - the compressed data. */ - bool return_error=false); - /*!< in: true if only an error should - be produced when decompression fails. - By default this parameter is false. */ +/** Decompress a page that may be subject to page_compressed compression. +@param[in,out] tmp_buf temporary buffer (of innodb_page_size) +@param[in,out] buf compressed page buffer +@return size of the compressed data +@retval 0 if decompression failed +@retval srv_page_size if the page was not compressed */ +ulint fil_page_decompress(byte* tmp_buf, byte* buf) + MY_ATTRIBUTE((nonnull, warn_unused_result)); #endif diff --git a/storage/innobase/include/fsp0pagecompress.ic b/storage/innobase/include/fsp0pagecompress.ic index d1f2ea45fbd..5285a7c238f 100644 --- a/storage/innobase/include/fsp0pagecompress.ic +++ b/storage/innobase/include/fsp0pagecompress.ic @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (C) 2013, 2017, MariaDB Corporation. All Rights Reserved. +Copyright (C) 2013, 2018, 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 @@ -38,18 +38,6 @@ fsp_flags_get_page_compression_level( } -/*******************************************************************//** -Find out wheather the page is index page or not -@return true if page type index page, false if not */ -UNIV_INLINE -bool -fil_page_is_index_page( -/*===================*/ - byte* buf) /*!< in: page */ -{ - return(mach_read_from_2(buf+FIL_PAGE_TYPE) == FIL_PAGE_INDEX); -} - /*******************************************************************//** Find out wheather the page is page compressed @return true if page is page compressed, false if not */ @@ -73,59 +61,3 @@ fil_page_is_compressed_encrypted( { return(mach_read_from_2(buf+FIL_PAGE_TYPE) == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED); } - -/****************************************************************//** -Get the name of the compression algorithm used for page -compression. -@return compression algorithm name or "UNKNOWN" if not known*/ -UNIV_INLINE -const char* -fil_get_compression_alg_name( -/*=========================*/ - ib_uint64_t comp_alg) /*! @@ -406,12 +412,9 @@ public: updated then its state must be set to BUF_PAGE_NOT_USED. For compressed tables the page descriptor memory will be at offset: block->frame + UNIV_PAGE_SIZE; - @param offset - physical offset within the file - @param block - block read from file, note it is not from the buffer pool + @param block block read from file, note it is not from the buffer pool @retval DB_SUCCESS or error code. */ - virtual dberr_t operator()( - os_offset_t offset, - buf_block_t* block) UNIV_NOTHROW = 0; + virtual dberr_t operator()(buf_block_t* block) UNIV_NOTHROW = 0; /** @return the space id of the tablespace */ @@ -635,12 +638,9 @@ struct FetchIndexRootPages : public AbstractCallback { } /** Called for each block as it is read from the file. - @param offset physical offset in the file @param block block to convert, it is not from the buffer pool. @retval DB_SUCCESS or error code. */ - virtual dberr_t operator() ( - os_offset_t offset, - buf_block_t* block) UNIV_NOTHROW; + dberr_t operator()(buf_block_t* block) UNIV_NOTHROW; /** Update the import configuration that will be used to import the tablespace. */ @@ -657,13 +657,9 @@ struct FetchIndexRootPages : public AbstractCallback { determine the exact row format. We can't get that from the tablespace header flags alone. -@param offset physical offset in the file @param block block to convert, it is not from the buffer pool. @retval DB_SUCCESS or error code. */ -dberr_t -FetchIndexRootPages::operator() ( - os_offset_t offset, - buf_block_t* block) UNIV_NOTHROW +dberr_t FetchIndexRootPages::operator()(buf_block_t* block) UNIV_NOTHROW { if (is_interrupted()) return DB_INTERRUPTED; @@ -671,15 +667,7 @@ FetchIndexRootPages::operator() ( ulint page_type = fil_page_get_type(page); - if (block->page.id.page_no() * m_page_size.physical() != offset) { - - ib::error() << "Page offset doesn't match file offset:" - " page offset: " << block->page.id.page_no() - << ", file offset: " - << (offset / m_page_size.physical()); - - return DB_CORRUPTION; - } else if (page_type == FIL_PAGE_TYPE_XDES) { + if (page_type == FIL_PAGE_TYPE_XDES) { return set_current_xdes(block->page.id.page_no(), page); } else if (fil_page_index_page_check(page) && !is_free(block->page.id.page_no()) @@ -825,12 +813,9 @@ public: } /** Called for each block as it is read from the file. - @param offset physical offset in the file @param block block to convert, it is not from the buffer pool. @retval DB_SUCCESS or error code. */ - virtual dberr_t operator() ( - os_offset_t offset, - buf_block_t* block) UNIV_NOTHROW; + dberr_t operator()(buf_block_t* block) UNIV_NOTHROW; private: /** Update the page, set the space id, max trx id and index id. @param block block read from file @@ -1954,8 +1939,7 @@ PageConverter::update_page( updated then its state must be set to BUF_PAGE_NOT_USED. @param block block read from file, note it is not from the buffer pool @retval DB_SUCCESS or error code. */ -dberr_t -PageConverter::operator() (os_offset_t, buf_block_t* block) UNIV_NOTHROW +dberr_t PageConverter::operator()(buf_block_t* block) UNIV_NOTHROW { /* If we already had an old page with matching number in the buffer pool, evict it now, because @@ -3299,15 +3283,29 @@ fil_iterate( const ulint size = callback.get_page_size().physical(); ulint n_bytes = iter.n_io_buffers * size; + const ulint buf_size = srv_page_size +#ifdef HAVE_LZO + + LZO1X_1_15_MEM_COMPRESS +#elif defined HAVE_SNAPPY + + snappy_max_compressed_length(srv_page_size) +#endif + ; + byte* page_compress_buf = static_cast(malloc(buf_size)); ut_ad(!srv_read_only_mode); + if (!page_compress_buf) { + return DB_OUT_OF_MEMORY; + } + /* TODO: For ROW_FORMAT=COMPRESSED tables we do a lot of useless copying for non-index pages. Unfortunately, it is required by buf_zip_decompress() */ + dberr_t err = DB_SUCCESS; for (offset = iter.start; offset < iter.end; offset += n_bytes) { if (callback.is_interrupted()) { - return DB_INTERRUPTED; + err = DB_INTERRUPTED; + goto func_exit; } byte* io_buffer = iter.io_buffer; @@ -3337,11 +3335,12 @@ fil_iterate( IORequest read_request(IORequest::READ); read_request.disable_partial_io_warnings(); - dberr_t err = os_file_read_no_error_handling( + err = os_file_read_no_error_handling( read_request, iter.file, readptr, offset, n_bytes, 0); if (err != DB_SUCCESS) { ib::error() << iter.filepath << ": os_file_read() failed"; + goto func_exit; } bool updated = false; @@ -3352,18 +3351,9 @@ fil_iterate( for (ulint i = 0; i < n_pages_read; block->page.id.set_page_no(block->page.id.page_no() + 1), ++i, page_off += size, block->frame += size) { - bool decrypted = false; - err = DB_SUCCESS; byte* src = readptr + i * size; - byte* dst = io_buffer + i * size; - bool frame_changed = false; - ulint page_type = mach_read_from_2(src+FIL_PAGE_TYPE); - const bool page_compressed - = page_type - == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED - || page_type == FIL_PAGE_PAGE_COMPRESSED; const ulint page_no = page_get_page_no(src); - if (!page_no && page_off) { + if (!page_no && block->page.id.page_no()) { const ulint* b = reinterpret_cast (src); const ulint* const e = b + size / sizeof *b; @@ -3378,54 +3368,84 @@ fil_iterate( continue; } - if (page_no != page_off / size) { + if (page_no != block->page.id.page_no()) { +page_corrupted: + ib::warn() << callback.filename() + << ": Page " << (offset / size) + << " at offset " << offset + << " looks corrupted."; + err = DB_CORRUPTION; + goto func_exit; + } + + const bool page_compressed + = fil_page_is_compressed_encrypted(src) + || fil_page_is_compressed(src); + + if (page_compressed && block->page.zip.data) { goto page_corrupted; } - if (encrypted) { + bool decrypted = false; + byte* dst = io_buffer + i * size; + bool frame_changed = false; + + if (!encrypted) { + } else if (!mach_read_from_4( + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION + + src)) { +not_encrypted: + if (!page_compressed + && !block->page.zip.data) { + block->frame = src; + frame_changed = true; + } else { + ut_ad(dst != src); + memcpy(dst, src, size); + } + } else { + if (!fil_space_verify_crypt_checksum( + src, callback.get_page_size(), + block->page.id.space(), + block->page.id.page_no())) { + goto page_corrupted; + } + decrypted = fil_space_decrypt( iter.crypt_data, dst, callback.get_page_size(), src, &err); if (err != DB_SUCCESS) { - return err; + goto func_exit; } - if (decrypted) { - updated = true; - } else { - if (!page_compressed - && !block->page.zip.data) { - block->frame = src; - frame_changed = true; - } else { - ut_ad(dst != src); - memcpy(dst, src, size); - } + if (!decrypted) { + goto not_encrypted; } + + updated = true; } /* If the original page is page_compressed, we need to decompress it before adjusting further. */ if (page_compressed) { - fil_decompress_page(NULL, dst, ulong(size), - NULL); + ulint compress_length = fil_page_decompress( + page_compress_buf, dst); + ut_ad(compress_length != srv_page_size); + if (compress_length == 0) { + goto page_corrupted; + } updated = true; } else if (buf_page_is_corrupted( false, encrypted && !frame_changed ? dst : src, callback.get_page_size(), NULL)) { -page_corrupted: - ib::warn() << callback.filename() - << ": Page " << (offset / size) - << " at offset " << offset - << " looks corrupted."; - return DB_CORRUPTION; + goto page_corrupted; } - if ((err = callback(page_off, block)) != DB_SUCCESS) { - return err; + if ((err = callback(block)) != DB_SUCCESS) { + goto func_exit; } else if (!updated) { updated = buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE; @@ -3475,20 +3495,18 @@ page_corrupted: src = io_buffer + (i * size); if (page_compressed) { - ulint len = 0; - - fil_compress_page( - NULL, - src, - NULL, - size, - 0,/* FIXME: compression level */ - 512,/* FIXME: use proper block size */ - encrypted, - &len); - ut_ad(len <= size); - memset(src + len, 0, size - len); updated = true; + if (ulint len = fil_page_compress( + src, + page_compress_buf, + 0,/* FIXME: compression level */ + 512,/* FIXME: proper block size */ + encrypted)) { + /* FIXME: remove memcpy() */ + memcpy(src, page_compress_buf, len); + memset(src + len, 0, + srv_page_size - len); + } } /* Encrypt the page if encryption was used. */ @@ -3520,12 +3538,14 @@ page_corrupted: writeptr, offset, n_bytes); if (err != DB_SUCCESS) { - return err; + goto func_exit; } } } - return DB_SUCCESS; +func_exit: + free(page_compress_buf); + return err; } /********************************************************************//** From a79b033b359b882f2fe88051781c6e850bd71780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 Jun 2018 14:23:20 +0300 Subject: [PATCH 13/15] MDEV-16457 mariabackup 10.2+ should default to innodb_checksum_algorithm=crc32 Since MariaDB Server 10.2.2 (and MySQL 5.7), the default value of innodb_checksum_algorithm is crc32 (CRC-32C), not the inefficient "innodb" checksum. Change Mariabackup to use the same default, so that checksum validation (when using the default algorithm on the server) will take less time during mariabackup --backup. Also, mariabackup --prepare should be a little faster, and the server should read backups faster, because the page checksums would only be validated against CRC-32C. --- extra/mariabackup/xtrabackup.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index 4d88778f020..bd5e28a4f5f 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -1193,7 +1193,7 @@ struct my_option xb_server_options[] = "The algorithm InnoDB uses for page checksumming. [CRC32, STRICT_CRC32, " "INNODB, STRICT_INNODB, NONE, STRICT_NONE]", &srv_checksum_algorithm, &srv_checksum_algorithm, &innodb_checksum_algorithm_typelib, GET_ENUM, - REQUIRED_ARG, SRV_CHECKSUM_ALGORITHM_INNODB, 0, 0, 0, 0, 0}, + REQUIRED_ARG, SRV_CHECKSUM_ALGORITHM_CRC32, 0, 0, 0, 0, 0}, {"innodb_undo_directory", OPT_INNODB_UNDO_DIRECTORY, "Directory where undo tablespace files live, this path can be absolute.", From ec4fdd574964d3611e904fa5f1751d88d8e8286f Mon Sep 17 00:00:00 2001 From: Galina Shalygina Date: Wed, 13 Jun 2018 16:32:25 +0200 Subject: [PATCH 14/15] MDEV-16386: Wrong result when pushdown into the HAVING clause of the materialized derived table/view that uses aliases is done The problem appears when a column alias inside the materialized derived table/view t1 definition coincides with the column name used in the GROUP BY clause of t1. If the condition that can be pushed into t1 uses that ambiguous column name this column is determined as a column that is used in the GROUP BY clause instead of the alias used in the projection list of t1. That causes wrong result. To prevent it resolve_ref_in_select_and_group() was changed. --- mysql-test/r/derived_cond_pushdown.result | 196 ++++++++++++++++++++++ mysql-test/t/derived_cond_pushdown.test | 56 +++++++ sql/item.cc | 11 +- sql/sql_lex.cc | 1 + sql/sql_lex.h | 5 + sql/sql_select.cc | 2 + 6 files changed, 267 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/derived_cond_pushdown.result b/mysql-test/r/derived_cond_pushdown.result index 3fbc81019cc..83e70dd634c 100644 --- a/mysql-test/r/derived_cond_pushdown.result +++ b/mysql-test/r/derived_cond_pushdown.result @@ -9473,3 +9473,199 @@ WHERE (a>0 AND a<2 OR a IN (2,3)) AND a 2 DROP TABLE t1; +# +# MDEV-16386: pushing condition into the HAVING clause when ambiguous +# fields warning appears +# +CREATE TABLE t1 (a INT, b INT); +INSERT INTO t1 VALUES (1,2),(2,3),(3,4); +SELECT * FROM +( +SELECT t1.b AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a=2); +a +2 +EXPLAIN FORMAT=JSON SELECT * FROM +( +SELECT t1.b AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a=2); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "", + "access_type": "ALL", + "rows": 3, + "filtered": 100, + "attached_condition": "dt.a = 2", + "materialized": { + "query_block": { + "select_id": 2, + "having_condition": "a = 2", + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 3, + "filtered": 100 + } + } + } + } + } + } + } +} +SELECT * FROM +( +SELECT t1.b AS a +FROM t1 +GROUP BY t1.a +HAVING (t1.a<3) +) dt +WHERE (dt.a>1); +a +2 +3 +EXPLAIN FORMAT=JSON SELECT * FROM +( +SELECT t1.b AS a +FROM t1 +GROUP BY t1.a +HAVING (t1.a<3) +) dt +WHERE (dt.a>1); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "", + "access_type": "ALL", + "rows": 3, + "filtered": 100, + "attached_condition": "dt.a > 1", + "materialized": { + "query_block": { + "select_id": 2, + "having_condition": "t1.a < 3 and a > 1", + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 3, + "filtered": 100 + } + } + } + } + } + } + } +} +SELECT * FROM +( +SELECT 'ab' AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a='ab'); +a +ab +ab +ab +EXPLAIN FORMAT=JSON SELECT * FROM +( +SELECT 'ab' AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a='ab'); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "", + "access_type": "ALL", + "rows": 3, + "filtered": 100, + "attached_condition": "dt.a = 'ab'", + "materialized": { + "query_block": { + "select_id": 2, + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 3, + "filtered": 100 + } + } + } + } + } + } + } +} +SELECT * FROM +( +SELECT 1 AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a=1); +a +1 +1 +1 +EXPLAIN FORMAT=JSON SELECT * FROM +( +SELECT 1 AS a +FROM t1 +GROUP BY t1.a +) dt +WHERE (dt.a=1); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "", + "access_type": "ALL", + "rows": 3, + "filtered": 100, + "attached_condition": "dt.a = 1", + "materialized": { + "query_block": { + "select_id": 2, + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 3, + "filtered": 100 + } + } + } + } + } + } + } +} +DROP TABLE t1; diff --git a/mysql-test/t/derived_cond_pushdown.test b/mysql-test/t/derived_cond_pushdown.test index d3832ce1ec3..718140d3a77 100644 --- a/mysql-test/t/derived_cond_pushdown.test +++ b/mysql-test/t/derived_cond_pushdown.test @@ -1745,3 +1745,59 @@ WHERE (a>0 AND a<2 OR a IN (2,3)) AND (a=2 OR 0); DROP TABLE t1; + +--echo # +--echo # MDEV-16386: pushing condition into the HAVING clause when ambiguous +--echo # fields warning appears +--echo # + +CREATE TABLE t1 (a INT, b INT); + +INSERT INTO t1 VALUES (1,2),(2,3),(3,4); + +LET $query= +SELECT * FROM +( + SELECT t1.b AS a + FROM t1 + GROUP BY t1.a +) dt +WHERE (dt.a=2); +EVAL $query; +EVAL EXPLAIN FORMAT=JSON $query; + +LET $query= +SELECT * FROM +( + SELECT t1.b AS a + FROM t1 + GROUP BY t1.a + HAVING (t1.a<3) +) dt +WHERE (dt.a>1); +EVAL $query; +EVAL EXPLAIN FORMAT=JSON $query; + +LET $query= +SELECT * FROM +( + SELECT 'ab' AS a + FROM t1 + GROUP BY t1.a +) dt +WHERE (dt.a='ab'); +EVAL $query; +EVAL EXPLAIN FORMAT=JSON $query; + +LET $query= +SELECT * FROM +( + SELECT 1 AS a + FROM t1 + GROUP BY t1.a +) dt +WHERE (dt.a=1); +EVAL $query; +EVAL EXPLAIN FORMAT=JSON $query; + +DROP TABLE t1; diff --git a/sql/item.cc b/sql/item.cc index f9200ccf56d..4bc6fe7e5bc 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -4984,9 +4984,11 @@ static Item** find_field_in_group_list(Item *find_item, ORDER *group_list) in the SELECT clause of Q. - Search for a column named col_ref_i [in table T_j] in the GROUP BY clause of Q. - - If found different columns with the same name in GROUP BY and SELECT - - issue a warning and return the GROUP BY column, - - otherwise + - If found different columns with the same name in GROUP BY and SELECT: + - if the condition that uses this column name is pushed down into + the HAVING clause return the SELECT column + - else issue a warning and return the GROUP BY column. + - Otherwise - if the MODE_ONLY_FULL_GROUP_BY mode is enabled return error - else return the found SELECT column. @@ -5025,7 +5027,8 @@ resolve_ref_in_select_and_group(THD *thd, Item_ident *ref, SELECT_LEX *select) /* Check if the fields found in SELECT and GROUP BY are the same field. */ if (group_by_ref && (select_ref != not_found_item) && - !((*group_by_ref)->eq(*select_ref, 0))) + !((*group_by_ref)->eq(*select_ref, 0)) && + (!select->having_fix_field_for_pushed_cond)) { ambiguous_fields= TRUE; push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 750fb75bfb9..d3ddd9e208c 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2107,6 +2107,7 @@ void st_select_lex::init_query() cond_pushed_into_where= cond_pushed_into_having= 0; olap= UNSPECIFIED_OLAP_TYPE; having_fix_field= 0; + having_fix_field_for_pushed_cond= 0; context.select_lex= this; context.init(); /* diff --git a/sql/sql_lex.h b/sql/sql_lex.h index ed5e423fd5a..a1f6b202ae6 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -887,6 +887,11 @@ public: bool braces; /* SELECT ... UNION (SELECT ... ) <- this braces */ /* TRUE when having fix field called in processing of this SELECT */ bool having_fix_field; + /* + TRUE when fix field is called for a new condition pushed into the + HAVING clause of this SELECT + */ + bool having_fix_field_for_pushed_cond; /* List of references to fields referenced from inner selects */ List inner_refs_list; /* Number of Item_sum-derived objects in this SELECT */ diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ee5ba4ade54..4ebd5ef4954 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1350,9 +1350,11 @@ JOIN::optimize_inner() if (having) { select_lex->having_fix_field= 1; + select_lex->having_fix_field_for_pushed_cond= 1; if (having->fix_fields(thd, &having)) DBUG_RETURN(1); select_lex->having_fix_field= 0; + select_lex->having_fix_field_for_pushed_cond= 0; } } From c55de8d40bba29503773a6a56d6f13f19ca7e339 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 15 Jun 2018 10:11:51 +0400 Subject: [PATCH 15/15] MDEV-9334 ALTER from DECIMAL to BIGINT UNSIGNED returns a wrong result When altering from DECIMAL to *INT UNIGNED or to BIT, go through val_decimal(), to avoid truncation to the biggest possible signed integer (0x7FFFFFFFFFFFFFFF / 9223372036854775807). --- mysql-test/r/type_bit.result | 19 +++++++++++++++++++ mysql-test/r/type_int.result | 13 +++++++++++++ mysql-test/t/type_bit.test | 25 +++++++++++++++++++++++++ mysql-test/t/type_int.test | 12 ++++++++++++ sql/field.h | 4 ++++ 5 files changed, 73 insertions(+) diff --git a/mysql-test/r/type_bit.result b/mysql-test/r/type_bit.result index 30cd94c9277..eeedc501dc4 100644 --- a/mysql-test/r/type_bit.result +++ b/mysql-test/r/type_bit.result @@ -830,3 +830,22 @@ def COALESCE(val, 1) 246 2 1 Y 32896 0 63 COALESCE(val, 1) 0 DROP TABLE t1; +# +# End of 10.1 tests +# +# +# Start of 10.2 tests +# +# +# MDEV-9334 ALTER from DECIMAL to BIGINT UNSIGNED returns a wrong result +# +CREATE TABLE t1 (a DECIMAL(30,0)); +INSERT INTO t1 VALUES (CAST(0xFFFFFFFFFFFFFFFF AS UNSIGNED)); +ALTER TABLE t1 MODIFY a BIT(64); +SELECT a+0 FROM t1; +a+0 +18446744073709551615 +DROP TABLE IF EXISTS t1; +# +# End of 10.2 tests +# diff --git a/mysql-test/r/type_int.result b/mysql-test/r/type_int.result index 39e2e91ecc7..77e6ee14c25 100644 --- a/mysql-test/r/type_int.result +++ b/mysql-test/r/type_int.result @@ -91,5 +91,18 @@ a 10 DROP TABLE t1; # +# MDEV-9334 ALTER from DECIMAL to BIGINT UNSIGNED returns a wrong result +# +CREATE TABLE t1 (a DECIMAL(30,0)); +INSERT INTO t1 VALUES (CAST(0xFFFFFFFFFFFFFFFF AS UNSIGNED)); +SELECT * FROM t1; +a +18446744073709551615 +ALTER TABLE t1 MODIFY a BIGINT UNSIGNED; +SELECT * FROM t1; +a +18446744073709551615 +DROP TABLE t1; +# # End of 10.2 tests # diff --git a/mysql-test/t/type_bit.test b/mysql-test/t/type_bit.test index bb282fc15e5..04db1511833 100644 --- a/mysql-test/t/type_bit.test +++ b/mysql-test/t/type_bit.test @@ -458,3 +458,28 @@ DROP TABLE t2; SELECT COALESCE(val, 1) FROM t1; --disable_metadata DROP TABLE t1; + +--echo # +--echo # End of 10.1 tests +--echo # + + +--echo # +--echo # Start of 10.2 tests +--echo # + +--echo # +--echo # MDEV-9334 ALTER from DECIMAL to BIGINT UNSIGNED returns a wrong result +--echo # + +CREATE TABLE t1 (a DECIMAL(30,0)); +INSERT INTO t1 VALUES (CAST(0xFFFFFFFFFFFFFFFF AS UNSIGNED)); +ALTER TABLE t1 MODIFY a BIT(64); +SELECT a+0 FROM t1; +DROP TABLE IF EXISTS t1; + + +--echo # +--echo # End of 10.2 tests +--echo # + diff --git a/mysql-test/t/type_int.test b/mysql-test/t/type_int.test index 271b4d5862a..76ed7eee824 100644 --- a/mysql-test/t/type_int.test +++ b/mysql-test/t/type_int.test @@ -73,6 +73,18 @@ ALTER TABLE t1 MODIFY a INT; SELECT * FROM t1; DROP TABLE t1; +--echo # +--echo # MDEV-9334 ALTER from DECIMAL to BIGINT UNSIGNED returns a wrong result +--echo # + +CREATE TABLE t1 (a DECIMAL(30,0)); +INSERT INTO t1 VALUES (CAST(0xFFFFFFFFFFFFFFFF AS UNSIGNED)); +SELECT * FROM t1; +ALTER TABLE t1 MODIFY a BIGINT UNSIGNED; +SELECT * FROM t1; +DROP TABLE t1; + + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/field.h b/sql/field.h index ca85a7c276f..e554f92031c 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1636,6 +1636,8 @@ public: bool eq_def(const Field *field) const; Copy_func *get_copy_func(const Field *from) const { + if (unsigned_flag && from->cmp_type() == DECIMAL_RESULT) + return do_field_decimal; return do_field_int; } int save_in_field(Field *to) @@ -3661,6 +3663,8 @@ public: } Copy_func *get_copy_func(const Field *from) const { + if (from->cmp_type() == DECIMAL_RESULT) + return do_field_decimal; return do_field_int; } int save_in_field(Field *to) { return to->store(val_int(), true); }