From 91780f025d98c69de3fa67ea403297cdb1219444 Mon Sep 17 00:00:00 2001 From: Christopher Powers Date: Sat, 1 Dec 2018 17:28:23 -0600 Subject: [PATCH] Bug#27839644 GLOBAL STATUS VARIABLES DRIFT AFTER ROLLBACK This patch addresses a problem where the status counters for a closing connection may increase after they have been added to the global status totals, and then decrease when the associated THD is deleted. This temporary bump in the counters can confuse monitoring processes. To fix the problem, add_to_status() is moved to the end of THD::release_resources(), ensuring that status counter increments will be added to the global status totals. --- sql/handler.cc | 14 ++++++++++++++ sql/rpl_master.cc | 4 +++- sql/sql_class.cc | 36 +++++++++++++++++++++++++----------- sql/sql_class.h | 1 + 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index 0a5b01f790c0..f1547a2ffa6b 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1459,6 +1459,7 @@ int ha_prepare(THD *thd) while (ha_info) { handlerton *ht= ha_info->ht(); + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_prepare_count++; if (ht->prepare) { @@ -1904,6 +1905,7 @@ int ha_commit_low(THD *thd, bool all, bool run_after_commit) my_error(ER_ERROR_DURING_COMMIT, MYF(0), err); error=1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_commit_count++; ha_info_next= ha_info->next(); if (restore_backup_ha_data) @@ -1976,6 +1978,7 @@ int ha_rollback_low(THD *thd, bool all) my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error= 1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_rollback_count++; ha_info_next= ha_info->next(); if (restore_backup_ha_data) @@ -2156,6 +2159,7 @@ int ha_commit_attachable(THD *thd) DBUG_ASSERT(false); error= 1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_commit_count++; ha_info_next= ha_info->next(); @@ -2288,6 +2292,7 @@ int ha_rollback_to_savepoint(THD *thd, SAVEPOINT *sv) my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error=1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_savepoint_rollback_count++; if (ht->prepare == 0) trn_ctx->set_no_2pc(trx_scope, true); @@ -2307,6 +2312,7 @@ int ha_rollback_to_savepoint(THD *thd, SAVEPOINT *sv) my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error=1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_rollback_count++; ha_info_next= ha_info->next(); ha_info->reset(); /* keep it conveniently zero-filled */ @@ -2348,6 +2354,7 @@ int ha_prepare_low(THD *thd, bool all) my_error(ER_ERROR_DURING_COMMIT, MYF(0), err); error= 1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_prepare_count++; } DBUG_EXECUTE_IF("crash_commit_after_prepare", DBUG_SUICIDE();); @@ -2388,6 +2395,7 @@ int ha_savepoint(THD *thd, SAVEPOINT *sv) my_error(ER_GET_ERRNO, MYF(0), err); error=1; } + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_savepoint_count++; } /* @@ -5633,7 +5641,10 @@ int ha_discover(THD *thd, const char *db, const char *name, error= 0; if (!error) + { + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.ha_discover_count++; + } DBUG_RETURN(error); } @@ -6545,7 +6556,10 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, is_mrr_assoc= !MY_TEST(mode & HA_MRR_NO_ASSOCIATION); if (is_mrr_assoc) + { + DBUG_ASSERT(!thd->status_var_aggregated); table->in_use->status_var.ha_multi_range_read_init_count++; + } rowids_buf_end= buf->buffer_end; elem_size= h->ref_length + (int)is_mrr_assoc * sizeof(void*); diff --git a/sql/rpl_master.cc b/sql/rpl_master.cc index 030db49d2ca4..61471ba6dce2 100644 --- a/sql/rpl_master.cc +++ b/sql/rpl_master.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2010, 2017, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -318,6 +318,7 @@ bool com_binlog_dump(THD *thd, char *packet, size_t packet_length) const uchar* packet_position= (uchar *) packet; size_t packet_bytes_todo= packet_length; + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.com_other++; thd->enable_slow_log= opt_log_slow_admin_statements; if (check_global_access(thd, REPL_SLAVE_ACL)) @@ -368,6 +369,7 @@ bool com_binlog_dump_gtid(THD *thd, char *packet, size_t packet_length) Sid_map sid_map(NULL/*no sid_lock because this is a completely local object*/); Gtid_set slave_gtid_executed(&sid_map); + DBUG_ASSERT(!thd->status_var_aggregated); thd->status_var.com_other++; thd->enable_slow_log= opt_log_slow_admin_statements; if (check_global_access(thd, REPL_SLAVE_ACL)) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 931bb26b462e..49ac2015b3ba 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1789,17 +1789,6 @@ void THD::release_resources() Global_THD_manager::get_instance()->release_thread_id(m_thread_id); - mysql_mutex_lock(&LOCK_status); - add_to_status(&global_status_var, &status_var, false); - /* - Status queries after this point should not aggregate THD::status_var - since the values has been added to global_status_var. - The status values are not reset so that they can still be read - by performance schema. - */ - status_var_aggregated= true; - mysql_mutex_unlock(&LOCK_status); - /* Ensure that no one is using THD */ mysql_mutex_lock(&LOCK_thd_data); mysql_mutex_lock(&LOCK_query_plan); @@ -1855,6 +1844,18 @@ void THD::release_resources() if (current_thd == this) restore_globals(); + + mysql_mutex_lock(&LOCK_status); + add_to_status(&global_status_var, &status_var, false); + /* + Status queries after this point should not aggregate THD::status_var + since the values has been added to global_status_var. + The status values are not reset so that they can still be read + by performance schema. + */ + status_var_aggregated= true; + mysql_mutex_unlock(&LOCK_status); + m_release_resources_done= true; } @@ -2062,7 +2063,10 @@ void THD::awake(THD::killed_state state_to_set) ha_kill_connection(this); if (state_to_set == THD::KILL_TIMEOUT) + { + DBUG_ASSERT(!status_var_aggregated); status_var.max_execution_time_exceeded++; + } /* Broadcast a condition to kick the target if it is waiting on it. */ @@ -4222,6 +4226,7 @@ void THD::inc_examined_row_count(ha_rows count) void THD::inc_status_created_tmp_disk_tables() { + DBUG_ASSERT(!status_var_aggregated); status_var.created_tmp_disk_tables++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_created_tmp_disk_tables)(m_statement_psi, 1); @@ -4230,6 +4235,7 @@ void THD::inc_status_created_tmp_disk_tables() void THD::inc_status_created_tmp_tables() { + DBUG_ASSERT(!status_var_aggregated); status_var.created_tmp_tables++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_created_tmp_tables)(m_statement_psi, 1); @@ -4238,6 +4244,7 @@ void THD::inc_status_created_tmp_tables() void THD::inc_status_select_full_join() { + DBUG_ASSERT(!status_var_aggregated); status_var.select_full_join_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_select_full_join)(m_statement_psi, 1); @@ -4246,6 +4253,7 @@ void THD::inc_status_select_full_join() void THD::inc_status_select_full_range_join() { + DBUG_ASSERT(!status_var_aggregated); status_var.select_full_range_join_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_select_full_range_join)(m_statement_psi, 1); @@ -4254,6 +4262,7 @@ void THD::inc_status_select_full_range_join() void THD::inc_status_select_range() { + DBUG_ASSERT(!status_var_aggregated); status_var.select_range_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_select_range)(m_statement_psi, 1); @@ -4262,6 +4271,7 @@ void THD::inc_status_select_range() void THD::inc_status_select_range_check() { + DBUG_ASSERT(!status_var_aggregated); status_var.select_range_check_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_select_range_check)(m_statement_psi, 1); @@ -4270,6 +4280,7 @@ void THD::inc_status_select_range_check() void THD::inc_status_select_scan() { + DBUG_ASSERT(!status_var_aggregated); status_var.select_scan_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_select_scan)(m_statement_psi, 1); @@ -4278,6 +4289,7 @@ void THD::inc_status_select_scan() void THD::inc_status_sort_merge_passes() { + DBUG_ASSERT(!status_var_aggregated); status_var.filesort_merge_passes++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_sort_merge_passes)(m_statement_psi, 1); @@ -4286,6 +4298,7 @@ void THD::inc_status_sort_merge_passes() void THD::inc_status_sort_range() { + DBUG_ASSERT(!status_var_aggregated); status_var.filesort_range_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_sort_range)(m_statement_psi, 1); @@ -4303,6 +4316,7 @@ void THD::inc_status_sort_rows(ha_rows count) void THD::inc_status_sort_scan() { + DBUG_ASSERT(!status_var_aggregated); status_var.filesort_scan_count++; #ifdef HAVE_PSI_STATEMENT_INTERFACE PSI_STATEMENT_CALL(inc_statement_sort_scan)(m_statement_psi, 1); diff --git a/sql/sql_class.h b/sql/sql_class.h index 87ee8f307a3a..1db14c77d82f 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1660,6 +1660,7 @@ class THD :public MDL_context_owner, */ void save_current_query_costs() { + DBUG_ASSERT(!status_var_aggregated); status_var.last_query_cost= m_current_query_cost; status_var.last_query_partial_plans= m_current_query_partial_plans; }