From 8c8bee0a5635d7f8197a148f267c04c1452b47fc Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 30 Apr 2019 15:51:49 +0400 Subject: MDEV-10307 CAST(11068046444225730969 AS SIGNED) does not return a warning --- sql/item.cc | 9 +++++++++ sql/item.h | 7 +++++++ sql/sql_type.cc | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/item.cc b/sql/item.cc index faae5c5ad03..a9139ceb0b1 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -333,6 +333,15 @@ longlong Item::val_int_unsigned_typecast_from_int() } +longlong Item::val_int_signed_typecast_from_int() +{ + longlong value= val_int(); + if (!null_value && unsigned_flag && value < 0) + push_note_converted_to_negative_complement(current_thd); + return value; +} + + String *Item::val_string_from_date(String *str) { MYSQL_TIME ltime; diff --git a/sql/item.h b/sql/item.h index c11a4fe56c0..4261ef64950 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1057,6 +1057,13 @@ public: longlong val_int_unsigned_typecast_from_decimal(); longlong val_int_unsigned_typecast_from_int(); longlong val_int_unsigned_typecast_from_str(); + + /** + Get a value for CAST(x AS UNSIGNED). + Huge positive unsigned values are converted to negative complements. + */ + longlong val_int_signed_typecast_from_int(); + /* This is just a shortcut to avoid the cast. You should still use unsigned_flag to check the sign of the item. diff --git a/sql/sql_type.cc b/sql/sql_type.cc index c2c853efa23..a40172967a7 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -3380,7 +3380,7 @@ longlong Type_handler_real_result:: longlong Type_handler_int_result:: Item_val_int_signed_typecast(Item *item) const { - return item->val_int(); + return item->val_int_signed_typecast_from_int(); } longlong Type_handler_decimal_result:: -- cgit v1.2.1 From cb9fa1a08bd1f4f609324971e58b9b300a5cb55f Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Tue, 30 Apr 2019 21:05:50 +0530 Subject: MDEV-9959: A serious MariaDB server performance bug If a derived table has SELECT DISTINCT, provide index statistics for it so that the join optimizer in the upper select knows that ref access to the table will produce one row. --- sql/sql_class.cc | 1 + sql/sql_lex.h | 1 + sql/sql_union.cc | 40 ++++++++++++++++++++++++++++++++++++++++ sql/table.cc | 20 ++++++++++++++++++++ 4 files changed, 62 insertions(+) (limited to 'sql') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c26aaff10d8..d247c806a7f 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -847,6 +847,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) invoker.init(); prepare_derived_at_open= FALSE; create_tmp_table_for_derived= FALSE; + force_read_stats= FALSE; save_prep_leaf_list= FALSE; org_charset= 0; /* Restore THR_THD */ diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 72ca4ac0b43..4eaec7d062b 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -896,6 +896,7 @@ public: bool union_needs_tmp_table(); void set_unique_exclude(); + bool check_distinct_in_union(); friend struct LEX; friend int subselect_union_engine::exec(); diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 87fbbebe4ba..3fb5552c77a 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -2049,3 +2049,43 @@ void st_select_lex_unit::set_unique_exclude() } } } + +/** + @brief + Check if the derived table is guaranteed to have distinct rows because of + UNION operations used to populate it. + + @detail + UNION operation removes duplicate rows from its output. That is, a query like + + select * from t1 UNION select * from t2 + + will not produce duplicate rows in its output, even if table t1 (and/or t2) + contain duplicate rows. EXCEPT and INTERSECT operations also have this + property. + + On the other hand, UNION ALL operation doesn't remove duplicates. (The SQL + standard also defines EXCEPT ALL and INTERSECT ALL, but we don't support + them). + + st_select_lex_unit computes its value left to right. That is, if there is + a st_select_lex_unit object describing + + (select #1) OP1 (select #2) OP2 (select #3) + + then ((select #1) OP1 (select #2)) is computed first, and OP2 is computed + second. + + How can one tell if st_select_lex_unit is guaranteed to have distinct + output rows? This depends on whether the last operation was duplicate- + removing or not: + - UNION ALL is not duplicate-removing + - all other operations are duplicate-removing +*/ + +bool st_select_lex_unit::check_distinct_in_union() +{ + if (union_distinct && !union_distinct->next_select()) + return true; + return false; +} diff --git a/sql/table.cc b/sql/table.cc index b4dcbaea6dc..7042959215d 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -7270,6 +7270,26 @@ bool TABLE::add_tmp_key(uint key, uint key_parts, key_part_info++; } + /* + For the case when there is a derived table that would give distinct rows, + the index statistics are passed to the join optimizer to tell that a ref + access to all the fields of the derived table will produce only one row. + */ + + st_select_lex_unit* derived= pos_in_table_list ? + pos_in_table_list->derived: NULL; + if (derived) + { + st_select_lex* first= derived->first_select(); + uint select_list_items= first->get_item_list()->elements; + if (key_parts == select_list_items) + { + if ((!first->is_part_of_union() && (first->options & SELECT_DISTINCT)) || + derived->check_distinct_in_union()) + keyinfo->rec_per_key[key_parts - 1]= 1; + } + } + set_if_bigger(s->max_key_length, keyinfo->key_length); s->keys++; return FALSE; -- cgit v1.2.1 From dabef66e66a48f2d5f4ca5fdfd1f9d927e935471 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Mon, 29 Apr 2019 20:32:36 +1000 Subject: MDEV-19188 Server Crash When Using a Trigger With A Number of Virtual Columns on INSERT/UPDATE use s->fields instead of s->stored_fields. extra_null_bitmap is allocated in Table_triggers_list::prepare_record_accessors with respect to virtual fields, so it will not overflow Closes #1292 --- sql/sql_trigger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/sql_trigger.h b/sql/sql_trigger.h index 9d1c79cc7cf..6e94f348447 100644 --- a/sql/sql_trigger.h +++ b/sql/sql_trigger.h @@ -274,7 +274,7 @@ public: Field **nullable_fields() { return record0_field; } void reset_extra_null_bitmap() { - size_t null_bytes= (trigger_table->s->stored_fields - + size_t null_bytes= (trigger_table->s->fields - trigger_table->s->null_fields + 7)/8; bzero(extra_null_bitmap, null_bytes); } -- cgit v1.2.1 From b953bf7eb2cbebf74b1f8861f4c45a789d365f28 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 1 May 2019 13:14:50 +0200 Subject: compilation fixes for VS 2019 --- sql/sql_analyze_stmt.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'sql') diff --git a/sql/sql_analyze_stmt.cc b/sql/sql_analyze_stmt.cc index 8e67267f6a0..9aed432c2b6 100644 --- a/sql/sql_analyze_stmt.cc +++ b/sql/sql_analyze_stmt.cc @@ -45,7 +45,7 @@ void Filesort_tracker::print_json_members(Json_writer *writer) else if (r_limit == 0) writer->add_str(varied_str); else - writer->add_ll((longlong) rint(r_limit)); + writer->add_ll(r_limit); } writer->add_member("r_used_priority_queue"); @@ -61,13 +61,13 @@ void Filesort_tracker::print_json_members(Json_writer *writer) if (!get_r_loops()) writer->add_member("r_output_rows").add_null(); else - writer->add_member("r_output_rows").add_ll((longlong) rint(r_output_rows / - get_r_loops())); + writer->add_member("r_output_rows").add_ll( + (longlong) rint((double)r_output_rows / get_r_loops())); if (sort_passes) { - writer->add_member("r_sort_passes").add_ll((longlong) rint(sort_passes / - get_r_loops())); + writer->add_member("r_sort_passes").add_ll( + (longlong) rint((double)sort_passes / get_r_loops())); } if (sort_buffer_size != 0) -- cgit v1.2.1 From dc8e15db7e170964699dec7b5214d20200b4b182 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 2 Jan 2018 12:00:55 +1100 Subject: MDEV-15051: signal handler - output information about the core generation The working directory, resource limits and core pattern will aid the user finding a core file in the case of failure. While the core file size is most relevant however other resource limits may give a clue as the the cause of the fatal signal so include them also. As signal handler functions are limited, proc filesystem reads/ readlink calls are used instead of the more obvious getcwd/getrlimits functions which aren't listed as signal safe. Results in output of the form: Writing a core file: working directory at /tmp/datadir Resource Limits: Limit Soft Limit Hard Limit Units Max cpu time unlimited unlimited seconds Max file size unlimited unlimited bytes Max data size unlimited unlimited bytes Max stack size 8388608 unlimited bytes Max core file size unlimited unlimited bytes Max resident set unlimited unlimited bytes Max processes 47194 47194 processes Max open files 1024 4096 files Max locked memory 65536 65536 bytes Max address space unlimited unlimited bytes Max file locks unlimited unlimited locks Max pending signals 47194 47194 signals Max msgqueue size 819200 819200 bytes Max nice priority 0 0 Max realtime priority 0 0 Max realtime timeout unlimited unlimited us Core pattern: |/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t %P %I Segmentation fault (core dumped) Closes #537 --- sql/signal_handler.cc | 46 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/signal_handler.cc b/sql/signal_handler.cc index 68e801d5885..6c0220b1254 100644 --- a/sql/signal_handler.cc +++ b/sql/signal_handler.cc @@ -30,6 +30,10 @@ #define SIGNAL_FMT "signal %d" #endif +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif + /* We are handling signals/exceptions in this file. Any global variables we read should be 'volatile sig_atomic_t' @@ -44,6 +48,43 @@ extern volatile sig_atomic_t ld_assume_kernel_is_set; extern const char *optimizer_switch_names[]; +static inline void output_core_info() +{ + /* proc is optional on some BSDs so it can't hurt to look */ +#ifdef HAVE_READLINK + char buff[PATH_MAX]; + ssize_t len; + int fd; + if ((len= readlink("/proc/self/cwd", buff, sizeof(buff))) >= 0) + { + my_safe_printf_stderr("Writing a core file...\nWorking directory at %.*s\n", + (int) len, buff); + } + if ((fd= my_open("/proc/self/limits", O_RDONLY, MYF(0))) >= 0) + { + my_safe_printf_stderr("Resource Limits:\n"); + while ((len= my_read(fd, (uchar*)buff, sizeof(buff), MYF(0))) > 0) + { + my_write_stderr(buff, len); + } + my_close(fd, MYF(0)); + } +#ifdef __linux__ + if ((fd= my_open("/proc/sys/kernel/core_pattern", O_RDONLY, MYF(0))) >= 0) + { + len= my_read(fd, (uchar*)buff, sizeof(buff), MYF(0)); + my_safe_printf_stderr("Core pattern: %.*s\n", (int) len, buff); + my_close(fd, MYF(0)); + } +#endif +#else + char buff[80]; + my_getwd(buff, sizeof(buff), 0); + my_safe_printf_stderr("Writing a core file at %s\n", buff); + fflush(stderr); +#endif +} + /** * Handler for fatal signals on POSIX, exception handler on Windows. * @@ -295,13 +336,10 @@ extern "C" sig_handler handle_fatal_signal(int sig) } #endif + output_core_info(); #ifdef HAVE_WRITE_CORE if (test_flags & TEST_CORE_ON_SIGNAL) { - char buff[80]; - my_getwd(buff, sizeof(buff), 0); - my_safe_printf_stderr("Writing a core file at %s\n", buff); - fflush(stderr); my_write_core(sig); } #endif -- cgit v1.2.1 From 2370eeb028b269243633b18f7661dca999089a41 Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Fri, 9 Nov 2018 06:12:43 -0800 Subject: MDEV-17654 Incorrect syntax returned for column with CHECK constraint in the "SHOW CREATE TABLE ..." result Prepend COMMENT before CHECK constraint in SHOW CREATE Closes #924 --- sql/sql_show.cc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'sql') diff --git a/sql/sql_show.cc b/sql/sql_show.cc index a29a6871b78..c84ac5f4977 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2065,6 +2065,16 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet, !(sql_mode & MODE_NO_FIELD_OPTIONS)) packet->append(STRING_WITH_LEN(" AUTO_INCREMENT")); } + + if (field->comment.length) + { + packet->append(STRING_WITH_LEN(" COMMENT ")); + append_unescaped(packet, field->comment.str, field->comment.length); + } + + append_create_options(thd, packet, field->option_list, check_options, + hton->field_options); + if (field->check_constraint) { StringBuffer str(&my_charset_utf8mb4_general_ci); @@ -2074,13 +2084,6 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet, packet->append(STRING_WITH_LEN(")")); } - if (field->comment.length) - { - packet->append(STRING_WITH_LEN(" COMMENT ")); - append_unescaped(packet, field->comment.str, field->comment.length); - } - append_create_options(thd, packet, field->option_list, check_options, - hton->field_options); } key_info= table->key_info; -- cgit v1.2.1 From d46ffaf6afdcfc5b9241d22b176000e9e9936477 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 19 Dec 2018 16:48:07 +0300 Subject: MDEV-17655 Inconsistent grant-name usage between grant-statement and privilege tables Closes #1044 --- sql/sql_acl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 4ffb2c8b8f1..ed200bba763 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -8437,13 +8437,13 @@ static const char *command_array[]= "LOCK TABLES", "EXECUTE", "REPLICATION SLAVE", "REPLICATION CLIENT", "CREATE VIEW", "SHOW VIEW", "CREATE ROUTINE", "ALTER ROUTINE", "CREATE USER", "EVENT", "TRIGGER", "CREATE TABLESPACE", - "DELETE VERSIONING ROWS" + "DELETE HISTORY" }; static uint command_lengths[]= { 6, 6, 6, 6, 6, 4, 6, 8, 7, 4, 5, 10, 5, 5, 14, 5, 23, 11, 7, 17, 18, 11, 9, - 14, 13, 11, 5, 7, 17, 22, + 14, 13, 11, 5, 7, 17, 14, }; -- cgit v1.2.1 From 0d6fb43e6d6ca1eb9060d7369efcbabcda324f1e Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 2 May 2019 16:49:47 +0300 Subject: Fixed some compilation warnings/errors --- sql/item_sum.cc | 2 +- sql/mysqld.cc | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/item_sum.cc b/sql/item_sum.cc index eb6d71cf99b..4e449198fe0 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -2046,7 +2046,7 @@ double Item_sum_std::val_real() { DBUG_ASSERT(fixed == 1); double nr= Item_sum_variance::val_real(); - if (isnan(nr)) + if (std::isnan(nr)) { /* variance_fp_recurrence_next() can overflow in some cases and return "nan": diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 212ab8e2a13..f0ffa7eae8c 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3043,9 +3043,8 @@ static bool cache_thread(THD *thd) Create new instrumentation for the new THD job, and attach it to this running pthread. */ - PSI_thread *psi= PSI_CALL_new_thread(key_thread_one_connection, - thd, thd->thread_id); - PSI_CALL_set_thread(psi); + PSI_CALL_set_thread(PSI_CALL_new_thread(key_thread_one_connection, + thd, thd->thread_id)); /* reset abort flag for the thread */ thd->mysys_var->abort= 0; -- cgit v1.2.1 From 879878e43d5ffd2bf4d18ffe4d0186f8926e58ca Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Thu, 2 May 2019 14:38:43 +0530 Subject: MDEV-18943: Group Concat with limit not working with views Adjusted the Item_func_group_concat::print function to take into account limit if present with GROUP_CONCAT --- sql/item_sum.cc | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 4e449198fe0..ce0c9d3e944 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -4172,7 +4172,19 @@ void Item_func_group_concat::print(String *str, enum_query_type query_type) } str->append(STRING_WITH_LEN(" separator \'")); str->append_for_single_quote(separator->ptr(), separator->length()); - str->append(STRING_WITH_LEN("\')")); + str->append(STRING_WITH_LEN("\'")); + + if (limit_clause) + { + str->append(STRING_WITH_LEN(" limit ")); + if (offset_limit) + { + offset_limit->print(str, query_type); + str->append(','); + } + row_limit->print(str, query_type); + } + str->append(STRING_WITH_LEN(")")); } -- cgit v1.2.1 From 0e91e0c377f38c31cd5ad766b9a6faceacb7ce18 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 13 Mar 2019 14:41:52 +0400 Subject: A proper State_tracker::m_changed enacpsulation Note: preserved original behaviour, where remaining trackers are not reset on error from store(). This effectively means subsequent statements will start tracking from unclean state. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 91 +++++++------------------------------------------- sql/session_tracker.h | 5 +-- 2 files changed, 15 insertions(+), 81 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index b59475645fa..c049e1bab33 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -209,7 +209,6 @@ public: return result; } - void reset(); bool enable(THD *thd); bool check_str(THD *thd, LEX_STRING *val); bool update(THD *thd, set_var *var); @@ -241,7 +240,6 @@ class Current_schema_tracker : public State_tracker { private: bool schema_track_inited; - void reset(); public: @@ -272,17 +270,11 @@ public: class Session_state_change_tracker : public State_tracker { -private: - - void reset(); - public: - Session_state_change_tracker(); bool enable(THD *thd) { return update(thd, NULL); }; bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); - bool is_state_changed(THD*); }; @@ -829,7 +821,7 @@ bool Session_sysvars_tracker::store(THD *thd, String *buf) if (orig_list->store(thd, buf)) return true; - reset(); + orig_list->reset(); return false; } @@ -892,17 +884,6 @@ void Session_sysvars_tracker::vars_list::reset() my_hash_iterate(&m_registered_sysvars, &reset_variable, NULL); } -/** - Prepare/reset the m_registered_sysvars hash for next statement. -*/ - -void Session_sysvars_tracker::reset() -{ - - orig_list->reset(); - m_changed= false; -} - static Session_sysvars_tracker* sysvar_tracker(THD *thd) { return (Session_sysvars_tracker*) @@ -991,24 +972,10 @@ bool Current_schema_tracker::store(THD *thd, String *buf) /* Length and current schema name */ buf->q_net_store_data((const uchar *)thd->db.str, thd->db.length); - reset(); - return false; } -/** - Reset the m_changed flag for next statement. - - @return void -*/ - -void Current_schema_tracker::reset() -{ - m_changed= false; -} - - /////////////////////////////////////////////////////////////////////////////// @@ -1331,24 +1298,13 @@ bool Transaction_state_tracker::store(THD *thd, String *buf) } } - reset(); + tx_reported_state= tx_curr_state; + tx_changed= TX_CHG_NONE; return false; } -/** - Reset the m_changed flag for next statement. -*/ - -void Transaction_state_tracker::reset() -{ - m_changed= false; - tx_reported_state= tx_curr_state; - tx_changed= TX_CHG_NONE; -} - - /** Helper function: turn table info into table access flag. Accepts table lock type and engine type flag (transactional/ @@ -1518,11 +1474,6 @@ void Transaction_state_tracker::set_isol_level(THD *thd, /////////////////////////////////////////////////////////////////////////////// -Session_state_change_tracker::Session_state_change_tracker() -{ - m_changed= false; -} - /** @Enable/disable the tracker based on @@session_track_state_change value. @@ -1560,34 +1511,12 @@ bool Session_state_change_tracker::store(THD *thd, String *buf) /* Length of the overall entity (1 byte) */ buf->q_append('\1'); - DBUG_ASSERT(is_state_changed(thd)); + DBUG_ASSERT(is_changed()); buf->q_append('1'); - reset(); - return false; } - -/** - Reset the m_changed flag for next statement. -*/ - -void Session_state_change_tracker::reset() -{ - m_changed= false; -} - - -/** - Find if there is a session state change. -*/ - -bool Session_state_change_tracker::is_state_changed(THD *) -{ - return m_changed; -} - /////////////////////////////////////////////////////////////////////////////// /** @@ -1682,11 +1611,15 @@ void Session_tracker::store(THD *thd, String *buf) /* Get total length. */ for (int i= 0; i < SESSION_TRACKER_END; i++) { - if (m_trackers[i]->is_changed() && - m_trackers[i]->store(thd, buf)) + if (m_trackers[i]->is_changed()) { - buf->length(start); // it is safer to have 0-length block in case of error - return; + if (m_trackers[i]->store(thd, buf)) + { + // it is safer to have 0-length block in case of error + buf->length(start); + return; + } + m_trackers[i]->reset_changed(); } } diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 684692aae0c..2452183acd4 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -67,6 +67,7 @@ protected: */ bool m_enabled; +private: /** Has the session state type changed ? */ bool m_changed; @@ -86,6 +87,8 @@ public: bool is_changed() const { return m_changed; } + void reset_changed() { m_changed= false; } + /** Called in the constructor of THD*/ virtual bool enable(THD *thd)= 0; @@ -276,8 +279,6 @@ private: /** isolation level */ enum enum_tx_isol_level tx_isol_level; - void reset(); - inline void update_change_flags(THD *thd) { tx_changed &= uint(~TX_CHG_STATE); -- cgit v1.2.1 From 8f594b3384f5c68f530730a037a7e74fa215b67d Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Mon, 18 Mar 2019 14:43:14 +0400 Subject: Session_sysvars_tracker::vars_list cleanups - return proper type - removed useless node argument - removed useless mem_flag - classes are "private" by default - simplified iterators Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 142 +++++++++++++++++-------------------------------- 1 file changed, 50 insertions(+), 92 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index c049e1bab33..04a8b783837 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -70,7 +70,6 @@ private: class vars_list { - private: /** Registered system variables. (@@session_track_system_variables) A hash to store the name of all the system variables specified by the @@ -101,12 +100,20 @@ private: } } - uchar* search(const sys_var *svar) + sysvar_node_st *search(const sys_var *svar) { - return (my_hash_search(&m_registered_sysvars, (const uchar *)&svar, - sizeof(sys_var *))); + return reinterpret_cast( + my_hash_search(&m_registered_sysvars, + reinterpret_cast(&svar), + sizeof(sys_var*))); } + sysvar_node_st *at(ulong i) + { + DBUG_ASSERT(i < m_registered_sysvars.records); + return reinterpret_cast( + my_hash_element(&m_registered_sysvars, i)); + } public: vars_list() : buffer_length(0) @@ -129,22 +136,21 @@ private: } } - uchar* insert_or_search(sysvar_node_st *node, const sys_var *svar) + sysvar_node_st *insert_or_search(const sys_var *svar) { - uchar *res; - res= search(svar); + sysvar_node_st *res= search(svar); if (!res) { if (track_all) { - insert(node, svar, m_mem_flag); + insert(svar); return search(svar); } } return res; } - bool insert(sysvar_node_st *node, const sys_var *svar, myf mem_flag); + bool insert(const sys_var *svar); void reinit(); void reset(); inline bool is_enabled() @@ -218,11 +224,6 @@ public: static uchar *sysvars_get_key(const char *entry, size_t *length, my_bool not_used __attribute__((unused))); - // hash iterators - static my_bool name_array_filler(void *ptr, void *data_ptr); - static my_bool store_variable(void *ptr, void *data_ptr); - static my_bool reset_variable(void *ptr, void *data_ptr); - static bool check_var_list(THD *thd, LEX_STRING var_list, bool throw_error, CHARSET_INFO *char_set, bool take_mutex); }; @@ -313,25 +314,20 @@ void Session_sysvars_tracker::vars_list::copy(vars_list* from, THD *thd) /** Inserts the variable to be tracked into m_registered_sysvars hash. - @param node Node to be inserted. @param svar address of the system variable @retval false success @retval true error */ -bool Session_sysvars_tracker::vars_list::insert(sysvar_node_st *node, - const sys_var *svar, - myf mem_flag) +bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) { - if (!node) + sysvar_node_st *node; + if (!(node= (sysvar_node_st *) my_malloc(sizeof(sysvar_node_st), + MYF(MY_WME | m_mem_flag)))) { - if (!(node= (sysvar_node_st *) my_malloc(sizeof(sysvar_node_st), - MYF(MY_WME | mem_flag)))) - { - reinit(); - return true; - } + reinit(); + return true; } node->m_svar= (sys_var *)svar; @@ -433,7 +429,7 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, else if ((svar= find_sys_var_ex(thd, var.str, var.length, throw_error, true))) { - if (insert(NULL, svar, m_mem_flag) == TRUE) + if (insert(svar) == TRUE) goto error; } else if (throw_error && thd) @@ -536,24 +532,6 @@ bool Session_sysvars_tracker::check_var_list(THD *thd, return false; } -struct name_array_filler_data -{ - LEX_CSTRING **names; - uint idx; - -}; - -/** Collects variable references into array */ -my_bool Session_sysvars_tracker::name_array_filler(void *ptr, - void *data_ptr) -{ - Session_sysvars_tracker::sysvar_node_st *node= - (Session_sysvars_tracker::sysvar_node_st *)ptr; - name_array_filler_data *data= (struct name_array_filler_data *)data_ptr; - if (*node->test_load) - data->names[data->idx++]= &node->m_svar->name; - return FALSE; -} /* Sorts variable references array */ static int name_array_sorter(const void *a, const void *b) @@ -573,7 +551,8 @@ static int name_array_sorter(const void *a, const void *b) bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, size_t buf_len) { - struct name_array_filler_data data; + LEX_CSTRING **names; + uint idx; size_t left= buf_len; size_t names_size= m_registered_sysvars.records * sizeof(LEX_CSTRING *); const char separator= ','; @@ -596,16 +575,19 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, return false; } - data.names= (LEX_CSTRING**)my_safe_alloca(names_size); - - if (unlikely(!data.names)) + if (unlikely(!(names= (LEX_CSTRING**) my_safe_alloca(names_size)))) return true; - data.idx= 0; + idx= 0; mysql_mutex_lock(&LOCK_plugin); - my_hash_iterate(&m_registered_sysvars, &name_array_filler, &data); - DBUG_ASSERT(data.idx <= m_registered_sysvars.records); + for (ulong i= 0; i < m_registered_sysvars.records; i++) + { + sysvar_node_st *node= at(i); + if (*node->test_load) + names[idx++]= &node->m_svar->name; + } + DBUG_ASSERT(idx <= m_registered_sysvars.records); /* We check number of records again here because number of variables @@ -618,17 +600,16 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, return false; } - my_qsort(data.names, data.idx, sizeof(LEX_CSTRING *), - &name_array_sorter); + my_qsort(names, idx, sizeof(LEX_CSTRING*), &name_array_sorter); - for(uint i= 0; i < data.idx; i++) + for(uint i= 0; i < idx; i++) { - LEX_CSTRING *nm= data.names[i]; + LEX_CSTRING *nm= names[i]; size_t ln= nm->length + 1; if (ln > left) { mysql_mutex_unlock(&LOCK_plugin); - my_safe_afree(data.names, names_size); + my_safe_afree(names, names_size); return true; } memcpy(buf, nm->str, nm->length); @@ -639,7 +620,7 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, mysql_mutex_unlock(&LOCK_plugin); buf--; buf[0]= '\0'; - my_safe_afree(data.names, names_size); + my_safe_afree(names, names_size); return false; } @@ -724,24 +705,15 @@ bool Session_sysvars_tracker::update(THD *thd, set_var *var) } -/* - Function and structure to support storing variables from hash to the buffer. -*/ - -struct st_store_variable_param -{ - THD *thd; - String *buf; -}; - -my_bool Session_sysvars_tracker::store_variable(void *ptr, void *data_ptr) +bool Session_sysvars_tracker::vars_list::store(THD *thd, String *buf) { - Session_sysvars_tracker::sysvar_node_st *node= - (Session_sysvars_tracker::sysvar_node_st *)ptr; - if (node->m_changed) + for (ulong i= 0; i < m_registered_sysvars.records; i++) { - THD *thd= ((st_store_variable_param *)data_ptr)->thd; - String *buf= ((st_store_variable_param *)data_ptr)->buf; + sysvar_node_st *node= at(i); + + if (!node->m_changed) + continue; + char val_buf[SHOW_VAR_FUNC_BUFF_SIZE]; SHOW_VAR show; CHARSET_INFO *charset; @@ -750,7 +722,7 @@ my_bool Session_sysvars_tracker::store_variable(void *ptr, void *data_ptr) if (!*node->test_load) { mysql_mutex_unlock(&LOCK_plugin); - return false; + continue; } sys_var *svar= node->m_svar; bool is_plugin= svar->cast_pluginvar(); @@ -795,11 +767,6 @@ my_bool Session_sysvars_tracker::store_variable(void *ptr, void *data_ptr) return false; } -bool Session_sysvars_tracker::vars_list::store(THD *thd, String *buf) -{ - st_store_variable_param data= {thd, buf}; - return my_hash_iterate(&m_registered_sysvars, &store_variable, &data); -} /** Store the data for changed system variables in the specified buffer. @@ -836,14 +803,13 @@ bool Session_sysvars_tracker::store(THD *thd, String *buf) void Session_sysvars_tracker::mark_as_changed(THD *thd, LEX_CSTRING *var) { - sysvar_node_st *node= NULL; + sysvar_node_st *node; sys_var *svar= (sys_var *)var; /* Check if the specified system variable is being tracked, if so mark it as changed and also set the class's m_changed flag. */ - if (orig_list->is_enabled() && - (node= (sysvar_node_st *) (orig_list->insert_or_search(node, svar)))) + if (orig_list->is_enabled() && (node= orig_list->insert_or_search(svar))) { node->m_changed= true; State_tracker::mark_as_changed(thd, var); @@ -870,18 +836,10 @@ uchar *Session_sysvars_tracker::sysvars_get_key(const char *entry, } -/* Function to support resetting hash nodes for the variables */ - -my_bool Session_sysvars_tracker::reset_variable(void *ptr, - void *data_ptr) -{ - ((Session_sysvars_tracker::sysvar_node_st *)ptr)->m_changed= false; - return false; -} - void Session_sysvars_tracker::vars_list::reset() { - my_hash_iterate(&m_registered_sysvars, &reset_variable, NULL); + for (ulong i= 0; i < m_registered_sysvars.records; i++) + at(i)->m_changed= false; } static Session_sysvars_tracker* sysvar_tracker(THD *thd) -- cgit v1.2.1 From 19d5ddccfde04c6b336bb4974407ecde4fb6fbc6 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 13 Mar 2019 14:16:49 +0400 Subject: Cleanup session tracker redundancy - m_enabled is initialised by the base class constructor - removed unused schema_track_inited - moved Transaction_state_tracker constructor to declaration - common enable() - removed unused Session_sysvars_tracker::check_str() - classes are "private" by default - don't even try to compile for embedded Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 51 -------------------------------------------------- sql/session_tracker.h | 13 +++++++------ 2 files changed, 7 insertions(+), 57 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 04a8b783837..14fc43e09e7 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -15,7 +15,6 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#ifndef EMBEDDED_LIBRARY #include "sql_plugin.h" #include "session_tracker.h" @@ -60,8 +59,6 @@ public: class Session_sysvars_tracker : public State_tracker { -private: - struct sysvar_node_st { sys_var *m_svar; bool *test_load; @@ -216,7 +213,6 @@ public: } bool enable(THD *thd); - bool check_str(THD *thd, LEX_STRING *val); bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); void mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name); @@ -239,18 +235,7 @@ public: class Current_schema_tracker : public State_tracker { -private: - bool schema_track_inited; - public: - - Current_schema_tracker() - { - schema_track_inited= false; - } - - bool enable(THD *thd) - { return update(thd, NULL); } bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); }; @@ -272,8 +257,6 @@ public: class Session_state_change_tracker : public State_tracker { public: - bool enable(THD *thd) - { return update(thd, NULL); }; bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); }; @@ -654,27 +637,6 @@ bool Session_sysvars_tracker::enable(THD *thd) } -/** - Check system variable name(s). - - @note This function is called from the ON_CHECK() function of the - session_track_system_variables' sys_var class. - - @param thd [IN] The thd handle. - @param var [IN] A pointer to set_var holding the specified list of - system variable names. - - @retval true Error - @retval false Success -*/ - -inline bool Session_sysvars_tracker::check_str(THD *thd, LEX_STRING *val) -{ - return Session_sysvars_tracker::check_var_list(thd, *val, true, - thd->charset(), true); -} - - /** Once the value of the @@session_track_system_variables has been successfully updated, this function calls @@ -936,17 +898,6 @@ bool Current_schema_tracker::store(THD *thd, String *buf) /////////////////////////////////////////////////////////////////////////////// - -Transaction_state_tracker::Transaction_state_tracker() -{ - m_enabled = false; - tx_changed = TX_CHG_NONE; - tx_curr_state = - tx_reported_state= TX_EMPTY; - tx_read_flags = TX_READ_INHERIT; - tx_isol_level = TX_ISOL_INHERIT; -} - /** Enable/disable the tracker based on @@session_track_transaction_info. @@ -1597,5 +1548,3 @@ void Session_tracker::store(THD *thd, String *buf) net_store_length(data - 1, length); } - -#endif //EMBEDDED_LIBRARY diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 2452183acd4..1d0f55c252a 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -90,7 +90,7 @@ public: void reset_changed() { m_changed= false; } /** Called in the constructor of THD*/ - virtual bool enable(THD *thd)= 0; + virtual bool enable(THD *thd) { return update(thd, 0); } /** To be invoked when the tracker's system variable is updated (ON_UPDATE).*/ virtual bool update(THD *thd, set_var *var)= 0; @@ -119,7 +119,6 @@ bool sysvartrack_value_construct(THD *thd, char *val, size_t len); class Session_tracker { -private: State_tracker *m_trackers[SESSION_TRACKER_END]; /* The following two functions are private to disable copying. */ @@ -234,14 +233,16 @@ enum enum_session_track_transaction_info { class Transaction_state_tracker : public State_tracker { -private: /** Helper function: turn table info into table access flag */ enum_tx_state calc_trx_state(THD *thd, thr_lock_type l, bool has_trx); public: /** Constructor */ - Transaction_state_tracker(); - bool enable(THD *thd) - { return update(thd, NULL); } + Transaction_state_tracker(): tx_changed(TX_CHG_NONE), + tx_curr_state(TX_EMPTY), + tx_reported_state(TX_EMPTY), + tx_read_flags(TX_READ_INHERIT), + tx_isol_level(TX_ISOL_INHERIT) {} + bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); -- cgit v1.2.1 From 2be28a91b15010c5e6146e78e78fbe10a9b86153 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Fri, 15 Mar 2019 11:52:26 +0400 Subject: Cleanup session tracker API - Session_sysvars_tracker::server_init_check() -> sysvartrack_validate_value() - Session_sysvars_tracker::check_var_list() -> sysvartrack_validate_value() - Session_sysvars_tracker::server_init_process() -> sysvartrack_global_update() - sysvartrack_reprint_value() -> sysvartrack_global_update() - sysvartrack_value_len() -> sysvartrack_session_value_ptr() - sysvartrack_value_construct() -> sysvartrack_session_value_ptr() - sysvartrack_update() -> Session_sysvars_tracker::update() - Session_tracker::server_boot_verify() -> session_tracker_init() - sysvar_tracker() -> /dev/null Part of MDEV-14984 - regression in connect performance --- sql/mysqld.cc | 10 +--- sql/session_tracker.cc | 136 +++++++++++++++++-------------------------------- sql/session_tracker.h | 9 ++-- sql/sys_vars.ic | 18 ++----- 4 files changed, 56 insertions(+), 117 deletions(-) (limited to 'sql') diff --git a/sql/mysqld.cc b/sql/mysqld.cc index f0ffa7eae8c..73dd7ce36af 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -5420,14 +5420,8 @@ static int init_server_components() #endif #ifndef EMBEDDED_LIBRARY - { - if (Session_tracker::server_boot_verify(system_charset_info)) - { - sql_print_error("The variable session_track_system_variables has " - "invalid values."); - unireg_abort(1); - } - } + if (session_tracker_init()) + return 1; #endif //EMBEDDED_LIBRARY /* we do want to exit if there are any other unknown options */ diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 14fc43e09e7..7908c083446 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -190,28 +190,6 @@ public: return orig_list->construct_var_list(buf, buf_len); } - /** - Method used to check the validity of string provided - for session_track_system_variables during the server - startup. - */ - static bool server_init_check(THD *thd, CHARSET_INFO *char_set, - LEX_STRING var_list) - { - return check_var_list(thd, var_list, false, char_set, false); - } - - static bool server_init_process(THD *thd, CHARSET_INFO *char_set, - LEX_STRING var_list) - { - vars_list dummy; - bool result; - result= dummy.parse_var_list(thd, var_list, false, char_set, false); - if (!result) - dummy.construct_var_list(var_list.str, var_list.length + 1); - return result; - } - bool enable(THD *thd); bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); @@ -220,8 +198,7 @@ public: static uchar *sysvars_get_key(const char *entry, size_t *length, my_bool not_used __attribute__((unused))); - static bool check_var_list(THD *thd, LEX_STRING var_list, bool throw_error, - CHARSET_INFO *char_set, bool take_mutex); + friend bool sysvartrack_global_update(THD *thd, char *str, size_t len); }; @@ -442,12 +419,9 @@ error: } -bool Session_sysvars_tracker::check_var_list(THD *thd, - LEX_STRING var_list, - bool throw_error, - CHARSET_INFO *char_set, - bool take_mutex) +bool sysvartrack_validate_value(THD *thd, const char *str, size_t len) { + LEX_STRING var_list= { (char *) str, len }; const char separator= ','; char *token, *lasts= NULL; size_t rest= var_list.length; @@ -466,7 +440,7 @@ bool Session_sysvars_tracker::check_var_list(THD *thd, token value. Hence the mutex is handled here to avoid a performance overhead. */ - if (!thd || take_mutex) + if (!thd) mysql_mutex_lock(&LOCK_plugin); for (;;) { @@ -484,24 +458,14 @@ bool Session_sysvars_tracker::check_var_list(THD *thd, var.length= rest; /* Remove leading/trailing whitespace. */ - trim_whitespace(char_set, &var); + trim_whitespace(system_charset_info, &var); if(!strcmp(var.str, "*") && - !find_sys_var_ex(thd, var.str, var.length, throw_error, true)) + !find_sys_var_ex(thd, var.str, var.length, false, true)) { - if (throw_error && take_mutex && thd) - { - push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, - ER_WRONG_VALUE_FOR_VAR, - "%.*s is not a valid system variable and will" - "be ignored.", (int)var.length, token); - } - else - { - if (!thd || take_mutex) - mysql_mutex_unlock(&LOCK_plugin); - return true; - } + if (!thd) + mysql_mutex_unlock(&LOCK_plugin); + return true; } if (lasts) @@ -509,7 +473,7 @@ bool Session_sysvars_tracker::check_var_list(THD *thd, else break; } - if (!thd || take_mutex) + if (!thd) mysql_mutex_unlock(&LOCK_plugin); return false; @@ -804,38 +768,50 @@ void Session_sysvars_tracker::vars_list::reset() at(i)->m_changed= false; } -static Session_sysvars_tracker* sysvar_tracker(THD *thd) -{ - return (Session_sysvars_tracker*) - thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER); -} -bool sysvartrack_validate_value(THD *thd, const char *str, size_t len) -{ - LEX_STRING tmp= {(char *)str, len}; - return Session_sysvars_tracker::server_init_check(thd, system_charset_info, - tmp); -} -bool sysvartrack_reprint_value(THD *thd, char *str, size_t len) +bool sysvartrack_global_update(THD *thd, char *str, size_t len) { - LEX_STRING tmp= {str, len}; - return Session_sysvars_tracker::server_init_process(thd, - system_charset_info, - tmp); -} -bool sysvartrack_update(THD *thd, set_var *var) -{ - return sysvar_tracker(thd)->update(thd, var); + LEX_STRING tmp= { str, len }; + Session_sysvars_tracker::vars_list dummy; + if (!dummy.parse_var_list(thd, tmp, false, system_charset_info, false)) + { + dummy.construct_var_list(str, len + 1); + return false; + } + return true; } -size_t sysvartrack_value_len(THD *thd) + + +uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base) { - return sysvar_tracker(thd)->get_buffer_length(); + Session_sysvars_tracker *tracker= static_cast + (thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)); + size_t len= tracker->get_buffer_length(); + char *res= (char*) thd->alloc(len + sizeof(char*)); + if (res) + { + char *buf= res + sizeof(char*); + *((char**) res)= buf; + tracker->construct_var_list(buf, len); + } + return (uchar*) res; } -bool sysvartrack_value_construct(THD *thd, char *val, size_t len) + + +int session_tracker_init() { - return sysvar_tracker(thd)->construct_var_list(val, len); + if (sysvartrack_validate_value(0, + global_system_variables.session_track_system_variables, + safe_strlen(global_system_variables.session_track_system_variables))) + { + sql_print_error("The variable session_track_system_variables has " + "invalid values."); + return 1; + } + return 0; } + /////////////////////////////////////////////////////////////////////////////// /** @@ -1477,26 +1453,6 @@ void Session_tracker::enable(THD *thd) } -/** - Method called during the server startup to verify the contents - of @@session_track_system_variables. - - @retval false Success - @retval true Failure -*/ - -bool Session_tracker::server_boot_verify(CHARSET_INFO *char_set) -{ - bool result; - LEX_STRING tmp; - tmp.str= global_system_variables.session_track_system_variables; - tmp.length= safe_strlen(tmp.str); - result= - Session_sysvars_tracker::server_init_check(NULL, char_set, tmp); - return result; -} - - /** @brief Store all change information in the specified buffer. diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 1d0f55c252a..8edc3ce2a39 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -103,10 +103,8 @@ public: }; bool sysvartrack_validate_value(THD *thd, const char *str, size_t len); -bool sysvartrack_reprint_value(THD *thd, char *str, size_t len); -bool sysvartrack_update(THD *thd, set_var *var); -size_t sysvartrack_value_len(THD *thd); -bool sysvartrack_value_construct(THD *thd, char *val, size_t len); +bool sysvartrack_global_update(THD *thd, char *str, size_t len); +uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base); /** @@ -152,7 +150,6 @@ public: } void enable(THD *thd); - static bool server_boot_verify(CHARSET_INFO *char_set); /** Returns the pointer to the tracker object for the specified tracker. */ inline State_tracker *get_tracker(enum_session_tracker tracker) const @@ -296,6 +293,8 @@ private: ->X; } } while(0) #define SESSION_TRACKER_CHANGED(A,B,C) \ thd->session_tracker.mark_as_changed(A,B,C) + +int session_tracker_init(); #else #define TRANSACT_TRACKER(X) do{}while(0) diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index 373df354268..7c375f16cdd 100644 --- a/sql/sys_vars.ic +++ b/sql/sys_vars.ic @@ -615,7 +615,7 @@ public: char *new_val= global_update_prepare(thd, var); if (new_val) { - if (sysvartrack_reprint_value(thd, new_val, + if (sysvartrack_global_update(thd, new_val, var->save_result.string_value.length)) new_val= 0; } @@ -624,7 +624,8 @@ public: } bool session_update(THD *thd, set_var *var) { - return sysvartrack_update(thd, var); + return thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> + update(thd, var); } void session_save_default(THD *thd, set_var *var) { @@ -644,18 +645,7 @@ public: } } uchar *session_value_ptr(THD *thd, const LEX_CSTRING *base) - { - DBUG_ASSERT(thd != NULL); - size_t len= sysvartrack_value_len(thd); - char *res= (char *)thd->alloc(len + sizeof(char *)); - if (res) - { - char *buf= res + sizeof(char *); - *((char**) res)= buf; - sysvartrack_value_construct(thd, buf, len); - } - return (uchar *)res; - } + { return sysvartrack_session_value_ptr(thd, base); } }; #endif //EMBEDDED_LIBRARY -- cgit v1.2.1 From 55bdd7f7b4a0898dca2a2e7d457b06cb81df7d6a Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 13 Mar 2019 12:20:11 +0400 Subject: Get rid of not implemented SESSION_GTIDS_TRACKER One less new/delete per connection. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 16 ---------------- sql/session_tracker.h | 1 - 2 files changed, 17 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 7908c083446..13da65b417e 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -34,20 +34,6 @@ void State_tracker::mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name) } -class Not_implemented_tracker : public State_tracker -{ -public: - bool enable(THD *thd) - { return false; } - bool update(THD *, set_var *) - { return false; } - bool store(THD *, String *) - { return false; } - void mark_as_changed(THD *, LEX_CSTRING *tracked_item_name) - {} - -}; - /** Session_sysvars_tracker @@ -1443,8 +1429,6 @@ void Session_tracker::enable(THD *thd) new (std::nothrow) Current_schema_tracker; m_trackers[SESSION_STATE_CHANGE_TRACKER]= new (std::nothrow) Session_state_change_tracker; - m_trackers[SESSION_GTIDS_TRACKER]= - new (std::nothrow) Not_implemented_tracker; m_trackers[TRANSACTION_INFO_TRACKER]= new (std::nothrow) Transaction_state_tracker; diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 8edc3ce2a39..8265e638f1b 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -32,7 +32,6 @@ enum enum_session_tracker SESSION_SYSVARS_TRACKER, /* Session system variables */ CURRENT_SCHEMA_TRACKER, /* Current schema */ SESSION_STATE_CHANGE_TRACKER, - SESSION_GTIDS_TRACKER, /* Tracks GTIDs */ TRANSACTION_INFO_TRACKER, /* Transaction state */ SESSION_TRACKER_END /* must be the last */ }; -- cgit v1.2.1 From 01e8f3c52b349a0374c04f0765a625b5ac03a652 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 13 Mar 2019 13:41:18 +0400 Subject: Allocate orig_list statically tool_list is a temporary list needed only for SET SESSION session_track_system_variables, so allocate it on stack instead. Sane reinit() calls. Saves 2 new/delete per connection. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 57 +++++++++++++++----------------------------------- 1 file changed, 17 insertions(+), 40 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 13da65b417e..1af1c8de05a 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -98,8 +98,7 @@ class Session_sysvars_tracker : public State_tracker my_hash_element(&m_registered_sysvars, i)); } public: - vars_list() : - buffer_length(0) + vars_list(): buffer_length(0), track_all(false) { m_mem_flag= current_thd ? MY_THREAD_SPECIFIC : 0; init(); @@ -150,30 +149,16 @@ class Session_sysvars_tracker : public State_tracker Two objects of vars_list type are maintained to manage various operations. */ - vars_list *orig_list, *tool_list; + vars_list orig_list; public: - Session_sysvars_tracker() - { - orig_list= new (std::nothrow) vars_list(); - tool_list= new (std::nothrow) vars_list(); - } - - ~Session_sysvars_tracker() - { - if (orig_list) - delete orig_list; - if (tool_list) - delete tool_list; - } - size_t get_buffer_length() { - return orig_list->get_buffer_length(); + return orig_list.get_buffer_length(); } bool construct_var_list(char *buf, size_t buf_len) { - return orig_list->construct_var_list(buf, buf_len); + return orig_list.construct_var_list(buf, buf_len); } bool enable(THD *thd); @@ -249,7 +234,6 @@ void Session_sysvars_tracker::vars_list::reinit() void Session_sysvars_tracker::vars_list::copy(vars_list* from, THD *thd) { - reinit(); track_all= from->track_all; free_hash(); buffer_length= from->buffer_length; @@ -271,10 +255,7 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) sysvar_node_st *node; if (!(node= (sysvar_node_st *) my_malloc(sizeof(sysvar_node_st), MYF(MY_WME | m_mem_flag)))) - { - reinit(); return true; - } node->m_svar= (sys_var *)svar; node->test_load= node->m_svar->test_load; @@ -285,7 +266,6 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) if (!search((sys_var *)svar)) { //EOF (error is already reported) - reinit(); return true; } } @@ -322,7 +302,6 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, const char separator= ','; char *token, *lasts= NULL; size_t rest= var_list.length; - reinit(); if (!var_list.str || var_list.length == 0) { @@ -573,14 +552,13 @@ bool Session_sysvars_tracker::enable(THD *thd) LEX_STRING tmp; tmp.str= global_system_variables.session_track_system_variables; tmp.length= safe_strlen(tmp.str); - if (tool_list->parse_var_list(thd, tmp, - true, thd->charset(), false) == true) + if (orig_list.parse_var_list(thd, tmp, true, thd->charset(), false) == true) { mysql_mutex_unlock(&LOCK_plugin); + orig_list.reinit(); return true; } mysql_mutex_unlock(&LOCK_plugin); - orig_list->copy(tool_list, thd); m_enabled= true; return false; @@ -593,6 +571,9 @@ bool Session_sysvars_tracker::enable(THD *thd) Session_sysvars_tracker::vars_list::copy updating the hash in orig_list which represents the system variables to be tracked. + We are doing via tool list because there possible errors with memory + in this case value will be unchanged. + @note This function is called from the ON_UPDATE() function of the session_track_system_variables' sys_var class. @@ -604,15 +585,11 @@ bool Session_sysvars_tracker::enable(THD *thd) bool Session_sysvars_tracker::update(THD *thd, set_var *var) { - /* - We are doing via tool list because there possible errors with memory - in this case value will be unchanged. - */ - tool_list->reinit(); - if (tool_list->parse_var_list(thd, var->save_result.string_value, true, - thd->charset(), true)) + vars_list tool_list; + if (tool_list.parse_var_list(thd, var->save_result.string_value, true, + thd->charset(), true)) return true; - orig_list->copy(tool_list, thd); + orig_list.copy(&tool_list, thd); return false; } @@ -694,13 +671,13 @@ bool Session_sysvars_tracker::vars_list::store(THD *thd, String *buf) bool Session_sysvars_tracker::store(THD *thd, String *buf) { - if (!orig_list->is_enabled()) + if (!orig_list.is_enabled()) return false; - if (orig_list->store(thd, buf)) + if (orig_list.store(thd, buf)) return true; - orig_list->reset(); + orig_list.reset(); return false; } @@ -721,7 +698,7 @@ void Session_sysvars_tracker::mark_as_changed(THD *thd, Check if the specified system variable is being tracked, if so mark it as changed and also set the class's m_changed flag. */ - if (orig_list->is_enabled() && (node= orig_list->insert_or_search(svar))) + if (orig_list.is_enabled() && (node= orig_list.insert_or_search(svar))) { node->m_changed= true; State_tracker::mark_as_changed(thd, var); -- cgit v1.2.1 From 47bd06d55ec211bc4d05d616a0833629504c7edf Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Mon, 18 Mar 2019 17:13:00 +0400 Subject: Static current schema and state change trackers Saves 2 new/delete per connection. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 47 +++------------------------------- sql/session_tracker.h | 69 +++++++++++++++++++++++++++++++++++++++++--------- sql/sys_vars.cc | 6 ++--- 3 files changed, 63 insertions(+), 59 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 1af1c8de05a..868e8295294 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -173,43 +173,6 @@ public: }; - -/** - Current_schema_tracker, - - This is a tracker class that enables & manages the tracking of current - schema for a particular connection. -*/ - -class Current_schema_tracker : public State_tracker -{ -public: - bool update(THD *thd, set_var *var); - bool store(THD *thd, String *buf); -}; - -/* - Session_state_change_tracker - - This is a boolean tracker class that will monitor any change that contributes - to a session state change. - Attributes that contribute to session state change include: - - Successful change to System variables - - User defined variables assignments - - temporary tables created, altered or deleted - - prepared statements added or removed - - change in current database - - change of current role -*/ - -class Session_state_change_tracker : public State_tracker -{ -public: - bool update(THD *thd, set_var *var); - bool store(THD *thd, String *buf); -}; - - /* To be used in expanding the buffer. */ static const unsigned int EXTRA_ALLOC= 1024; @@ -1379,8 +1342,10 @@ Session_tracker::Session_tracker() compile_time_assert((uint)SESSION_TRACK_always_at_the_end >= (uint)SESSION_TRACKER_END); - for (int i= 0; i < SESSION_TRACKER_END; i++) - m_trackers[i]= NULL; + m_trackers[SESSION_SYSVARS_TRACKER]= 0; + m_trackers[CURRENT_SCHEMA_TRACKER]= ¤t_schema; + m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change; + m_trackers[TRANSACTION_INFO_TRACKER]= 0; } @@ -1402,10 +1367,6 @@ void Session_tracker::enable(THD *thd) deinit(); m_trackers[SESSION_SYSVARS_TRACKER]= new (std::nothrow) Session_sysvars_tracker(); - m_trackers[CURRENT_SCHEMA_TRACKER]= - new (std::nothrow) Current_schema_tracker; - m_trackers[SESSION_STATE_CHANGE_TRACKER]= - new (std::nothrow) Session_state_change_tracker; m_trackers[TRANSACTION_INFO_TRACKER]= new (std::nothrow) Transaction_state_tracker; diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 8265e638f1b..ec6d21bd344 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -88,8 +88,18 @@ public: void reset_changed() { m_changed= false; } - /** Called in the constructor of THD*/ - virtual bool enable(THD *thd) { return update(thd, 0); } + /** + Called by THD::init() when new connection is being created + + We may inherit m_changed from previous connection served by this THD if + connection was broken or client didn't have session tracking capability. + Thus we have to reset it here. + */ + virtual bool enable(THD *thd) + { + reset_changed(); + return update(thd, 0); + } /** To be invoked when the tracker's system variable is updated (ON_UPDATE).*/ virtual bool update(THD *thd, set_var *var)= 0; @@ -101,11 +111,49 @@ public: virtual void mark_as_changed(THD *thd, LEX_CSTRING *name); }; + bool sysvartrack_validate_value(THD *thd, const char *str, size_t len); bool sysvartrack_global_update(THD *thd, char *str, size_t len); uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base); +/** + Current_schema_tracker, + + This is a tracker class that enables & manages the tracking of current + schema for a particular connection. +*/ + +class Current_schema_tracker: public State_tracker +{ +public: + bool update(THD *thd, set_var *var); + bool store(THD *thd, String *buf); +}; + + +/* + Session_state_change_tracker + + This is a boolean tracker class that will monitor any change that contributes + to a session state change. + Attributes that contribute to session state change include: + - Successful change to System variables + - User defined variables assignments + - temporary tables created, altered or deleted + - prepared statements added or removed + - change in current database + - change of current role +*/ + +class Session_state_change_tracker: public State_tracker +{ +public: + bool update(THD *thd, set_var *var); + bool store(THD *thd, String *buf); +}; + + /** Session_tracker @@ -130,22 +178,19 @@ class Session_tracker } public: + Current_schema_tracker current_schema; + Session_state_change_tracker state_change; Session_tracker(); - ~Session_tracker() - { - deinit(); - } + ~Session_tracker() { deinit(); } /* trick to make happy memory accounting system */ void deinit() { - for (int i= 0; i < SESSION_TRACKER_END; i++) - { - if (m_trackers[i]) - delete m_trackers[i]; - m_trackers[i]= NULL; - } + delete m_trackers[SESSION_SYSVARS_TRACKER]; + m_trackers[SESSION_SYSVARS_TRACKER]= 0; + delete m_trackers[TRANSACTION_INFO_TRACKER]; + m_trackers[TRANSACTION_INFO_TRACKER]= 0; } void enable(THD *thd); diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index ac245d1ee54..bea46578148 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -6128,8 +6128,7 @@ static bool update_session_track_schema(sys_var *self, THD *thd, enum_var_type type) { DBUG_ENTER("update_session_track_schema"); - DBUG_RETURN(thd->session_tracker.get_tracker(CURRENT_SCHEMA_TRACKER)-> - update(thd, NULL)); + DBUG_RETURN(thd->session_tracker.current_schema.update(thd, NULL)); } static Sys_var_mybool Sys_session_track_schema( @@ -6172,8 +6171,7 @@ static bool update_session_track_state_change(sys_var *self, THD *thd, enum_var_type type) { DBUG_ENTER("update_session_track_state_change"); - DBUG_RETURN(thd->session_tracker.get_tracker(SESSION_STATE_CHANGE_TRACKER)-> - update(thd, NULL)); + DBUG_RETURN(thd->session_tracker.state_change.update(thd, NULL)); } static Sys_var_mybool Sys_session_track_state_change( -- cgit v1.2.1 From a7adc2ce1680f00635b8241202066fd5542d286f Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Mon, 18 Mar 2019 19:18:54 +0400 Subject: Allocate Transaction_state_tracker statically One less new/delete per connection. Part of MDEV-14984 - regression in connect performance --- sql/lock.cc | 11 ++-- sql/session_tracker.cc | 4 +- sql/session_tracker.h | 139 +++++++++++++++++++++++++------------------------ sql/sys_vars.cc | 11 ++-- sql/sys_vars.ic | 18 ++----- sql/transaction.cc | 37 ++++--------- 6 files changed, 94 insertions(+), 126 deletions(-) (limited to 'sql') diff --git a/sql/lock.cc b/sql/lock.cc index 5420e9f42b5..aeba6cc7504 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -252,16 +252,11 @@ static void track_table_access(THD *thd, TABLE **tables, size_t count) { if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) { - Transaction_state_tracker *tst= (Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER); - while (count--) { - TABLE *t= tables[count]; - - if (t) - tst->add_trx_state(thd, t->reginfo.lock_type, - t->file->has_transaction_manager()); + if (TABLE *t= tables[count]) + thd->session_tracker.transaction_info.add_trx_state(thd, + t->reginfo.lock_type, t->file->has_transaction_manager()); } } } diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 868e8295294..2f72b7198f9 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -1345,7 +1345,7 @@ Session_tracker::Session_tracker() m_trackers[SESSION_SYSVARS_TRACKER]= 0; m_trackers[CURRENT_SCHEMA_TRACKER]= ¤t_schema; m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change; - m_trackers[TRANSACTION_INFO_TRACKER]= 0; + m_trackers[TRANSACTION_INFO_TRACKER]= &transaction_info; } @@ -1367,8 +1367,6 @@ void Session_tracker::enable(THD *thd) deinit(); m_trackers[SESSION_SYSVARS_TRACKER]= new (std::nothrow) Session_sysvars_tracker(); - m_trackers[TRANSACTION_INFO_TRACKER]= - new (std::nothrow) Transaction_state_tracker; for (int i= 0; i < SESSION_TRACKER_END; i++) m_trackers[i]->enable(thd); diff --git a/sql/session_tracker.h b/sql/session_tracker.h index ec6d21bd344..faa34e88c74 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -154,65 +154,6 @@ public: }; -/** - Session_tracker - - This class holds an object each for all tracker classes and provides - methods necessary for systematic detection and generation of session - state change information. -*/ - -class Session_tracker -{ - State_tracker *m_trackers[SESSION_TRACKER_END]; - - /* The following two functions are private to disable copying. */ - Session_tracker(Session_tracker const &other) - { - DBUG_ASSERT(FALSE); - } - Session_tracker& operator= (Session_tracker const &rhs) - { - DBUG_ASSERT(FALSE); - return *this; - } - -public: - Current_schema_tracker current_schema; - Session_state_change_tracker state_change; - - Session_tracker(); - ~Session_tracker() { deinit(); } - - /* trick to make happy memory accounting system */ - void deinit() - { - delete m_trackers[SESSION_SYSVARS_TRACKER]; - m_trackers[SESSION_SYSVARS_TRACKER]= 0; - delete m_trackers[TRANSACTION_INFO_TRACKER]; - m_trackers[TRANSACTION_INFO_TRACKER]= 0; - } - - void enable(THD *thd); - - /** Returns the pointer to the tracker object for the specified tracker. */ - inline State_tracker *get_tracker(enum_session_tracker tracker) const - { - return m_trackers[tracker]; - } - - inline void mark_as_changed(THD *thd, enum enum_session_tracker tracker, - LEX_CSTRING *data) - { - if (m_trackers[tracker]->is_enabled()) - m_trackers[tracker]->mark_as_changed(thd, data); - } - - - void store(THD *thd, String *main_buf); -}; - - /* Transaction_state_tracker */ @@ -277,12 +218,17 @@ class Transaction_state_tracker : public State_tracker /** Helper function: turn table info into table access flag */ enum_tx_state calc_trx_state(THD *thd, thr_lock_type l, bool has_trx); public: - /** Constructor */ - Transaction_state_tracker(): tx_changed(TX_CHG_NONE), - tx_curr_state(TX_EMPTY), - tx_reported_state(TX_EMPTY), - tx_read_flags(TX_READ_INHERIT), - tx_isol_level(TX_ISOL_INHERIT) {} + + bool enable(THD *thd) + { + m_enabled= false; + tx_changed= TX_CHG_NONE; + tx_curr_state= TX_EMPTY; + tx_reported_state= TX_EMPTY; + tx_read_flags= TX_READ_INHERIT; + tx_isol_level= TX_ISOL_INHERIT; + return State_tracker::enable(thd); + } bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); @@ -332,12 +278,69 @@ private: #define TRANSACT_TRACKER(X) \ do { if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) \ - {((Transaction_state_tracker *) \ - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER)) \ - ->X; } } while(0) + thd->session_tracker.transaction_info.X; } while(0) #define SESSION_TRACKER_CHANGED(A,B,C) \ thd->session_tracker.mark_as_changed(A,B,C) + +/** + Session_tracker + + This class holds an object each for all tracker classes and provides + methods necessary for systematic detection and generation of session + state change information. +*/ + +class Session_tracker +{ + State_tracker *m_trackers[SESSION_TRACKER_END]; + + /* The following two functions are private to disable copying. */ + Session_tracker(Session_tracker const &other) + { + DBUG_ASSERT(FALSE); + } + Session_tracker& operator= (Session_tracker const &rhs) + { + DBUG_ASSERT(FALSE); + return *this; + } + +public: + Current_schema_tracker current_schema; + Session_state_change_tracker state_change; + Transaction_state_tracker transaction_info; + + Session_tracker(); + ~Session_tracker() { deinit(); } + + /* trick to make happy memory accounting system */ + void deinit() + { + delete m_trackers[SESSION_SYSVARS_TRACKER]; + m_trackers[SESSION_SYSVARS_TRACKER]= 0; + } + + void enable(THD *thd); + + /** Returns the pointer to the tracker object for the specified tracker. */ + inline State_tracker *get_tracker(enum_session_tracker tracker) const + { + return m_trackers[tracker]; + } + + inline void mark_as_changed(THD *thd, enum enum_session_tracker tracker, + LEX_CSTRING *data) + { + if (m_trackers[tracker]->is_enabled()) + m_trackers[tracker]->mark_as_changed(thd, data); + } + + + void store(THD *thd, String *main_buf); +}; + + int session_tracker_init(); #else diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index bea46578148..004fd4baecb 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3743,14 +3743,12 @@ bool Sys_var_tx_read_only::session_update(THD *thd, set_var *var) #ifndef EMBEDDED_LIBRARY if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) { - Transaction_state_tracker *tst= (Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER); - if (var->type == OPT_DEFAULT) - tst->set_read_flags(thd, + thd->session_tracker.transaction_info.set_read_flags(thd, thd->tx_read_only ? TX_READ_ONLY : TX_READ_WRITE); else - tst->set_read_flags(thd, TX_READ_INHERIT); + thd->session_tracker.transaction_info.set_read_flags(thd, + TX_READ_INHERIT); } #endif //EMBEDDED_LIBRARY } @@ -6145,8 +6143,7 @@ static bool update_session_track_tx_info(sys_var *self, THD *thd, enum_var_type type) { DBUG_ENTER("update_session_track_tx_info"); - DBUG_RETURN(thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER)-> - update(thd, NULL)); + DBUG_RETURN(thd->session_tracker.transaction_info.update(thd, NULL)); } static const char *session_track_transaction_info_names[]= diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index 7c375f16cdd..042b7c75da7 100644 --- a/sql/sys_vars.ic +++ b/sql/sys_vars.ic @@ -2219,14 +2219,6 @@ public: return TRUE; if (var->type == OPT_DEFAULT || !thd->in_active_multi_stmt_transaction()) { -#ifndef EMBEDDED_LIBRARY - Transaction_state_tracker *tst= NULL; - - if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) - tst= (Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER); -#endif //EMBEDDED_LIBRARY - thd->tx_isolation= (enum_tx_isolation) var->save_result.ulonglong_value; #ifndef EMBEDDED_LIBRARY @@ -2250,13 +2242,11 @@ public: DBUG_ASSERT(0); return TRUE; } - if (tst) - tst->set_isol_level(thd, l); - } - else if (tst) - { - tst->set_isol_level(thd, TX_ISOL_INHERIT); + if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.set_isol_level(thd, l); } + else if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.set_isol_level(thd, TX_ISOL_INHERIT); #endif //EMBEDDED_LIBRARY } return FALSE; diff --git a/sql/transaction.cc b/sql/transaction.cc index 1c2820200d1..74f4eda881b 100644 --- a/sql/transaction.cc +++ b/sql/transaction.cc @@ -32,10 +32,7 @@ static void trans_track_end_trx(THD *thd) { if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) - { - ((Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER))->end_trx(thd); - } + thd->session_tracker.transaction_info.end_trx(thd); } #else #define trans_track_end_trx(A) do{}while(0) @@ -51,11 +48,8 @@ void trans_reset_one_shot_chistics(THD *thd) #ifndef EMBEDDED_LIBRARY if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) { - Transaction_state_tracker *tst= (Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER); - - tst->set_read_flags(thd, TX_READ_INHERIT); - tst->set_isol_level(thd, TX_ISOL_INHERIT); + thd->session_tracker.transaction_info.set_read_flags(thd, TX_READ_INHERIT); + thd->session_tracker.transaction_info.set_isol_level(thd, TX_ISOL_INHERIT); } #endif //EMBEDDED_LIBRARY thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation; @@ -162,20 +156,11 @@ static bool xa_trans_force_rollback(THD *thd) bool trans_begin(THD *thd, uint flags) { int res= FALSE; -#ifndef EMBEDDED_LIBRARY - Transaction_state_tracker *tst= NULL; -#endif //EMBEDDED_LIBRARY DBUG_ENTER("trans_begin"); if (trans_check(thd)) DBUG_RETURN(TRUE); -#ifndef EMBEDDED_LIBRARY - if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) - tst= (Transaction_state_tracker *) - thd->session_tracker.get_tracker(TRANSACTION_INFO_TRACKER); -#endif //EMBEDDED_LIBRARY - thd->locked_tables_list.unlock_locked_tables(thd); DBUG_ASSERT(!thd->locked_tables_mode); @@ -221,8 +206,8 @@ bool trans_begin(THD *thd, uint flags) { thd->tx_read_only= true; #ifndef EMBEDDED_LIBRARY - if (tst) - tst->set_read_flags(thd, TX_READ_ONLY); + if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.set_read_flags(thd, TX_READ_ONLY); #endif //EMBEDDED_LIBRARY } else if (flags & MYSQL_START_TRANS_OPT_READ_WRITE) @@ -246,8 +231,8 @@ bool trans_begin(THD *thd, uint flags) just from the session's default. */ #ifndef EMBEDDED_LIBRARY - if (tst) - tst->set_read_flags(thd, TX_READ_WRITE); + if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.set_read_flags(thd, TX_READ_WRITE); #endif //EMBEDDED_LIBRARY } @@ -264,16 +249,16 @@ bool trans_begin(THD *thd, uint flags) DBUG_PRINT("info", ("setting SERVER_STATUS_IN_TRANS")); #ifndef EMBEDDED_LIBRARY - if (tst) - tst->add_trx_state(thd, TX_EXPLICIT); + if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.add_trx_state(thd, TX_EXPLICIT); #endif //EMBEDDED_LIBRARY /* ha_start_consistent_snapshot() relies on OPTION_BEGIN flag set. */ if (flags & MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT) { #ifndef EMBEDDED_LIBRARY - if (tst) - tst->add_trx_state(thd, TX_WITH_SNAPSHOT); + if (thd->variables.session_track_transaction_info > TX_TRACK_NONE) + thd->session_tracker.transaction_info.add_trx_state(thd, TX_WITH_SNAPSHOT); #endif //EMBEDDED_LIBRARY res= ha_start_consistent_snapshot(thd); } -- cgit v1.2.1 From 554ac6f393941040cea6d45d57a298e900bff193 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Tue, 19 Mar 2019 00:40:26 +0400 Subject: Allocate Session_sysvars_tracker statically One less new/delete per connection. Removed m_mem_flag since most allocs are thread specific. The only exception are allocs performed during initialization. Removed State_tracker and Session_tracker constructors as they don't make sense anymore. No reason to access session_sysvars_tracker via get_tracker(), so access it directly instead. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 196 +++---------------------------------------------- sql/session_tracker.h | 151 +++++++++++++++++++++++++++++++++---- sql/set_var.cc | 8 +- sql/sql_class.cc | 9 +-- sql/sys_vars.ic | 5 +- 5 files changed, 156 insertions(+), 213 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 2f72b7198f9..81da43a0946 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -16,8 +16,6 @@ #include "sql_plugin.h" -#include "session_tracker.h" - #include "hash.h" #include "table.h" #include "rpl_gtid.h" @@ -34,145 +32,6 @@ void State_tracker::mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name) } -/** - Session_sysvars_tracker - - This is a tracker class that enables & manages the tracking of session - system variables. It internally maintains a hash of user supplied variable - references and a boolean field to store if the variable was changed by the - last statement. -*/ - -class Session_sysvars_tracker : public State_tracker -{ - struct sysvar_node_st { - sys_var *m_svar; - bool *test_load; - bool m_changed; - }; - - class vars_list - { - /** - Registered system variables. (@@session_track_system_variables) - A hash to store the name of all the system variables specified by the - user. - */ - HASH m_registered_sysvars; - /** Size of buffer for string representation */ - size_t buffer_length; - myf m_mem_flag; - /** - If TRUE then we want to check all session variable. - */ - bool track_all; - void init() - { - my_hash_init(&m_registered_sysvars, - &my_charset_bin, - 4, 0, 0, (my_hash_get_key) sysvars_get_key, - my_free, MYF(HASH_UNIQUE | - ((m_mem_flag & MY_THREAD_SPECIFIC) ? - HASH_THREAD_SPECIFIC : 0))); - } - void free_hash() - { - if (my_hash_inited(&m_registered_sysvars)) - { - my_hash_free(&m_registered_sysvars); - } - } - - sysvar_node_st *search(const sys_var *svar) - { - return reinterpret_cast( - my_hash_search(&m_registered_sysvars, - reinterpret_cast(&svar), - sizeof(sys_var*))); - } - - sysvar_node_st *at(ulong i) - { - DBUG_ASSERT(i < m_registered_sysvars.records); - return reinterpret_cast( - my_hash_element(&m_registered_sysvars, i)); - } - public: - vars_list(): buffer_length(0), track_all(false) - { - m_mem_flag= current_thd ? MY_THREAD_SPECIFIC : 0; - init(); - } - - size_t get_buffer_length() - { - DBUG_ASSERT(buffer_length != 0); // asked earlier then should - return buffer_length; - } - ~vars_list() - { - /* free the allocated hash. */ - if (my_hash_inited(&m_registered_sysvars)) - { - my_hash_free(&m_registered_sysvars); - } - } - - sysvar_node_st *insert_or_search(const sys_var *svar) - { - sysvar_node_st *res= search(svar); - if (!res) - { - if (track_all) - { - insert(svar); - return search(svar); - } - } - return res; - } - - bool insert(const sys_var *svar); - void reinit(); - void reset(); - inline bool is_enabled() - { - return track_all || m_registered_sysvars.records; - } - void copy(vars_list* from, THD *thd); - bool parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error, - CHARSET_INFO *char_set, bool take_mutex); - bool construct_var_list(char *buf, size_t buf_len); - bool store(THD *thd, String *buf); - }; - /** - Two objects of vars_list type are maintained to manage - various operations. - */ - vars_list orig_list; - -public: - size_t get_buffer_length() - { - return orig_list.get_buffer_length(); - } - bool construct_var_list(char *buf, size_t buf_len) - { - return orig_list.construct_var_list(buf, buf_len); - } - - bool enable(THD *thd); - bool update(THD *thd, set_var *var); - bool store(THD *thd, String *buf); - void mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name); - /* callback */ - static uchar *sysvars_get_key(const char *entry, size_t *length, - my_bool not_used __attribute__((unused))); - - friend bool sysvartrack_global_update(THD *thd, char *str, size_t len); -}; - - /* To be used in expanding the buffer. */ static const unsigned int EXTRA_ALLOC= 1024; @@ -217,7 +76,9 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) { sysvar_node_st *node; if (!(node= (sysvar_node_st *) my_malloc(sizeof(sysvar_node_st), - MYF(MY_WME | m_mem_flag)))) + MYF(MY_WME | + (mysqld_server_initialized ? + MY_THREAD_SPECIFIC : 0))))) return true; node->m_svar= (sys_var *)svar; @@ -511,6 +372,7 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, bool Session_sysvars_tracker::enable(THD *thd) { + orig_list.reinit(); mysql_mutex_lock(&LOCK_plugin); LEX_STRING tmp; tmp.str= global_system_variables.session_track_system_variables; @@ -519,6 +381,7 @@ bool Session_sysvars_tracker::enable(THD *thd) { mysql_mutex_unlock(&LOCK_plugin); orig_list.reinit(); + m_enabled= false; return true; } mysql_mutex_unlock(&LOCK_plugin); @@ -1330,49 +1193,6 @@ bool Session_state_change_tracker::store(THD *thd, String *buf) /////////////////////////////////////////////////////////////////////////////// -/** - @brief Initialize session tracker objects. -*/ - -Session_tracker::Session_tracker() -{ - /* track data ID fit into one byte in net coding */ - compile_time_assert(SESSION_TRACK_always_at_the_end < 251); - /* one tracker could serv several tracking data */ - compile_time_assert((uint)SESSION_TRACK_always_at_the_end >= - (uint)SESSION_TRACKER_END); - - m_trackers[SESSION_SYSVARS_TRACKER]= 0; - m_trackers[CURRENT_SCHEMA_TRACKER]= ¤t_schema; - m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change; - m_trackers[TRANSACTION_INFO_TRACKER]= &transaction_info; -} - - -/** - @brief Enables the tracker objects. - - @param thd [IN] The thread handle. - - @return void -*/ - -void Session_tracker::enable(THD *thd) -{ - /* - Originally and correctly this allocation was in the constructor and - deallocation in the destructor, but in this case memory counting - system works incorrectly (for example in INSERT DELAYED thread) - */ - deinit(); - m_trackers[SESSION_SYSVARS_TRACKER]= - new (std::nothrow) Session_sysvars_tracker(); - - for (int i= 0; i < SESSION_TRACKER_END; i++) - m_trackers[i]->enable(thd); -} - - /** @brief Store all change information in the specified buffer. @@ -1385,6 +1205,12 @@ void Session_tracker::store(THD *thd, String *buf) { size_t start; + /* track data ID fit into one byte in net coding */ + compile_time_assert(SESSION_TRACK_always_at_the_end < 251); + /* one tracker could serv several tracking data */ + compile_time_assert((uint) SESSION_TRACK_always_at_the_end >= + (uint) SESSION_TRACKER_END); + /* Probably most track result will fit in 251 byte so lets made it at least efficient. We allocate 1 byte for length and then will move diff --git a/sql/session_tracker.h b/sql/session_tracker.h index faa34e88c74..51e32dde639 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -71,13 +71,7 @@ private: bool m_changed; public: - /** Constructor */ - State_tracker() : m_enabled(false), m_changed(false) - {} - - /** Destructor */ - virtual ~State_tracker() - {} + virtual ~State_tracker() {} /** Getters */ bool is_enabled() const @@ -112,6 +106,130 @@ public: }; +/** + Session_sysvars_tracker + + This is a tracker class that enables & manages the tracking of session + system variables. It internally maintains a hash of user supplied variable + references and a boolean field to store if the variable was changed by the + last statement. +*/ + +class Session_sysvars_tracker: public State_tracker +{ + struct sysvar_node_st { + sys_var *m_svar; + bool *test_load; + bool m_changed; + }; + + class vars_list + { + /** + Registered system variables. (@@session_track_system_variables) + A hash to store the name of all the system variables specified by the + user. + */ + HASH m_registered_sysvars; + /** Size of buffer for string representation */ + size_t buffer_length; + /** + If TRUE then we want to check all session variable. + */ + bool track_all; + void init() + { + my_hash_init(&m_registered_sysvars, &my_charset_bin, 0, 0, 0, + (my_hash_get_key) sysvars_get_key, my_free, + HASH_UNIQUE | (mysqld_server_initialized ? + HASH_THREAD_SPECIFIC : 0)); + } + void free_hash() + { + DBUG_ASSERT(my_hash_inited(&m_registered_sysvars)); + my_hash_free(&m_registered_sysvars); + } + + sysvar_node_st *search(const sys_var *svar) + { + return reinterpret_cast( + my_hash_search(&m_registered_sysvars, + reinterpret_cast(&svar), + sizeof(sys_var*))); + } + + sysvar_node_st *at(ulong i) + { + DBUG_ASSERT(i < m_registered_sysvars.records); + return reinterpret_cast( + my_hash_element(&m_registered_sysvars, i)); + } + public: + vars_list(): buffer_length(0), track_all(false) { init(); } + void deinit() { free_hash(); } + + size_t get_buffer_length() + { + DBUG_ASSERT(buffer_length != 0); // asked earlier then should + return buffer_length; + } + + sysvar_node_st *insert_or_search(const sys_var *svar) + { + sysvar_node_st *res= search(svar); + if (!res) + { + if (track_all) + { + insert(svar); + return search(svar); + } + } + return res; + } + + bool insert(const sys_var *svar); + void reinit(); + void reset(); + inline bool is_enabled() + { + return track_all || m_registered_sysvars.records; + } + void copy(vars_list* from, THD *thd); + bool parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error, + CHARSET_INFO *char_set, bool take_mutex); + bool construct_var_list(char *buf, size_t buf_len); + bool store(THD *thd, String *buf); + }; + /** + Two objects of vars_list type are maintained to manage + various operations. + */ + vars_list orig_list; + +public: + size_t get_buffer_length() + { + return orig_list.get_buffer_length(); + } + bool construct_var_list(char *buf, size_t buf_len) + { + return orig_list.construct_var_list(buf, buf_len); + } + + bool enable(THD *thd); + bool update(THD *thd, set_var *var); + bool store(THD *thd, String *buf); + void mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name); + void deinit() { orig_list.deinit(); } + /* callback */ + static uchar *sysvars_get_key(const char *entry, size_t *length, + my_bool not_used __attribute__((unused))); + + friend bool sysvartrack_global_update(THD *thd, char *str, size_t len); +}; + + bool sysvartrack_validate_value(THD *thd, const char *str, size_t len); bool sysvartrack_global_update(THD *thd, char *str, size_t len); uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base); @@ -310,18 +428,21 @@ public: Current_schema_tracker current_schema; Session_state_change_tracker state_change; Transaction_state_tracker transaction_info; + Session_sysvars_tracker sysvars; - Session_tracker(); - ~Session_tracker() { deinit(); } - - /* trick to make happy memory accounting system */ - void deinit() + Session_tracker() { - delete m_trackers[SESSION_SYSVARS_TRACKER]; - m_trackers[SESSION_SYSVARS_TRACKER]= 0; + m_trackers[SESSION_SYSVARS_TRACKER]= &sysvars; + m_trackers[CURRENT_SCHEMA_TRACKER]= ¤t_schema; + m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change; + m_trackers[TRANSACTION_INFO_TRACKER]= &transaction_info; } - void enable(THD *thd); + void enable(THD *thd) + { + for (int i= 0; i < SESSION_TRACKER_END; i++) + m_trackers[i]->enable(thd); + } /** Returns the pointer to the tracker object for the specified tracker. */ inline State_tracker *get_tracker(enum_session_tracker tracker) const diff --git a/sql/set_var.cc b/sql/set_var.cc index 8ab892068b3..ddaf747908c 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -1016,13 +1016,13 @@ int set_var_collation_client::update(THD *thd) /* Mark client collation variables as changed */ #ifndef EMBEDDED_LIBRARY - if (thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->is_enabled()) + if (thd->session_tracker.sysvars.is_enabled()) { - thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> + thd->session_tracker.sysvars. mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_client_ptr); - thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> + thd->session_tracker.sysvars. mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_results_ptr); - thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> + thd->session_tracker.sysvars. mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_connection_ptr); } thd->session_tracker.mark_as_changed(thd, SESSION_STATE_CHANGE_TRACKER, NULL); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index d247c806a7f..97233895fd5 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1661,7 +1661,7 @@ THD::~THD() /* trick to make happy memory accounting system */ #ifndef EMBEDDED_LIBRARY - session_tracker.deinit(); + session_tracker.sysvars.deinit(); #endif //EMBEDDED_LIBRARY if (status_var.local_memory_used != 0) @@ -7302,12 +7302,11 @@ void THD::set_last_commit_gtid(rpl_gtid >id) #endif m_last_commit_gtid= gtid; #ifndef EMBEDDED_LIBRARY - if (changed_gtid && - session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->is_enabled()) + if (changed_gtid && session_tracker.sysvars.is_enabled()) { - session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> + session_tracker.sysvars. mark_as_changed(this, (LEX_CSTRING*)Sys_last_gtid_ptr); - } + } #endif } diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index 042b7c75da7..24fdc2d04a4 100644 --- a/sql/sys_vars.ic +++ b/sql/sys_vars.ic @@ -623,10 +623,7 @@ public: return (new_val == 0 && var->save_result.string_value.str != 0); } bool session_update(THD *thd, set_var *var) - { - return thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)-> - update(thd, var); - } + { return thd->session_tracker.sysvars.update(thd, var); } void session_save_default(THD *thd, set_var *var) { var->save_result.string_value.str= global_var(char*); -- cgit v1.2.1 From 1b5cf2f7e7995f8ee17da76eb28633f652852d8f Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Tue, 19 Mar 2019 20:04:10 +0400 Subject: Safe session_track_system_variables snapshot When enabling system variables tracker, make a copy of global_system_variables.session_track_system_variables under LOCK_global_system_variables. This protects from concurrent variable updates and potential freed memory access, as well as from variable reconstruction (which was previously protected by LOCK_plugin). We can also use this copy as a session variable pointer, so that we don't have to allocate memory and reconstruct it every time it is referenced. For this very reason we don't need buffer_length stuff anymore. As well as don't we need to take LOCK_plugin early in ::enable(). Unified ::parse_var_list() to acquire LOCK_plugin unconditionally. For no apparent reason it wasn't previously acquired for global variable update. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 91 ++++++++++++++++++++++++++------------------------ sql/session_tracker.h | 30 +++-------------- sql/sql_plugin.cc | 12 +++++++ sql/sys_vars.ic | 2 -- 4 files changed, 64 insertions(+), 71 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 81da43a0946..8b0e247767c 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -38,7 +38,6 @@ static const unsigned int EXTRA_ALLOC= 1024; void Session_sysvars_tracker::vars_list::reinit() { - buffer_length= 0; track_all= 0; if (m_registered_sysvars.records) my_hash_reset(&m_registered_sysvars); @@ -58,7 +57,6 @@ void Session_sysvars_tracker::vars_list::copy(vars_list* from, THD *thd) { track_all= from->track_all; free_hash(); - buffer_length= from->buffer_length; m_registered_sysvars= from->m_registered_sysvars; from->init(); } @@ -111,7 +109,6 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) in case of invalid/duplicate values. @param char_set [IN] charecter set information used for string manipulations. - @param take_mutex [IN] take LOCK_plugin @return true Error @@ -120,27 +117,21 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar) bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error, - CHARSET_INFO *char_set, - bool take_mutex) + CHARSET_INFO *char_set) { const char separator= ','; char *token, *lasts= NULL; size_t rest= var_list.length; if (!var_list.str || var_list.length == 0) - { - buffer_length= 1; return false; - } if(!strcmp(var_list.str, "*")) { track_all= true; - buffer_length= 2; return false; } - buffer_length= var_list.length + 1; token= var_list.str; track_all= false; @@ -150,8 +141,7 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, token value. Hence the mutex is handled here to avoid a performance overhead. */ - if (!thd || take_mutex) - mysql_mutex_lock(&LOCK_plugin); + mysql_mutex_lock(&LOCK_plugin); for (;;) { sys_var *svar; @@ -196,14 +186,12 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, else break; } - if (!thd || take_mutex) - mysql_mutex_unlock(&LOCK_plugin); + mysql_mutex_unlock(&LOCK_plugin); return false; error: - if (!thd || take_mutex) - mysql_mutex_unlock(&LOCK_plugin); + mysql_mutex_unlock(&LOCK_plugin); return true; } @@ -361,6 +349,25 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, return false; } + +void Session_sysvars_tracker::init(THD *thd) +{ + DBUG_ASSERT(thd->variables.session_track_system_variables == + global_system_variables.session_track_system_variables); + DBUG_ASSERT(global_system_variables.session_track_system_variables); + thd->variables.session_track_system_variables= + my_strdup(global_system_variables.session_track_system_variables, + MYF(MY_WME | MY_THREAD_SPECIFIC)); +} + + +void Session_sysvars_tracker::deinit(THD *thd) +{ + my_free(thd->variables.session_track_system_variables); + thd->variables.session_track_system_variables= 0; +} + + /** Enable session tracker by parsing global value of tracked variables. @@ -372,21 +379,16 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, bool Session_sysvars_tracker::enable(THD *thd) { + LEX_STRING tmp= { thd->variables.session_track_system_variables, + safe_strlen(thd->variables.session_track_system_variables) }; orig_list.reinit(); - mysql_mutex_lock(&LOCK_plugin); - LEX_STRING tmp; - tmp.str= global_system_variables.session_track_system_variables; - tmp.length= safe_strlen(tmp.str); - if (orig_list.parse_var_list(thd, tmp, true, thd->charset(), false) == true) + if (orig_list.parse_var_list(thd, tmp, true, thd->charset()) == true) { - mysql_mutex_unlock(&LOCK_plugin); orig_list.reinit(); m_enabled= false; return true; } - mysql_mutex_unlock(&LOCK_plugin); m_enabled= true; - return false; } @@ -412,10 +414,28 @@ bool Session_sysvars_tracker::enable(THD *thd) bool Session_sysvars_tracker::update(THD *thd, set_var *var) { vars_list tool_list; + void *copy= var->save_result.string_value.str ? + my_memdup(var->save_result.string_value.str, + var->save_result.string_value.length + 1, + MYF(MY_WME | MY_THREAD_SPECIFIC)) : + my_strdup("", MYF(MY_WME | MY_THREAD_SPECIFIC)); + + if (!copy) + return true; + if (tool_list.parse_var_list(thd, var->save_result.string_value, true, - thd->charset(), true)) + thd->charset())) + { + my_free(copy); return true; + } + + my_free(thd->variables.session_track_system_variables); + thd->variables.session_track_system_variables= static_cast(copy); + orig_list.copy(&tool_list, thd); + orig_list.construct_var_list(thd->variables.session_track_system_variables, + var->save_result.string_value.length + 1); return false; } @@ -562,7 +582,7 @@ bool sysvartrack_global_update(THD *thd, char *str, size_t len) { LEX_STRING tmp= { str, len }; Session_sysvars_tracker::vars_list dummy; - if (!dummy.parse_var_list(thd, tmp, false, system_charset_info, false)) + if (!dummy.parse_var_list(thd, tmp, false, system_charset_info)) { dummy.construct_var_list(str, len + 1); return false; @@ -571,27 +591,12 @@ bool sysvartrack_global_update(THD *thd, char *str, size_t len) } -uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base) -{ - Session_sysvars_tracker *tracker= static_cast - (thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)); - size_t len= tracker->get_buffer_length(); - char *res= (char*) thd->alloc(len + sizeof(char*)); - if (res) - { - char *buf= res + sizeof(char*); - *((char**) res)= buf; - tracker->construct_var_list(buf, len); - } - return (uchar*) res; -} - - int session_tracker_init() { + DBUG_ASSERT(global_system_variables.session_track_system_variables); if (sysvartrack_validate_value(0, global_system_variables.session_track_system_variables, - safe_strlen(global_system_variables.session_track_system_variables))) + strlen(global_system_variables.session_track_system_variables))) { sql_print_error("The variable session_track_system_variables has " "invalid values."); diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 51e32dde639..55c78a75978 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -131,8 +131,6 @@ class Session_sysvars_tracker: public State_tracker user. */ HASH m_registered_sysvars; - /** Size of buffer for string representation */ - size_t buffer_length; /** If TRUE then we want to check all session variable. */ @@ -165,15 +163,9 @@ class Session_sysvars_tracker: public State_tracker my_hash_element(&m_registered_sysvars, i)); } public: - vars_list(): buffer_length(0), track_all(false) { init(); } + vars_list(): track_all(false) { init(); } void deinit() { free_hash(); } - size_t get_buffer_length() - { - DBUG_ASSERT(buffer_length != 0); // asked earlier then should - return buffer_length; - } - sysvar_node_st *insert_or_search(const sys_var *svar) { sysvar_node_st *res= search(svar); @@ -197,7 +189,7 @@ class Session_sysvars_tracker: public State_tracker } void copy(vars_list* from, THD *thd); bool parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error, - CHARSET_INFO *char_set, bool take_mutex); + CHARSET_INFO *char_set); bool construct_var_list(char *buf, size_t buf_len); bool store(THD *thd, String *buf); }; @@ -208,15 +200,8 @@ class Session_sysvars_tracker: public State_tracker vars_list orig_list; public: - size_t get_buffer_length() - { - return orig_list.get_buffer_length(); - } - bool construct_var_list(char *buf, size_t buf_len) - { - return orig_list.construct_var_list(buf, buf_len); - } - + void init(THD *thd); + void deinit(THD *thd); bool enable(THD *thd); bool update(THD *thd, set_var *var); bool store(THD *thd, String *buf); @@ -232,7 +217,6 @@ public: bool sysvartrack_validate_value(THD *thd, const char *str, size_t len); bool sysvartrack_global_update(THD *thd, char *str, size_t len); -uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base); /** @@ -444,12 +428,6 @@ public: m_trackers[i]->enable(thd); } - /** Returns the pointer to the tracker object for the specified tracker. */ - inline State_tracker *get_tracker(enum_session_tracker tracker) const - { - return m_trackers[tracker]; - } - inline void mark_as_changed(THD *thd, enum enum_session_tracker tracker, LEX_CSTRING *data) { diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index b2ceb1627a1..d0682ecb151 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -3149,6 +3149,11 @@ void plugin_thdvar_init(THD *thd) thd->variables.enforced_table_plugin= NULL; cleanup_variables(&thd->variables); + /* This and all other variable cleanups are here for COM_CHANGE_USER :( */ +#ifndef EMBEDDED_LIBRARY + thd->session_tracker.sysvars.deinit(thd); +#endif + thd->variables= global_system_variables; /* we are going to allocate these lazily */ @@ -3170,6 +3175,9 @@ void plugin_thdvar_init(THD *thd) intern_plugin_unlock(NULL, old_enforced_table_plugin); mysql_mutex_unlock(&LOCK_plugin); +#ifndef EMBEDDED_LIBRARY + thd->session_tracker.sysvars.init(thd); +#endif DBUG_VOID_RETURN; } @@ -3235,6 +3243,10 @@ void plugin_thdvar_cleanup(THD *thd) plugin_ref *list; DBUG_ENTER("plugin_thdvar_cleanup"); +#ifndef EMBEDDED_LIBRARY + thd->session_tracker.sysvars.deinit(thd); +#endif + mysql_mutex_lock(&LOCK_plugin); unlock_variables(thd, &thd->variables); diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index 24fdc2d04a4..d8d95046cc2 100644 --- a/sql/sys_vars.ic +++ b/sql/sys_vars.ic @@ -641,8 +641,6 @@ public: DBUG_ASSERT(res == 0); } } - uchar *session_value_ptr(THD *thd, const LEX_CSTRING *base) - { return sysvartrack_session_value_ptr(thd, base); } }; #endif //EMBEDDED_LIBRARY -- cgit v1.2.1 From 53671a1fff8d4aa0978be2fb916f8e053c09424a Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 20 Mar 2019 01:32:10 +0400 Subject: Make connect speed great again Rather than parsing session_track_system_variables when thread starts, do it when first trackable event occurs. Benchmarked on a 2socket/20core/40threads Broadwell system using sysbench connect brencmark @40 threads (with select 1 disabled): 101379.77 -> 143016.68 CPS, whereas 10.2 is currently at 137766.31 CPS. Part of MDEV-14984 - regression in connect performance --- sql/log.cc | 2 ++ sql/session_tracker.cc | 27 ++++++++++++++++++--------- sql/session_tracker.h | 1 + sql/sql_class.cc | 1 + 4 files changed, 22 insertions(+), 9 deletions(-) (limited to 'sql') diff --git a/sql/log.cc b/sql/log.cc index 92b2061d4bf..ac885746c62 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -7903,6 +7903,7 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader) */ for (current= queue; current != NULL; current= current->next) { + set_current_thd(current->thd); binlog_cache_mngr *cache_mngr= current->cache_mngr; /* @@ -7938,6 +7939,7 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader) cache_mngr->delayed_error= false; } } + set_current_thd(leader->thd); bool synced= 0; if (unlikely(flush_and_sync(&synced))) diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 8b0e247767c..db82b7dffe9 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -379,16 +379,10 @@ void Session_sysvars_tracker::deinit(THD *thd) bool Session_sysvars_tracker::enable(THD *thd) { - LEX_STRING tmp= { thd->variables.session_track_system_variables, - safe_strlen(thd->variables.session_track_system_variables) }; orig_list.reinit(); - if (orig_list.parse_var_list(thd, tmp, true, thd->charset()) == true) - { - orig_list.reinit(); - m_enabled= false; - return true; - } - m_enabled= true; + m_parsed= false; + m_enabled= thd->variables.session_track_system_variables && + *thd->variables.session_track_system_variables; return false; } @@ -433,6 +427,7 @@ bool Session_sysvars_tracker::update(THD *thd, set_var *var) my_free(thd->variables.session_track_system_variables); thd->variables.session_track_system_variables= static_cast(copy); + m_parsed= true; orig_list.copy(&tool_list, thd); orig_list.construct_var_list(thd->variables.session_track_system_variables, var->save_result.string_value.length + 1); @@ -540,6 +535,20 @@ void Session_sysvars_tracker::mark_as_changed(THD *thd, { sysvar_node_st *node; sys_var *svar= (sys_var *)var; + + if (!m_parsed) + { + DBUG_ASSERT(thd->variables.session_track_system_variables); + LEX_STRING tmp= { thd->variables.session_track_system_variables, + strlen(thd->variables.session_track_system_variables) }; + if (orig_list.parse_var_list(thd, tmp, true, thd->charset())) + { + orig_list.reinit(); + return; + } + m_parsed= true; + } + /* Check if the specified system variable is being tracked, if so mark it as changed and also set the class's m_changed flag. diff --git a/sql/session_tracker.h b/sql/session_tracker.h index 55c78a75978..b6694970c38 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -198,6 +198,7 @@ class Session_sysvars_tracker: public State_tracker various operations. */ vars_list orig_list; + bool m_parsed; public: void init(THD *thd); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 97233895fd5..da04ddad49c 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -7304,6 +7304,7 @@ void THD::set_last_commit_gtid(rpl_gtid >id) #ifndef EMBEDDED_LIBRARY if (changed_gtid && session_tracker.sysvars.is_enabled()) { + DBUG_ASSERT(current_thd == this); session_tracker.sysvars. mark_as_changed(this, (LEX_CSTRING*)Sys_last_gtid_ptr); } -- cgit v1.2.1 From 894df7edb67b888c41eae5ffbe654ceba97c6b8f Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 21 Mar 2019 00:42:48 +0400 Subject: Adieu find_sys_var_ex() Only take LOCK_plugin for plugin system variables. Reverted optimisation that was originally done for session tracker: it makes much less sense now. Specifically only if connections would want to track plugin session variables changes and these changes would actually happen frequently. If this ever becomes an issue, there're much better ways to optimise this workload. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.cc | 38 ++++---------------------------------- sql/set_var.h | 3 ++- sql/sql_lex.cc | 5 ++--- sql/sql_plugin.cc | 35 +++++++++-------------------------- sql/sql_plugin.h | 3 --- 5 files changed, 17 insertions(+), 67 deletions(-) (limited to 'sql') diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index db82b7dffe9..63a6770f7d1 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -135,13 +135,6 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, token= var_list.str; track_all= false; - /* - If Lock to the plugin mutex is not acquired here itself, it results - in having to acquire it multiple times in find_sys_var_ex for each - token value. Hence the mutex is handled here to avoid a performance - overhead. - */ - mysql_mutex_lock(&LOCK_plugin); for (;;) { sys_var *svar; @@ -165,11 +158,10 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, { track_all= true; } - else if ((svar= - find_sys_var_ex(thd, var.str, var.length, throw_error, true))) + else if ((svar= find_sys_var(thd, var.str, var.length, throw_error))) { if (insert(svar) == TRUE) - goto error; + return true; } else if (throw_error && thd) { @@ -179,20 +171,14 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd, "be ignored.", (int)var.length, token); } else - goto error; + return true; if (lasts) token= lasts + 1; else break; } - mysql_mutex_unlock(&LOCK_plugin); - return false; - -error: - mysql_mutex_unlock(&LOCK_plugin); - return true; } @@ -211,14 +197,6 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len) token= var_list.str; - /* - If Lock to the plugin mutex is not acquired here itself, it results - in having to acquire it multiple times in find_sys_var_ex for each - token value. Hence the mutex is handled here to avoid a performance - overhead. - */ - if (!thd) - mysql_mutex_lock(&LOCK_plugin); for (;;) { LEX_CSTRING var; @@ -237,22 +215,14 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len) /* Remove leading/trailing whitespace. */ trim_whitespace(system_charset_info, &var); - if(!strcmp(var.str, "*") && - !find_sys_var_ex(thd, var.str, var.length, false, true)) - { - if (!thd) - mysql_mutex_unlock(&LOCK_plugin); + if (!strcmp(var.str, "*") && !find_sys_var(thd, var.str, var.length)) return true; - } if (lasts) token= lasts + 1; else break; } - if (!thd) - mysql_mutex_unlock(&LOCK_plugin); - return false; } diff --git a/sql/set_var.h b/sql/set_var.h index 6097b28e76f..6e673cffefb 100644 --- a/sql/set_var.h +++ b/sql/set_var.h @@ -398,7 +398,8 @@ extern SHOW_COMP_OPTION have_openssl; SHOW_VAR* enumerate_sys_vars(THD *thd, bool sorted, enum enum_var_type type); int fill_sysvars(THD *thd, TABLE_LIST *tables, COND *cond); -sys_var *find_sys_var(THD *thd, const char *str, size_t length=0); +sys_var *find_sys_var(THD *thd, const char *str, size_t length= 0, + bool throw_error= false); int sql_set_variables(THD *thd, List *var_list, bool free); #define SYSVAR_AUTOSIZE(VAR,VAL) \ diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index b5ff060ecc6..6ce778d03cf 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -7254,8 +7254,7 @@ bool LEX::set_system_variable(THD *thd, enum_var_type var_type, { sys_var *tmp; if (unlikely(check_reserved_words(name1)) || - unlikely(!(tmp= find_sys_var_ex(thd, name2->str, name2->length, true, - false)))) + unlikely(!(tmp= find_sys_var(thd, name2->str, name2->length, true)))) { my_error(ER_UNKNOWN_STRUCTURED_VARIABLE, MYF(0), (int) name1->length, name1->str); @@ -7649,7 +7648,7 @@ int set_statement_var_if_exists(THD *thd, const char *var_name, my_error(ER_SP_BADSTATEMENT, MYF(0), "[NO]WAIT"); return 1; } - if ((sysvar= find_sys_var_ex(thd, var_name, var_name_length, true, false))) + if ((sysvar= find_sys_var(thd, var_name, var_name_length, true))) { Item *item= new (thd->mem_root) Item_uint(thd, value); set_var *var= new (thd->mem_root) set_var(thd, OPT_SESSION, sysvar, diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index d0682ecb151..b8aff064aca 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -2822,37 +2822,25 @@ static void update_func_double(THD *thd, struct st_mysql_sys_var *var, System Variables support ****************************************************************************/ -sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length, - bool throw_error, bool locked) +sys_var *find_sys_var(THD *thd, const char *str, size_t length, + bool throw_error) { sys_var *var; - sys_var_pluginvar *pi= NULL; - plugin_ref plugin; - DBUG_ENTER("find_sys_var_ex"); + sys_var_pluginvar *pi; + DBUG_ENTER("find_sys_var"); DBUG_PRINT("enter", ("var '%.*s'", (int)length, str)); - if (!locked) - mysql_mutex_lock(&LOCK_plugin); mysql_prlock_rdlock(&LOCK_system_variables_hash); if ((var= intern_find_sys_var(str, length)) && (pi= var->cast_pluginvar())) { - mysql_prlock_unlock(&LOCK_system_variables_hash); - LEX *lex= thd ? thd->lex : 0; - if (!(plugin= intern_plugin_lock(lex, plugin_int_to_ref(pi->plugin)))) + mysql_mutex_lock(&LOCK_plugin); + if (!intern_plugin_lock(thd ? thd->lex : 0, plugin_int_to_ref(pi->plugin), + PLUGIN_IS_READY)) var= NULL; /* failed to lock it, it must be uninstalling */ - else - if (!(plugin_state(plugin) & PLUGIN_IS_READY)) - { - /* initialization not completed */ - var= NULL; - intern_plugin_unlock(lex, plugin); - } - } - else - mysql_prlock_unlock(&LOCK_system_variables_hash); - if (!locked) mysql_mutex_unlock(&LOCK_plugin); + } + mysql_prlock_unlock(&LOCK_system_variables_hash); if (unlikely(!throw_error && !var)) my_error(ER_UNKNOWN_SYSTEM_VARIABLE, MYF(0), @@ -2861,11 +2849,6 @@ sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length, } -sys_var *find_sys_var(THD *thd, const char *str, size_t length) -{ - return find_sys_var_ex(thd, str, length, false, false); -} - /* called by register_var, construct_options and test_plugin_options. Returns the 'bookmark' for the named variable. diff --git a/sql/sql_plugin.h b/sql/sql_plugin.h index 4e899e18f9b..01ec0563050 100644 --- a/sql/sql_plugin.h +++ b/sql/sql_plugin.h @@ -196,9 +196,6 @@ extern void sync_dynamic_session_variables(THD* thd, bool global_lock); extern bool plugin_dl_foreach(THD *thd, const LEX_CSTRING *dl, plugin_foreach_func *func, void *arg); -sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length, - bool throw_error, bool locked); - extern void sync_dynamic_session_variables(THD* thd, bool global_lock); #endif -- cgit v1.2.1 From 779fb636daf4c127dbb90f75bab004ac1bbe12df Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 20 Mar 2019 18:35:20 +0400 Subject: Revert THD::THD(skip_global_sys_var_lock) argument Originally introduced by e972125f1 to avoid harmless wait for LOCK_global_system_variables in a newly created thread, which creation was initiated by system variable update. At the same time it opens dangerous hole, when system variable update thread already released LOCK_global_system_variables and ack_receiver thread haven't yet completed new THD construction. In this case THD constructor goes completely unprotected. Since ack_receiver.stop() waits for the thread to go down, we have to temporarily release LOCK_global_system_variables so that it doesn't deadlock with ack_receiver.run(). Unfortunately it breaks atomicity of rpl_semi_sync_master_enabled updates and makes them not serialized. LOCK_rpl_semi_sync_master_enabled was introduced to workaround the above. TODO: move ack_receiver start/stop into repl_semisync_master enable_master/disable_master under LOCK_binlog protection? Part of MDEV-14984 - regression in connect performance --- sql/mysqld.cc | 2 ++ sql/semisync_master.cc | 11 ++++++----- sql/semisync_master.h | 8 +++++--- sql/semisync_master_ack_receiver.cc | 3 +-- sql/session_tracker.cc | 1 + sql/sql_class.cc | 12 +++++------- sql/sql_class.h | 11 +++-------- sql/sys_vars.cc | 10 ++++++---- 8 files changed, 29 insertions(+), 29 deletions(-) (limited to 'sql') diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 73dd7ce36af..4f5026fd3b5 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -941,6 +941,7 @@ PSI_mutex_key key_LOCK_relaylog_end_pos; PSI_mutex_key key_LOCK_thread_id; PSI_mutex_key key_LOCK_slave_state, key_LOCK_binlog_state, key_LOCK_rpl_thread, key_LOCK_rpl_thread_pool, key_LOCK_parallel_entry; +PSI_mutex_key key_LOCK_rpl_semi_sync_master_enabled; PSI_mutex_key key_LOCK_binlog; PSI_mutex_key key_LOCK_stats, @@ -1038,6 +1039,7 @@ static PSI_mutex_info all_server_mutexes[]= { &key_LOCK_rpl_thread_pool, "LOCK_rpl_thread_pool", 0}, { &key_LOCK_parallel_entry, "LOCK_parallel_entry", 0}, { &key_LOCK_ack_receiver, "Ack_receiver::mutex", 0}, + { &key_LOCK_rpl_semi_sync_master_enabled, "LOCK_rpl_semi_sync_master_enabled", 0}, { &key_LOCK_binlog, "LOCK_binlog", 0} }; diff --git a/sql/semisync_master.cc b/sql/semisync_master.cc index 4e37e3af58d..f9fb4d9147d 100644 --- a/sql/semisync_master.cc +++ b/sql/semisync_master.cc @@ -352,7 +352,7 @@ Repl_semi_sync_master::Repl_semi_sync_master() int Repl_semi_sync_master::init_object() { - int result; + int result= 0; m_init_done = true; @@ -362,6 +362,8 @@ int Repl_semi_sync_master::init_object() set_wait_point(rpl_semi_sync_master_wait_point); /* Mutex initialization can only be done after MY_INIT(). */ + mysql_mutex_init(key_LOCK_rpl_semi_sync_master_enabled, + &LOCK_rpl_semi_sync_master_enabled, MY_MUTEX_INIT_FAST); mysql_mutex_init(key_LOCK_binlog, &LOCK_binlog, MY_MUTEX_INIT_FAST); mysql_cond_init(key_COND_binlog_send, @@ -383,7 +385,7 @@ int Repl_semi_sync_master::init_object() } else { - result = disable_master(); + disable_master(); } return result; @@ -421,7 +423,7 @@ int Repl_semi_sync_master::enable_master() return result; } -int Repl_semi_sync_master::disable_master() +void Repl_semi_sync_master::disable_master() { /* Must have the lock when we do enable of disable. */ lock(); @@ -446,14 +448,13 @@ int Repl_semi_sync_master::disable_master() } unlock(); - - return 0; } void Repl_semi_sync_master::cleanup() { if (m_init_done) { + mysql_mutex_destroy(&LOCK_rpl_semi_sync_master_enabled); mysql_mutex_destroy(&LOCK_binlog); mysql_cond_destroy(&COND_binlog_send); m_init_done= 0; diff --git a/sql/semisync_master.h b/sql/semisync_master.h index de5e3240802..517175b5b06 100644 --- a/sql/semisync_master.h +++ b/sql/semisync_master.h @@ -23,6 +23,7 @@ #include "semisync_master_ack_receiver.h" #ifdef HAVE_PSI_INTERFACE +extern PSI_mutex_key key_LOCK_rpl_semi_sync_master_enabled; extern PSI_mutex_key key_LOCK_binlog; extern PSI_cond_key key_COND_binlog_send; #endif @@ -365,7 +366,6 @@ public: */ class Repl_semi_sync_master :public Repl_semi_sync_base { - private: Active_tranx *m_active_tranxs; /* active transaction list: the list will be cleared when semi-sync switches off. */ @@ -491,8 +491,8 @@ class Repl_semi_sync_master /* Enable the object to enable semi-sync replication inside the master. */ int enable_master(); - /* Enable the object to enable semi-sync replication inside the master. */ - int disable_master(); + /* Disable the object to disable semi-sync replication inside the master. */ + void disable_master(); /* Add a semi-sync replication slave */ void add_slave(); @@ -619,6 +619,8 @@ class Repl_semi_sync_master int before_reset_master(); void check_and_switch(); + + mysql_mutex_t LOCK_rpl_semi_sync_master_enabled; }; enum rpl_semi_sync_master_wait_point_t { diff --git a/sql/semisync_master_ack_receiver.cc b/sql/semisync_master_ack_receiver.cc index 607d4844658..81f494c9d34 100644 --- a/sql/semisync_master_ack_receiver.cc +++ b/sql/semisync_master_ack_receiver.cc @@ -185,8 +185,7 @@ static void init_net(NET *net, unsigned char *buff, unsigned int buff_len) void Ack_receiver::run() { - // skip LOCK_global_system_variables due to the 3rd arg - THD *thd= new THD(next_thread_id(), false, true); + THD *thd= new THD(next_thread_id()); NET net; unsigned char net_buff[REPLY_MESSAGE_MAX_LENGTH]; diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 63a6770f7d1..f4dab11bb42 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -322,6 +322,7 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf, void Session_sysvars_tracker::init(THD *thd) { + mysql_mutex_assert_owner(&LOCK_global_system_variables); DBUG_ASSERT(thd->variables.session_track_system_variables == global_system_variables.session_track_system_variables); DBUG_ASSERT(global_system_variables.session_track_system_variables); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index da04ddad49c..2eebfbb6db0 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -597,7 +597,7 @@ extern "C" void thd_kill_timeout(THD* thd) thd->awake(KILL_TIMEOUT); } -THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) +THD::THD(my_thread_id id, bool is_wsrep_applier) :Statement(&main_lex, &main_mem_root, STMT_CONVENTIONAL_EXECUTION, /* statement id */ 0), rli_fake(0), rgi_fake(0), rgi_slave(NULL), @@ -792,7 +792,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) /* Call to init() below requires fully initialized Open_tables_state. */ reset_open_tables_state(this); - init(skip_global_sys_var_lock); + init(); #if defined(ENABLED_PROFILING) profiling.set_thd(this); #endif @@ -1167,11 +1167,10 @@ const Type_handler *THD::type_handler_for_date() const Init common variables that has to be reset on start and on change_user */ -void THD::init(bool skip_lock) +void THD::init() { DBUG_ENTER("thd::init"); - if (!skip_lock) - mysql_mutex_lock(&LOCK_global_system_variables); + mysql_mutex_lock(&LOCK_global_system_variables); plugin_thdvar_init(this); /* plugin_thd_var_init() sets variables= global_system_variables, which @@ -1184,8 +1183,7 @@ void THD::init(bool skip_lock) ::strmake(default_master_connection_buff, global_system_variables.default_master_connection.str, variables.default_master_connection.length); - if (!skip_lock) - mysql_mutex_unlock(&LOCK_global_system_variables); + mysql_mutex_unlock(&LOCK_global_system_variables); user_time.val= start_time= start_time_sec_part= 0; diff --git a/sql/sql_class.h b/sql/sql_class.h index e1119757957..221e453eab5 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3263,17 +3263,12 @@ public: /** @param id thread identifier @param is_wsrep_applier thread type - @param skip_lock instruct whether @c LOCK_global_system_variables - is already locked, to not acquire it then. */ - THD(my_thread_id id, bool is_wsrep_applier= false, bool skip_lock= false); + THD(my_thread_id id, bool is_wsrep_applier= false); ~THD(); - /** - @param skip_lock instruct whether @c LOCK_global_system_variables - is already locked, to not acquire it then. - */ - void init(bool skip_lock= false); + + void init(); /* Initialize memory roots necessary for query processing and (!) pre-allocate memory for it. We can't do that in THD constructor because diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 004fd4baecb..185078ff363 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3144,6 +3144,8 @@ static Sys_var_replicate_events_marked_for_skip Replicate_events_marked_for_skip static bool fix_rpl_semi_sync_master_enabled(sys_var *self, THD *thd, enum_var_type type) { + mysql_mutex_unlock(&LOCK_global_system_variables); + mysql_mutex_lock(&repl_semisync_master.LOCK_rpl_semi_sync_master_enabled); if (rpl_semi_sync_master_enabled) { if (repl_semisync_master.enable_master() != 0) @@ -3156,11 +3158,11 @@ static bool fix_rpl_semi_sync_master_enabled(sys_var *self, THD *thd, } else { - if (repl_semisync_master.disable_master() != 0) - rpl_semi_sync_master_enabled= true; - if (!rpl_semi_sync_master_enabled) - ack_receiver.stop(); + repl_semisync_master.disable_master(); + ack_receiver.stop(); } + mysql_mutex_unlock(&repl_semisync_master.LOCK_rpl_semi_sync_master_enabled); + mysql_mutex_lock(&LOCK_global_system_variables); return false; } -- cgit v1.2.1 From e8dd18a474ee6b48eb7f92e3831f9e359b0bdc6e Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sat, 4 May 2019 12:43:29 +0400 Subject: Restore vars_list destructor Regression after reverting fair THD members constructors/destructors. vars_list can be used standalone, in such cases destructor is needed. Part of MDEV-14984 - regression in connect performance --- sql/session_tracker.h | 1 + 1 file changed, 1 insertion(+) (limited to 'sql') diff --git a/sql/session_tracker.h b/sql/session_tracker.h index b6694970c38..226b026d590 100644 --- a/sql/session_tracker.h +++ b/sql/session_tracker.h @@ -164,6 +164,7 @@ class Session_sysvars_tracker: public State_tracker } public: vars_list(): track_all(false) { init(); } + ~vars_list() { if (my_hash_inited(&m_registered_sysvars)) free_hash(); } void deinit() { free_hash(); } sysvar_node_st *insert_or_search(const sys_var *svar) -- cgit v1.2.1