From c612a1e77c36abe08576d67de30b7e45365cad30 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Sat, 30 Jun 2018 11:02:49 +0100 Subject: MDEV-16596 : Windows - redo log does not work on native 4K sector disks. Disks with native 4K sectors need 4K alignment and size for unbuffered IO (i.e for files opened with FILE_FLAG_NO_BUFFERING) Innodb opens redo log with FILE_FLAG_NO_BUFFERING, however it always does 512byte IOs. Thus, the IO on 4K native sectors will fail, rendering Innodb non-functional. The fix is to check whether OS_FILE_LOG_BLOCK_SIZE is multiple of logical sector size, and if it is not, reopen the redo log without FILE_FLAG_NO_BUFFERING flag. --- cmake/os/Windows.cmake | 2 +- storage/innobase/os/os0file.cc | 97 +++++++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 31 deletions(-) diff --git a/cmake/os/Windows.cmake b/cmake/os/Windows.cmake index c6cbfca2346..e94099670f9 100644 --- a/cmake/os/Windows.cmake +++ b/cmake/os/Windows.cmake @@ -47,7 +47,7 @@ IF(CMAKE_C_COMPILER MATCHES "icl") ENDIF() ADD_DEFINITIONS(-D_WINDOWS -D__WIN__ -D_CRT_SECURE_NO_DEPRECATE) -ADD_DEFINITIONS(-D_WIN32_WINNT=0x0600) +ADD_DEFINITIONS(-D_WIN32_WINNT=0x0A00) # We do not want the windows.h macros min/max ADD_DEFINITIONS(-DNOMINMAX) # Speed up build process excluding unused header files diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 01b801461d6..45fb04600f3 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -4109,6 +4109,32 @@ next_file: return(status); } +/** Check that IO of specific size is possible for the file +opened with FILE_FLAG_NO_BUFFERING. + +The requirement is that IO is multiple of the disk sector size. + +@param[in] file file handle +@param[in] io_size expected io size +@return true - unbuffered io of requested size is possible, false otherwise. + +@note: this function only works correctly with Windows 8 or later, +(GetFileInformationByHandleEx with FileStorageInfo is only supported there). +It will return true on earlier Windows version. + */ +static bool unbuffered_io_possible(HANDLE file, size_t io_size) +{ + FILE_STORAGE_INFO info; + if (GetFileInformationByHandleEx( + file, FileStorageInfo, &info, sizeof(info))) { + ULONG sector_size = info.LogicalBytesPerSector; + if (sector_size) + return io_size % sector_size == 0; + } + return true; +} + + /** NOTE! Use the corresponding macro os_file_create(), not directly this function! Opens an existing file or creates a new. @@ -4284,46 +4310,57 @@ os_file_create_func( access |= GENERIC_WRITE; } - do { + for (;;) { + const char *operation; + /* Use default security attributes and no template file. */ file = CreateFile( - (LPCTSTR) name, access, share_mode, NULL, + name, access, share_mode, NULL, create_flag, attributes, NULL); - if (file == INVALID_HANDLE_VALUE) { - const char* operation; + /* If FILE_FLAG_NO_BUFFERING was set, check if this can work at all, + for expected IO sizes. Reopen without the unbuffered flag, if it is won't work*/ + if ((file != INVALID_HANDLE_VALUE) + && (attributes & FILE_FLAG_NO_BUFFERING) + && (type == OS_LOG_FILE) + && !unbuffered_io_possible(file, OS_FILE_LOG_BLOCK_SIZE)) { + ut_a(CloseHandle(file)); + attributes &= ~FILE_FLAG_NO_BUFFERING; + continue; + } - operation = (create_mode == OS_FILE_CREATE - && !read_only) - ? "create" : "open"; + *success = (file != INVALID_HANDLE_VALUE); + if (*success) { + break; + } - *success = false; + operation = (create_mode == OS_FILE_CREATE && !read_only) ? + "create" : "open"; - if (on_error_no_exit) { - retry = os_file_handle_error_no_exit( - name, operation, on_error_silent); - } else { - retry = os_file_handle_error(name, operation); - } - } else { - - retry = false; + if (on_error_no_exit) { + retry = os_file_handle_error_no_exit( + name, operation, on_error_silent); + } + else { + retry = os_file_handle_error(name, operation); + } - *success = true; + if (!retry) { + break; + } + } - if (srv_use_native_aio && ((attributes & FILE_FLAG_OVERLAPPED) != 0)) { - /* Bind the file handle to completion port. Completion port - might not be created yet, in some stages of backup, but - must always be there for the server.*/ - HANDLE port =(type == OS_LOG_FILE)? - log_completion_port : data_completion_port; - ut_a(port || srv_operation != SRV_OPERATION_NORMAL); - if (port) { - ut_a(CreateIoCompletionPort(file, port, 0, 0)); - } - } + if (*success && srv_use_native_aio && (attributes & FILE_FLAG_OVERLAPPED)) { + /* Bind the file handle to completion port. Completion port + might not be created yet, in some stages of backup, but + must always be there for the server.*/ + HANDLE port = (type == OS_LOG_FILE) ? + log_completion_port : data_completion_port; + ut_a(port || srv_operation != SRV_OPERATION_NORMAL); + if (port) { + ut_a(CreateIoCompletionPort(file, port, 0, 0)); } - } while (retry); + } return(file); } -- cgit v1.2.1 From b71c9ae030ecafb31bc0b424a19d1354f2dd424b Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Sun, 1 Jul 2018 13:55:38 +0100 Subject: amend fix for MDEV-16596 - do not use CREATE_NEW flag when reopening redo log file. use OPEN_ALWAYS instead, since we know file already exist. --- storage/innobase/os/os0file.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 45fb04600f3..9853263600e 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -4326,6 +4326,7 @@ os_file_create_func( && !unbuffered_io_possible(file, OS_FILE_LOG_BLOCK_SIZE)) { ut_a(CloseHandle(file)); attributes &= ~FILE_FLAG_NO_BUFFERING; + create_flag = OPEN_ALWAYS; continue; } -- cgit v1.2.1 From 8639e288086247ce39917f4cb55191c8bb5b5a8c Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Fri, 20 Apr 2018 07:06:25 +0000 Subject: MDEV-16630: Ambiguous error message when check constraint matches table name One can create table with the same name for `field` and `table` `check` constraint. For example: `create table t(a int check(a>0), constraint a check(a>10));` But when inserting new rows same error is always raised. For example with ```insert into t values (-1);``` and ```insert into t values (10);``` same error `ER_CONSTRAINT_FAILED` is obtained and it is not clear which constraint is violated. This patch solve this error so that in case if field constraint is violated the first parameter in the error message is `table.field_name` and if table constraint is violated the first parameter in error message is `constraint_name`. --- mysql-test/r/check_constraint.result | 37 ++++++++++++++++++++++++++++++------ mysql-test/r/constraints.result | 2 +- mysql-test/r/type_json.result | 2 +- mysql-test/t/check_constraint.test | 28 +++++++++++++++++++++++++++ sql/table.cc | 12 +++++++++++- 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/mysql-test/r/check_constraint.result b/mysql-test/r/check_constraint.result index 9a32e6f12bc..9a228f9ccc7 100644 --- a/mysql-test/r/check_constraint.result +++ b/mysql-test/r/check_constraint.result @@ -10,9 +10,9 @@ t1 CREATE TABLE `t1` ( ) ENGINE=MyISAM DEFAULT CHARSET=latin1 insert into t1 values (100,100); insert into t1 values (1,1); -ERROR 23000: CONSTRAINT `a` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.a` failed for `test`.`t1` insert into t1 values (20,1); -ERROR 23000: CONSTRAINT `b` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.b` failed for `test`.`t1` insert into t1 values (20,30); ERROR 23000: CONSTRAINT `min` failed for `test`.`t1` insert into t1 values (500,500); @@ -60,7 +60,7 @@ t1 CREATE TABLE `t1` ( CONSTRAINT `CONSTRAINT_1` CHECK (`a` + `b` + `c` < 500) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 insert into t1 values(105,105,105); -ERROR 23000: CONSTRAINT `c` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.c` failed for `test`.`t1` insert into t1 values(249,249,9); ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t1` insert into t1 values(105,105,9); @@ -145,7 +145,7 @@ ERROR HY000: Function or expression '@b' cannot be used in the CHECK clause of ` create table t1 (a int check (a = 1)); insert t1 values (1); insert t1 values (2); -ERROR 23000: CONSTRAINT `a` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.a` failed for `test`.`t1` insert t1 values (NULL); select * from t1; a @@ -165,13 +165,13 @@ ERROR 22007: Truncated incorrect DOUBLE value: 'Ken' SHOW WARNINGS; Level Code Message Error 1292 Truncated incorrect DOUBLE value: 'Ken' -Error 4025 CONSTRAINT `FirstName` failed for `test`.`t1` +Error 4025 CONSTRAINT `t1.FirstName` failed for `test`.`t1` INSERT INTO t1 VALUES (NULL, 'Ken'),(NULL, 'Brian'); ERROR 22007: Truncated incorrect DOUBLE value: 'Ken' SHOW WARNINGS; Level Code Message Error 1292 Truncated incorrect DOUBLE value: 'Ken' -Error 4025 CONSTRAINT `FirstName` failed for `test`.`t1` +Error 4025 CONSTRAINT `t1.FirstName` failed for `test`.`t1` INSERT IGNORE INTO t1 VALUES (NULL, 'Ken'); Warnings: Warning 1292 Truncated incorrect DOUBLE value: 'Ken' @@ -197,3 +197,28 @@ EmployeeID FirstName 5 Ken 6 Brian drop table t1; +# +# MDEV-16630: Ambiguous error message when check constraint +# matches table name +# +use test; +drop table if exists t; +create table t(a int, b int check(b>0), +constraint b check(a0), +constraint x check (a>10)); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL CHECK (`b` > 0), + CONSTRAINT `b` CHECK (`a` < `b`), + CONSTRAINT `a` CHECK (`a` > 0), + CONSTRAINT `x` CHECK (`a` > 10) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +# Field constraint 'b' will fail +insert into t values (-1, 0); +ERROR 23000: CONSTRAINT `t.b` failed for `test`.`t` +# Table constraint 'b' will fail +insert into t values (1,1); +ERROR 23000: CONSTRAINT `b` failed for `test`.`t` +drop table t; diff --git a/mysql-test/r/constraints.result b/mysql-test/r/constraints.result index fe9398ea8ce..57cfbfb3d37 100644 --- a/mysql-test/r/constraints.result +++ b/mysql-test/r/constraints.result @@ -7,7 +7,7 @@ t1 CREATE TABLE `t1` ( ) ENGINE=MyISAM DEFAULT CHARSET=latin1 insert into t1 values (1); insert into t1 values (0); -ERROR 23000: CONSTRAINT `a` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.a` failed for `test`.`t1` drop table t1; create table t1 (a int, b int, check (a>b)); show create table t1; diff --git a/mysql-test/r/type_json.result b/mysql-test/r/type_json.result index 2c1fdbe2b95..0045847097b 100644 --- a/mysql-test/r/type_json.result +++ b/mysql-test/r/type_json.result @@ -20,7 +20,7 @@ t1 CREATE TABLE `t1` ( ) ENGINE=MyISAM DEFAULT CHARSET=latin1 insert t1 values ('[]'); insert t1 values ('a'); -ERROR 23000: CONSTRAINT `a` failed for `test`.`t1` +ERROR 23000: CONSTRAINT `t1.a` failed for `test`.`t1` set timestamp=unix_timestamp('2010:11:12 13:14:15'); create or replace table t1(a json default(json_object('now', now()))); show create table t1; diff --git a/mysql-test/t/check_constraint.test b/mysql-test/t/check_constraint.test index 02081071bd4..be06c3ec579 100644 --- a/mysql-test/t/check_constraint.test +++ b/mysql-test/t/check_constraint.test @@ -135,3 +135,31 @@ INSERT INTO t1 VALUES (NULL, 'Ken'),(NULL, 'Brian'); set sql_mode=default; select * from t1; drop table t1; + +--echo # +--echo # MDEV-16630: Ambiguous error message when check constraint +--echo # matches table name +--echo # + +use test; +--disable_warnings +drop table if exists t; +--enable_warnings + +create table t(a int, b int check(b>0), + constraint b check(a0), + constraint x check (a>10)); + +show create table t; + +# Generate error when field constraint 'b' is violated +--echo # Field constraint 'b' will fail +--error ER_CONSTRAINT_FAILED +insert into t values (-1, 0); + +# Generate error when table constraint 'b' is violated. +--echo # Table constraint 'b' will fail +--error ER_CONSTRAINT_FAILED +insert into t values (1,1); + +drop table t; diff --git a/sql/table.cc b/sql/table.cc index 917d194047c..7cb84bcc5ea 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -5142,8 +5142,18 @@ int TABLE::verify_constraints(bool ignore_failure) if (((*chk)->expr->val_int() == 0 && !(*chk)->expr->null_value) || in_use->is_error()) { + StringBuffer field_error(system_charset_info); + enum_vcol_info_type vcol_type= (*chk)->get_vcol_type(); + DBUG_ASSERT(vcol_type == VCOL_CHECK_TABLE || + vcol_type == VCOL_CHECK_FIELD); + if (vcol_type == VCOL_CHECK_FIELD) + { + field_error.append(s->table_name.str); + field_error.append("."); + } + field_error.append((*chk)->name.str); my_error(ER_CONSTRAINT_FAILED, - MYF(ignore_failure ? ME_JUST_WARNING : 0), (*chk)->name.str, + MYF(ignore_failure ? ME_JUST_WARNING : 0), field_error.c_ptr(), s->db.str, s->table_name.str); return ignore_failure ? VIEW_CHECK_SKIP : VIEW_CHECK_ERROR; } -- cgit v1.2.1 From 400cf017152c732387c89deaa082b43c8fb42d71 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Mon, 2 Jul 2018 17:19:42 +0100 Subject: MDEV-16571 - some backup tests sometimes with missing data after restore. Marko mentions, it could be caused by MDEV-15740 where InnoDB does not flush redo log as often as it should, with innodb_flush_log_at_trx_commit=1 The workaround is to use innodb_flush_log_at_trx_commit=2, which, according to MDEV-15740 is more durable. --- mysql-test/suite/mariabackup/suite.opt | 2 +- mysql-test/suite/mariabackup/unsupported_redo.result | 3 --- mysql-test/suite/mariabackup/unsupported_redo.test | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/mysql-test/suite/mariabackup/suite.opt b/mysql-test/suite/mariabackup/suite.opt index 3b5cc4f4c45..de3637814b2 100644 --- a/mysql-test/suite/mariabackup/suite.opt +++ b/mysql-test/suite/mariabackup/suite.opt @@ -1 +1 @@ ---innodb --loose-changed_page_bitmaps --innodb-sys-tables +--innodb --loose-changed_page_bitmaps --innodb-sys-tables --innodb-flush-log-at-trx-commit=2 diff --git a/mysql-test/suite/mariabackup/unsupported_redo.result b/mysql-test/suite/mariabackup/unsupported_redo.result index 543e564d8a8..a1f95c099cd 100644 --- a/mysql-test/suite/mariabackup/unsupported_redo.result +++ b/mysql-test/suite/mariabackup/unsupported_redo.result @@ -5,9 +5,6 @@ call mtr.add_suppression("InnoDB: If you are installing InnoDB, remember that yo call mtr.add_suppression("InnoDB: Ignoring tablespace for `test`\\.`t21` because it could not be opened"); call mtr.add_suppression("InnoDB: Cannot open datafile for read-only: "); call mtr.add_suppression("Table .* in the InnoDB data dictionary has tablespace id .*, but tablespace with that id or name does not exist"); -SELECT @@GLOBAL.innodb_flush_log_at_trx_commit; -@@GLOBAL.innodb_flush_log_at_trx_commit -1 CREATE TABLE t1(i INT PRIMARY KEY auto_increment, a int) ENGINE INNODB; ALTER TABLE t1 FORCE, ALGORITHM=INPLACE; # Fails during full backup diff --git a/mysql-test/suite/mariabackup/unsupported_redo.test b/mysql-test/suite/mariabackup/unsupported_redo.test index 9d54c5bbe87..319ee2c7571 100644 --- a/mysql-test/suite/mariabackup/unsupported_redo.test +++ b/mysql-test/suite/mariabackup/unsupported_redo.test @@ -10,7 +10,6 @@ call mtr.add_suppression("Table .* in the InnoDB data dictionary has tablespace let $basedir=$MYSQLTEST_VARDIR/tmp/backup; let $incremental_dir=$MYSQLTEST_VARDIR/tmp/backup_inc1; -SELECT @@GLOBAL.innodb_flush_log_at_trx_commit; CREATE TABLE t1(i INT PRIMARY KEY auto_increment, a int) ENGINE INNODB; --source ../../suite/innodb/include/no_checkpoint_start.inc ALTER TABLE t1 FORCE, ALGORITHM=INPLACE; -- cgit v1.2.1