From 24faa5de16c980fa5575cfc4a89618e48e9a3305 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Wed, 28 Jun 2023 12:22:56 +1000 Subject: [PATCH 1/7] MDEV-30542 Fixing spider/bugfix.self_reference_multi The server needs to have a unique name --- .../spider/bugfix/r/self_reference_multi.result | 8 ++++---- .../spider/bugfix/t/self_reference_multi.test | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result b/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result index c4399ddf9d2..50db034c8cd 100644 --- a/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result +++ b/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result @@ -4,11 +4,11 @@ for child3 MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever -CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); create table t2 (c int); -create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"'; -create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t1"'; -alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t0"'; +create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t2"'; +create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t1"'; +alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t0"'; select * from t0; ERROR HY000: An infinite loop is detected when opening table test.t0 select * from t1; diff --git a/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test b/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test index 8b6f070d167..a3d561f3fae 100644 --- a/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test +++ b/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test @@ -8,12 +8,12 @@ --echo MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever --echo ---replace_regex /SOCKET ".*"/SOCKET "$MASTER_1_MYSOCK"/ -eval CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +--let $srv=srv_self_reference_multi +evalp CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); create table t2 (c int); -create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"'; -create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t1"'; -alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t0"'; +eval create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t2"'; +eval create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t1"'; +eval alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t0"'; --error 12719 select * from t0; --error 12719 From 33877cfeae6e36ac177e3102dd98ac19f520ef74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 28 Jun 2023 17:07:00 +0300 Subject: [PATCH 2/7] Fix WITH_UBSAN GCC -Wconversion --- storage/innobase/rem/rem0rec.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/storage/innobase/rem/rem0rec.cc b/storage/innobase/rem/rem0rec.cc index d202afa9e20..0adea369037 100644 --- a/storage/innobase/rem/rem0rec.cc +++ b/storage/innobase/rem/rem0rec.cc @@ -242,9 +242,11 @@ enum rec_leaf_format { REC_LEAF_INSTANT }; -#if defined __GNUC__ && !defined __clang__ && __GNUC__ < 12 +#if defined __GNUC__ && !defined __clang__ # pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wconversion" /* GCC 5 to 11 need this */ +# if __GNUC__ < 12 || defined WITH_UBSAN +# pragma GCC diagnostic ignored "-Wconversion" +# endif #endif /** Determine the offset to each field in a leaf-page record in ROW_FORMAT=COMPACT,DYNAMIC,COMPRESSED. @@ -1707,7 +1709,7 @@ rec_convert_dtuple_to_rec_new( REC_INFO_BITS_MASK, REC_INFO_BITS_SHIFT); return buf; } -#if defined __GNUC__ && !defined __clang__ && __GNUC__ < 11 +#if defined __GNUC__ && !defined __clang__ # pragma GCC diagnostic pop /* ignored "-Wconversion" */ #endif From 0d3720c12a6ff0a2bb104c852e6c204c887b3dd6 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 15 May 2023 12:41:31 +0400 Subject: [PATCH 3/7] MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks The parser works as follows: The rule expr_lex returns a pointer to a newly created sp_expr_lex instance which is not linked to any MariaDB structures yet - it is pointed only from a Bison stack variable. The sp_expr_lex instance gets linked to other structures (such as sp_instr_jump_if_not) later, after scanning some following grammar. Problem before the fix: If a parse error happened immediately after expr_lex (before it got linked), the created sp_expr_lex value got lost causing a memory leak. Fix: - Using Bison's "destructor" directive to free the results of expr_lex on parse/oom errors. - Moving the call for LEX::cleanup_lex_after_parse_error() from MYSQL_YYABORT and yyerror inside parse_sql(). This is needed because Bison calls destructors after yyerror(), while it's important to delete the sp_expr_lex instance before LEX::cleanup_lex_after_parse_error(). The latter frees the memory root containing the sp_expr_lex instance. After this change the code block are executed in the following order: - yyerror() -- now only raises the error to DA (no cleanup done any more) - %destructor { delete $$; } -- destructs the sp_expr_lex instance - LEX::cleanup_lex_after_parse_error() -- frees the memory root containing the sp_expr_lex instance - Removing the "delete sublex" related code from restore_lex(): - restore_lex() is called in most cases on success, when delete is not needed. - There is one place when restore_lex() is called on error: In sp_create_assignment_instr(). But in this case LEX::sp_lex_in_use is true anyway. The patch adds a new DBUG_ASSERT(lex->sp_lex_in_use) to guard this. --- mysql-test/main/sp-memory-leak.result | 21 +++++++++++++++++++ mysql-test/main/sp-memory-leak.test | 29 +++++++++++++++++++++++++++ sql/sp_head.h | 24 +++++++++++++--------- sql/sql_lex.cc | 11 ++++++---- sql/sql_parse.cc | 11 ++++++++++ sql/sql_yacc.yy | 24 ++++++++++++---------- 6 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 mysql-test/main/sp-memory-leak.result create mode 100644 mysql-test/main/sp-memory-leak.test diff --git a/mysql-test/main/sp-memory-leak.result b/mysql-test/main/sp-memory-leak.result new file mode 100644 index 00000000000..aea278801d8 --- /dev/null +++ b/mysql-test/main/sp-memory-leak.result @@ -0,0 +1,21 @@ +# +# MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks +# +BEGIN NOT ATOMIC +IF SCALAR() expected_THEN_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_THEN_here; +END' at line 2 +BEGIN NOT ATOMIC +WHILE SCALAR() expected_DO_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_DO_here; +END' at line 2 +BEGIN NOT ATOMIC +REPEAT SELECT 1; UNTIL SCALAR() expected_END_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_END_here; +END' at line 2 diff --git a/mysql-test/main/sp-memory-leak.test b/mysql-test/main/sp-memory-leak.test new file mode 100644 index 00000000000..5b346aa8b10 --- /dev/null +++ b/mysql-test/main/sp-memory-leak.test @@ -0,0 +1,29 @@ +--echo # +--echo # MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks +--echo # + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + IF SCALAR() expected_THEN_here; +END +$$ +DELIMITER ;$$ + + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + WHILE SCALAR() expected_DO_here; +END +$$ +DELIMITER ;$$ + + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + REPEAT SELECT 1; UNTIL SCALAR() expected_END_here; +END +$$ +DELIMITER ;$$ diff --git a/sql/sp_head.h b/sql/sp_head.h index 12db9c485cf..700485ffd21 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -608,20 +608,24 @@ public: restore_lex(THD *thd) { DBUG_ENTER("sp_head::restore_lex"); + /* + There is no a need to free the current thd->lex here. + - In the majority of the cases restore_lex() is called + on success and thd->lex does not need to be deleted. + - In cases when restore_lex() is called on error, + e.g. from sp_create_assignment_instr(), thd->lex is + already linked to some sp_instr_xxx (using sp_lex_keeper). + + Note, we don't get to here in case of a syntax error + when the current thd->lex is not yet completely + initialized and linked. It gets automatically deleted + by the Bison %destructor in sql_yacc.yy. + */ LEX *oldlex= (LEX *) m_lex.pop(); if (!oldlex) DBUG_RETURN(false); // Nothing to restore - LEX *sublex= thd->lex; // This restores thd->lex and thd->stmt_lex - if (thd->restore_from_local_lex_to_old_lex(oldlex)) - DBUG_RETURN(true); - if (!sublex->sp_lex_in_use) - { - sublex->sphead= NULL; - lex_end(sublex); - delete sublex; - } - DBUG_RETURN(false); + DBUG_RETURN(thd->restore_from_local_lex_to_old_lex(oldlex)); } /** diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 257acebe9e0..7e0ca6d868d 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -485,11 +485,15 @@ bool sp_create_assignment_instr(THD *thd, bool no_lookahead, be deleted by the destructor ~sp_instr_xxx(). So we should remove "lex" from the stack sp_head::m_lex, to avoid double free. - Note, in case "lex" is not owned by any sp_instr_xxx, - it's also safe to remove it from the stack right now. - So we can remove it unconditionally, without testing lex->sp_lex_in_use. */ lex->sphead->restore_lex(thd); + /* + No needs for "delete lex" here: "lex" is already linked + to the sp_instr_stmt (using sp_lex_keeper) instance created by + the call for new_sp_instr_stmt() above. It will be freed + by ~sp_head/~sp_instr/~sp_lex_keeper during THD::end_statement(). + */ + DBUG_ASSERT(lex->sp_lex_in_use); // used by sp_instr_stmt return true; } enum_var_type inner_option_type= lex->option_type; @@ -6773,7 +6777,6 @@ bool LEX::sp_for_loop_implicit_cursor_statement(THD *thd, if (unlikely(!(bounds->m_index= new (thd->mem_root) sp_assignment_lex(thd, this)))) return true; - bounds->m_index->sp_lex_in_use= true; sphead->reset_lex(thd, bounds->m_index); DBUG_ASSERT(thd->lex != this); /* diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 59f21247445..1e8ea5c5c80 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -10454,6 +10454,17 @@ bool parse_sql(THD *thd, Parser_state *parser_state, ((thd->variables.sql_mode & MODE_ORACLE) ? ORAparse(thd) : MYSQLparse(thd)) != 0; + + if (mysql_parse_status) + { + /* + Restore the original LEX if it was replaced when parsing + a stored procedure. We must ensure that a parsing error + does not leave any side effects in the THD. + */ + LEX::cleanup_lex_after_parse_error(thd); + } + DBUG_ASSERT(opt_bootstrap || mysql_parse_status || thd->lex->select_stack_top == 0); thd->lex->current_select= thd->lex->first_select_lex(); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index a5a39228776..601ff119fc3 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -98,7 +98,6 @@ int yylex(void *yylval, void *yythd); #define MYSQL_YYABORT \ do \ { \ - LEX::cleanup_lex_after_parse_error(thd); \ YYABORT; \ } while (0) @@ -149,13 +148,6 @@ static Item* escape(THD *thd) static void yyerror(THD *thd, const char *s) { - /* - Restore the original LEX if it was replaced when parsing - a stored procedure. We must ensure that a parsing error - does not leave any side effects in the THD. - */ - LEX::cleanup_lex_after_parse_error(thd); - /* "parse error" changed into "syntax error" between bison 1.75 and 1.875 */ if (strcmp(s,"parse error") == 0 || strcmp(s,"syntax error") == 0) s= ER_THD(thd, ER_SYNTAX_ERROR); @@ -1521,6 +1513,19 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); %type expr_lex +%destructor +{ + /* + In case of a syntax/oom error let's free the sp_expr_lex + instance, but only if it has not been linked to any structures + such as sp_instr_jump_if_not::m_lex_keeper yet, e.g.: + IF f1() THEN1 + i.e. THEN1 came instead of the expected THEN causing a syntax error. + */ + if (!$$->sp_lex_in_use) + delete $$; +} + %type assignment_source_lex assignment_source_expr @@ -3806,7 +3811,6 @@ expr_lex: expr { $$= $1; - $$->sp_lex_in_use= true; $$->set_item($2); Lex->pop_select(); //min select if (Lex->check_cte_dependencies_and_resolve_references()) @@ -3838,7 +3842,6 @@ assignment_source_expr: { DBUG_ASSERT($1 == thd->lex); $$= $1; - $$->sp_lex_in_use= true; $$->set_item_and_free_list($3, thd->free_list); thd->free_list= NULL; Lex->pop_select(); //min select @@ -3859,7 +3862,6 @@ for_loop_bound_expr: { DBUG_ASSERT($1 == thd->lex); $$= $1; - $$->sp_lex_in_use= true; $$->set_item_and_free_list($3, NULL); Lex->pop_select(); //main select if (unlikely($$->sphead->restore_lex(thd))) From fdab2c4c648cd3f2db524ef7534843a9d29c4f4f Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 29 Jun 2023 17:30:02 +0400 Subject: [PATCH 4/7] MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error When CURSOR parameters get parsed, their sp_assignment_lex instances (one instance per parameter) get collected to List. These instances get linked to sphead only in the end of the list. If a syntax error happened in the middle of the parameter list, these instances were not deleted, which caused memory leaks. Fix: using a Bison %destructor to free rules of the type (on syntax errors). Afte the fix these sp_assignment_lex instances from CURSOR parameters deleted as follows: - If the CURSOR statement was fully parsed, then these instances get properly linked to sp_head structures, so they are deleted during ~sp_head (this did not change) - If the CURSOR statement failed on a syntax error, then by Bison's %destructor (this is being added in the current patch). --- mysql-test/main/sp-memory-leak.result | 21 +++++++++++++++++++++ mysql-test/main/sp-memory-leak.test | 25 +++++++++++++++++++++++++ sql/sql_lex.h | 6 +++++- sql/sql_yacc.yy | 15 +++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/mysql-test/main/sp-memory-leak.result b/mysql-test/main/sp-memory-leak.result index aea278801d8..37a0f119341 100644 --- a/mysql-test/main/sp-memory-leak.result +++ b/mysql-test/main/sp-memory-leak.result @@ -19,3 +19,24 @@ END $$ ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_END_here; END' at line 2 +# +# MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error +# +BEGIN NOT ATOMIC +DECLARE cur CURSOR (a INT) FOR SELECT a+1; +OPEN cur(sp_followed_by_syntax_error(); +CLOSE cur; +END; +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '; +CLOSE cur; +END' at line 3 +BEGIN NOT ATOMIC +DECLARE cur CURSOR (a INT) FOR SELECT a+1; +OPEN cur(1,sp_followed_by_syntax_error(); +CLOSE cur; +END; +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '; +CLOSE cur; +END' at line 3 diff --git a/mysql-test/main/sp-memory-leak.test b/mysql-test/main/sp-memory-leak.test index 5b346aa8b10..0035044209a 100644 --- a/mysql-test/main/sp-memory-leak.test +++ b/mysql-test/main/sp-memory-leak.test @@ -27,3 +27,28 @@ BEGIN NOT ATOMIC END $$ DELIMITER ;$$ + + +--echo # +--echo # MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error +--echo # + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + DECLARE cur CURSOR (a INT) FOR SELECT a+1; + OPEN cur(sp_followed_by_syntax_error(); + CLOSE cur; +END; +$$ +DELIMITER ;$$ + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + DECLARE cur CURSOR (a INT) FOR SELECT a+1; + OPEN cur(1,sp_followed_by_syntax_error(); + CLOSE cur; +END; +$$ +DELIMITER ;$$ diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 2e3ce7d7503..5538fdd04e6 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -3429,7 +3429,11 @@ public: sp_head *sphead; sp_name *spname; bool sp_lex_in_use; // Keep track on lex usage in SPs for error handling - + void delete_if_not_sp_lex_in_use() + { + if (!sp_lex_in_use) + delete this; + } sp_pcontext *spcont; st_sp_chistics sp_chistics; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 601ff119fc3..524da7be4c3 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1535,6 +1535,21 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); cursor_actual_parameters opt_parenthesized_cursor_actual_parameters +%destructor +{ + if ($$) + { + sp_assignment_lex *elem; + List_iterator li(*$$); + while ((elem= li++)) + { + if (!elem->sp_lex_in_use) + delete elem; + } + } +} + + %type option_type opt_var_type opt_var_ident_type From 5f2a77cef1cced322d3a6e6a48f4f4e5480283dc Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sat, 10 Jun 2023 08:35:58 +1000 Subject: [PATCH 5/7] MDEV-31268 Fedora MariaDB-shared replaces mariadb-connector-c file /usr/lib64/libmariadb.so.3 from install of MariaDB-shared-10.11.3-1.fc38.x86_64 conflicts with file from package mariadb-connector-c-3.3.5-1.fc38.x86_64 --- cmake/cpack_rpm.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake index 7b1f0b0ff87..3cc24375c06 100644 --- a/cmake/cpack_rpm.cmake +++ b/cmake/cpack_rpm.cmake @@ -291,6 +291,7 @@ ENDIF() # MDEV-24629, we need it outside of ELSIFs IF(RPM MATCHES "fedora") ALTERNATIVE_NAME("common" "mariadb-connector-c-config" ${MARIADB_CONNECTOR_C_VERSION}-1) + ALTERNATIVE_NAME("shared" "mariadb-connector-c" ${MARIADB_CONNECTOR_C_VERSION}-1) ENDIF() SET(PYTHON_SHEBANG "/usr/bin/python3" CACHE STRING "python shebang") From 5b62644e68370d504ce4b779ed53e625d500451c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 5 Jul 2023 08:48:37 +0300 Subject: [PATCH 6/7] MDEV-31621 Remove ibuf_read_merge_pages() call from ibuf_insert_low() When InnoDB attempts to buffer a change operation of a secondary index leaf page (to insert, delete-mark or remove a record) and the change buffer is too large, InnoDB used to trigger a change buffer merge that could affect any tables. This could lead to huge variance in system throughput and potentially unpredictable crashes, in case the change buffer was corrupted and a crash occurred while attempting to merge changes to a table that is not being accessed by the current SQL statement. ibuf_insert_low(): Simply return DB_STRONG_FAIL when the maximum size of the change buffer is exceeded. ibuf_contract_after_insert(): Remove. ibuf_get_merge_page_nos_func(): Remove a constant parameter. The function ibuf_contract() will be our only caller, during shutdown with innodb_fast_shutdown=0. --- storage/innobase/ibuf/ibuf0ibuf.cc | 129 ++++------------------------- 1 file changed, 17 insertions(+), 112 deletions(-) diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc index d1d6176720a..8226ebbfd54 100644 --- a/storage/innobase/ibuf/ibuf0ibuf.cc +++ b/storage/innobase/ibuf/ibuf0ibuf.cc @@ -243,21 +243,11 @@ mysql_mutex_t ibuf_mutex, ibuf_pessimistic_insert_mutex; /** The area in pages from which contract looks for page numbers for merge */ -const ulint IBUF_MERGE_AREA = 8; +constexpr ulint IBUF_MERGE_AREA = 8; -/** Inside the merge area, pages which have at most 1 per this number less -buffered entries compared to maximum volume that can buffered for a single -page are merged along with the page whose buffer became full */ -const ulint IBUF_MERGE_THRESHOLD = 4; - -/** In ibuf_contract at most this number of pages is read to memory in one -batch, in order to merge the entries for them in the insert buffer */ -const ulint IBUF_MAX_N_PAGES_MERGED = IBUF_MERGE_AREA; - -/** If the combined size of the ibuf trees exceeds ibuf.max_size by -this many pages, we start to contract it synchronous contract, but do -not insert */ -const ulint IBUF_CONTRACT_DO_NOT_INSERT = 10; +/** In ibuf_contract() at most this number of pages is read to memory in one +batch, in order to merge the entries for them in the change buffer */ +constexpr ulint IBUF_MAX_N_PAGES_MERGED = IBUF_MERGE_AREA; /* TODO: how to cope with drop table if there are records in the insert buffer for the indexes of the table? Is there actually any problem, @@ -2004,11 +1994,11 @@ ibuf_free_excess_pages(void) } #ifdef UNIV_DEBUG -# define ibuf_get_merge_page_nos(contract,rec,mtr,ids,pages,n_stored) \ - ibuf_get_merge_page_nos_func(contract,rec,mtr,ids,pages,n_stored) +# define ibuf_get_merge_page_nos(rec,mtr,ids,pages,n_stored) \ + ibuf_get_merge_page_nos_func(rec,mtr,ids,pages,n_stored) #else /* UNIV_DEBUG */ -# define ibuf_get_merge_page_nos(contract,rec,mtr,ids,pages,n_stored) \ - ibuf_get_merge_page_nos_func(contract,rec,ids,pages,n_stored) +# define ibuf_get_merge_page_nos(rec,mtr,ids,pages,n_stored) \ + ibuf_get_merge_page_nos_func(rec,ids,pages,n_stored) #endif /* UNIV_DEBUG */ /*********************************************************************//** @@ -2019,10 +2009,6 @@ static ulint ibuf_get_merge_page_nos_func( /*=========================*/ - ibool contract,/*!< in: TRUE if this function is called to - contract the tree, FALSE if this is called - when a single page becomes full and we look - if it pays to read also nearby pages */ const rec_t* rec, /*!< in: insert buffer record */ #ifdef UNIV_DEBUG mtr_t* mtr, /*!< in: mini-transaction holding rec */ @@ -2153,22 +2139,10 @@ corruption: || rec_page_no != prev_page_no) && (prev_space_id != 0 || prev_page_no != 0)) { - if (contract - || (prev_page_no == first_page_no - && prev_space_id == first_space_id) - || (volume_for_page - > ((IBUF_MERGE_THRESHOLD - 1) - * 4U << srv_page_size_shift - / IBUF_PAGE_SIZE_PER_FREE_SPACE) - / IBUF_MERGE_THRESHOLD)) { - - space_ids[*n_stored] = prev_space_id; - page_nos[*n_stored] = prev_page_no; - - (*n_stored)++; - - sum_volumes += volume_for_page; - } + space_ids[*n_stored] = prev_space_id; + page_nos[*n_stored] = prev_page_no; + (*n_stored)++; + sum_volumes += volume_for_page; if (rec_space_id != first_space_id || rec_page_no / IBUF_MERGE_AREA @@ -2428,7 +2402,7 @@ tablespace_deleted: @return a lower limit for the combined size in bytes of entries which will be merged from ibuf trees to the pages read @retval 0 if ibuf.empty */ -ulint ibuf_contract() +ATTRIBUTE_COLD ulint ibuf_contract() { if (UNIV_UNLIKELY(!ibuf.index)) return 0; mtr_t mtr; @@ -2460,10 +2434,8 @@ ulint ibuf_contract() } ulint n_pages = 0; - sum_sizes = ibuf_get_merge_page_nos(TRUE, - btr_cur_get_rec(&cur), &mtr, - space_ids, - page_nos, &n_pages); + sum_sizes = ibuf_get_merge_page_nos(btr_cur_get_rec(&cur), &mtr, + space_ids, page_nos, &n_pages); ibuf_mtr_commit(&mtr); ibuf_read_merge_pages(space_ids, page_nos, n_pages); @@ -2553,30 +2525,6 @@ ibuf_merge_space( return(n_pages); } -/*********************************************************************//** -Contract insert buffer trees after insert if they are too big. */ -UNIV_INLINE -void -ibuf_contract_after_insert( -/*=======================*/ - ulint entry_size) /*!< in: size of a record which was inserted - into an ibuf tree */ -{ - /* dirty comparison, to avoid contention on ibuf_mutex */ - if (ibuf.size < ibuf.max_size) { - return; - } - - /* Contract at least entry_size many bytes */ - ulint sum_sizes = 0; - ulint size; - - do { - size = ibuf_contract(); - sum_sizes += size; - } while (size > 0 && sum_sizes < entry_size); -} - /** Determine if a change buffer record has been encountered already. @param rec change buffer record in the MySQL 5.5 format @param hash hash table of encountered records @@ -3175,10 +3123,6 @@ ibuf_insert_low( buf_block_t* block = NULL; page_t* root; dberr_t err; - ibool do_merge; - uint32_t space_ids[IBUF_MAX_N_PAGES_MERGED]; - uint32_t page_nos[IBUF_MAX_N_PAGES_MERGED]; - ulint n_stored; mtr_t mtr; mtr_t bitmap_mtr; @@ -3189,28 +3133,9 @@ ibuf_insert_low( ut_ad(page_id.space() == index->table->space_id); ut_a(op < IBUF_OP_COUNT); - do_merge = FALSE; - /* Perform dirty comparison of ibuf.max_size and ibuf.size to - reduce ibuf_mutex contention. This should be OK; at worst we - are doing some excessive ibuf_contract() or occasionally - skipping an ibuf_contract(). */ - const ulint max_size = ibuf.max_size; - - if (max_size == 0) { - return(DB_STRONG_FAIL); - } - - if (ibuf.size >= max_size + IBUF_CONTRACT_DO_NOT_INSERT) { - /* Insert buffer is now too big, contract it but do not try - to insert */ - - -#ifdef UNIV_IBUF_DEBUG - fputs("Ibuf too big\n", stderr); -#endif - ibuf_contract(); - + reduce ibuf_mutex contention. */ + if (ibuf.size >= ibuf.max_size) { return(DB_STRONG_FAIL); } @@ -3262,17 +3187,6 @@ func_exit: ibuf_mtr_commit(&mtr); ut_free(pcur.old_rec_buf); mem_heap_free(heap); - - if (err == DB_SUCCESS && mode == BTR_INSERT_TREE) { - ibuf_contract_after_insert(entry_size); - } - - if (do_merge) { -#ifdef UNIV_IBUF_DEBUG - ut_a(n_stored <= IBUF_MAX_N_PAGES_MERGED); -#endif - ibuf_read_merge_pages(space_ids, page_nos, n_stored); - } return err; } @@ -3362,15 +3276,6 @@ commit_exit: bits)) { /* Release the bitmap page latch early. */ ibuf_mtr_commit(&bitmap_mtr); - - /* It may not fit */ - do_merge = TRUE; - - ibuf_get_merge_page_nos(FALSE, - btr_pcur_get_rec(&pcur), &mtr, - space_ids, - page_nos, &n_stored); - goto fail_exit; } } From bd7908e6ace1edce3530a0aa663ec3527e53c8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 5 Jul 2023 15:15:04 +0300 Subject: [PATCH 7/7] MDEV-31568 InnoDB protection against dual processes accessing data insufficient fil_node_open_file_low(): Always acquire an advisory lock on the system tablespace. Originally, we already did this in SysTablespace::open_file(), but SysTablespace::open_or_create() would release those locks when it is closing the file handles. This is a 10.5+ specific follow up to commit 0ee1082bd2e7e7049c4f0e686bad53cf7ba053ab (MDEV-28495). Thanks to Daniel Black for verifying this bug. --- storage/innobase/fil/fil0fil.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index c75144413ac..725345957ee 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -391,8 +391,16 @@ static bool fil_node_open_file_low(fil_node_t *node) : OS_FILE_OPEN | OS_FILE_ON_ERROR_NO_EXIT, OS_FILE_AIO, type, srv_read_only_mode, &success); - if (success) + if (node->is_open()) + { + ut_ad(success); +#ifndef _WIN32 + if (!node->space->id && !srv_read_only_mode && my_disable_locking && + os_file_lock(node->handle, node->name)) + goto fail; +#endif break; + } /* The following call prints an error message */ if (os_file_get_last_error(true) == EMFILE + 100 && @@ -406,6 +414,9 @@ static bool fil_node_open_file_low(fil_node_t *node) if (node->size); else if (!node->read_page0() || !fil_comp_algo_validate(node->space)) { +#ifndef _WIN32 + fail: +#endif os_file_close(node->handle); node->handle= OS_FILE_CLOSED; return false;