From f486f49e8dca72023f7312da138d71eb504a16f4 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 11 Nov 2013 16:17:32 +0100 Subject: [PATCH] MDEV-4824 userstats - wrong user statistics (and valgrind warnings) * move thd userstat initialization to the same function that was adding thd userstat to global counters. * initialize thd->start_bytes_received in THD::init (when thd->userstat_running is set) --- mysql-test/r/userstat-badlogin-4824.result | 16 +++++++++++ mysql-test/t/userstat-badlogin-4824.test | 33 ++++++++++++++++++++++ sql/event_data_objects.cc | 2 +- sql/log_event.cc | 6 ++-- sql/log_event_old.cc | 2 +- sql/sql_class.cc | 1 + sql/sql_class.h | 2 +- sql/sql_parse.cc | 31 ++++++++++---------- sql/sql_parse.h | 2 +- sql/sql_prepare.cc | 8 +++--- 10 files changed, 76 insertions(+), 27 deletions(-) create mode 100644 mysql-test/r/userstat-badlogin-4824.result create mode 100644 mysql-test/t/userstat-badlogin-4824.test diff --git a/mysql-test/r/userstat-badlogin-4824.result b/mysql-test/r/userstat-badlogin-4824.result new file mode 100644 index 00000000000..8362e8d5239 --- /dev/null +++ b/mysql-test/r/userstat-badlogin-4824.result @@ -0,0 +1,16 @@ +create user foo@localhost identified by 'foo'; +flush user_statistics; +set global userstat=1; +select 1; +1 +1 +select user, bytes_received from information_schema.user_statistics where user = 'foo'; +user bytes_received +foo 18 +connect(localhost,foo,bar,test,MASTER_PORT,MASTER_SOCKET); +ERROR 28000: Access denied for user 'foo'@'localhost' (using password: YES) +select user, bytes_received from information_schema.user_statistics where user = 'foo'; +user bytes_received +foo 18 +drop user foo@localhost; +set global userstat=0; diff --git a/mysql-test/t/userstat-badlogin-4824.test b/mysql-test/t/userstat-badlogin-4824.test new file mode 100644 index 00000000000..97370c1d081 --- /dev/null +++ b/mysql-test/t/userstat-badlogin-4824.test @@ -0,0 +1,33 @@ +# +# MDEV-4824 userstats - wrong user statistics +# +--source include/not_embedded.inc + +create user foo@localhost identified by 'foo'; +flush user_statistics; +set global userstat=1; + +connect(foo, localhost, foo, foo); +select 1; +disconnect foo; +connection default; + +# wait for user_statistics changes to become visible +let $wait_condition= select count(*) = 1 from information_schema.processlist; +--source include/wait_condition.inc + +# 41 is for ps-procotol +--replace_result 41 18 +select user, bytes_received from information_schema.user_statistics where user = 'foo'; + +--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT +--error ER_ACCESS_DENIED_ERROR +connect(foo, localhost, foo, bar); + +connection default; + +--replace_result 41 18 +select user, bytes_received from information_schema.user_statistics where user = 'foo'; + +drop user foo@localhost; +set global userstat=0; diff --git a/sql/event_data_objects.cc b/sql/event_data_objects.cc index c41194d1a8d..dfd35583581 100644 --- a/sql/event_data_objects.cc +++ b/sql/event_data_objects.cc @@ -1337,7 +1337,7 @@ Event_job_data::execute(THD *thd, bool drop) DBUG_ENTER("Event_job_data::execute"); - mysql_reset_thd_for_next_command(thd, 0); + mysql_reset_thd_for_next_command(thd); /* MySQL parser currently assumes that current database is either diff --git a/sql/log_event.cc b/sql/log_event.cc index 928c65f3e44..e5af1e3a4af 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -5353,7 +5353,7 @@ int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli, */ lex_start(thd); thd->lex->local_file= local_fname; - mysql_reset_thd_for_next_command(thd, 0); + mysql_reset_thd_for_next_command(thd); if (!use_rli_only_for_errors) { @@ -7353,7 +7353,7 @@ int Append_block_log_event::do_apply_event(Relay_log_info const *rli) as the present method does not call mysql_parse(). */ lex_start(thd); - mysql_reset_thd_for_next_command(thd, 0); + mysql_reset_thd_for_next_command(thd); /* old copy may exist already */ mysql_file_delete(key_file_log_event_data, fname, MYF(0)); if ((fd= mysql_file_create(key_file_log_event_data, @@ -8308,7 +8308,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) we need to do any changes to that value after this function. */ lex_start(thd); - mysql_reset_thd_for_next_command(thd, 0); + mysql_reset_thd_for_next_command(thd); /* The current statement is just about to begin and has not yet modified anything. Note, all.modified is reset diff --git a/sql/log_event_old.cc b/sql/log_event_old.cc index 244999fc431..4ad07793973 100644 --- a/sql/log_event_old.cc +++ b/sql/log_event_old.cc @@ -87,7 +87,7 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info we need to do any changes to that value after this function. */ lex_start(ev_thd); - mysql_reset_thd_for_next_command(ev_thd, 0); + mysql_reset_thd_for_next_command(ev_thd); /* This is a row injection, so we flag the "statement" as diff --git a/sql/sql_class.cc b/sql/sql_class.cc index be1e24c61c1..63a5af3a707 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1260,6 +1260,7 @@ void THD::init(void) reset_current_stmt_binlog_format_row(); bzero((char *) &status_var, sizeof(status_var)); bzero((char *) &org_status_var, sizeof(org_status_var)); + start_bytes_received= 0; if (variables.sql_log_bin) variables.option_bits|= OPTION_BIN_LOG; diff --git a/sql/sql_class.h b/sql/sql_class.h index dab57a680f3..e9665959d04 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1556,7 +1556,7 @@ public: /* Slave applier execution context */ Relay_log_info* rli_slave; - void reset_for_next_command(bool calculate_userstat); + void reset_for_next_command(); /* Constant for THD::where initialization in the beginning of every query. diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2d4293fc1d7..e9adc3b669f 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -717,7 +717,6 @@ bool do_command(THD *thd) net_new_transaction(net); - /* Save for user statistics */ thd->start_bytes_received= thd->status_var.bytes_received; @@ -926,6 +925,14 @@ bool dispatch_command(enum enum_server_command command, THD *thd, if (!(server_command_flags[command] & CF_SKIP_QUESTIONS)) statistic_increment(thd->status_var.questions, &LOCK_status); + /* Copy data for user stats */ + if ((thd->userstat_running= opt_userstat_running)) + { + thd->start_cpu_time= my_getcputime(); + memcpy(&thd->org_status_var, &thd->status_var, sizeof(thd->status_var)); + thd->select_commands= thd->update_commands= thd->other_commands= 0; + } + /** Clear the set of flags that are expected to be cleared at the beginning of each command. @@ -1176,7 +1183,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, break; } packet= arg_end + 1; - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); lex_start(thd); /* Must be before we init the table list. */ if (lower_case_table_names) @@ -5446,15 +5453,15 @@ bool my_yyoverflow(short **yyss, YYSTYPE **yyvs, ulong *yystacksize) @todo Call it after we use THD for queries, not before. */ -void mysql_reset_thd_for_next_command(THD *thd, my_bool calculate_userstat) +void mysql_reset_thd_for_next_command(THD *thd) { - thd->reset_for_next_command(calculate_userstat); + thd->reset_for_next_command(); } -void THD::reset_for_next_command(bool calculate_userstat) +void THD::reset_for_next_command() { THD *thd= this; - DBUG_ENTER("mysql_reset_thd_for_next_command"); + DBUG_ENTER("THD::reset_for_next_command"); DBUG_ASSERT(!thd->spcont); /* not for substatements of routines */ DBUG_ASSERT(! thd->in_sub_stmt); DBUG_ASSERT(thd->transaction.on); @@ -5500,14 +5507,6 @@ void THD::reset_for_next_command(bool calculate_userstat) thd->sent_row_count= thd->examined_row_count= 0; thd->accessed_rows_and_keys= 0; - /* Copy data for user stats */ - if ((thd->userstat_running= calculate_userstat)) - { - thd->start_cpu_time= my_getcputime(); - memcpy(&thd->org_status_var, &thd->status_var, sizeof(thd->status_var)); - thd->select_commands= thd->update_commands= thd->other_commands= 0; - } - thd->query_plan_flags= QPLAN_INIT; thd->query_plan_fsort_passes= 0; @@ -5716,7 +5715,7 @@ void mysql_parse(THD *thd, char *rawbuf, uint length, FIXME: cleanup the dependencies in the code to simplify this. */ lex_start(thd); - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); if (query_cache_send_result_to_client(thd, rawbuf, length) <= 0) { @@ -5819,7 +5818,7 @@ bool mysql_test_parse_for_slave(THD *thd, char *rawbuf, uint length) if (!(error= parser_state.init(thd, rawbuf, length))) { lex_start(thd); - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); if (!parse_sql(thd, & parser_state, NULL) && all_tables_not_ok(thd, lex->select_lex.table_list.first)) diff --git a/sql/sql_parse.h b/sql/sql_parse.h index 6d47e648be2..d1d6458d22c 100644 --- a/sql/sql_parse.h +++ b/sql/sql_parse.h @@ -83,7 +83,7 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length); void mysql_init_select(LEX *lex); void mysql_parse(THD *thd, char *rawbuf, uint length, Parser_state *parser_state); -void mysql_reset_thd_for_next_command(THD *thd, my_bool calculate_userstat); +void mysql_reset_thd_for_next_command(THD *thd); bool mysql_new_select(LEX *lex, bool move_down); void create_select_for_variable(const char *var_name); void create_table_set_open_action_and_adjust_tables(LEX *lex); diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index a0ec07b9db2..0446e8bf18c 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2269,7 +2269,7 @@ void mysqld_stmt_prepare(THD *thd, const char *packet, uint packet_length) DBUG_PRINT("prep_query", ("%s", packet)); /* First of all clear possible warnings from the previous command */ - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); if (! (stmt= new Prepared_statement(thd))) goto end; /* out of memory: error is set in Sql_alloc */ @@ -2659,7 +2659,7 @@ void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length) packet+= 9; /* stmt_id + 5 bytes of flags */ /* First of all clear possible warnings from the previous command */ - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); if (!(stmt= find_prepared_statement(thd, stmt_id))) { @@ -2758,7 +2758,7 @@ void mysqld_stmt_fetch(THD *thd, char *packet, uint packet_length) DBUG_ENTER("mysqld_stmt_fetch"); /* First of all clear possible warnings from the previous command */ - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); status_var_increment(thd->status_var.com_stmt_fetch); if (!(stmt= find_prepared_statement(thd, stmt_id))) @@ -2818,7 +2818,7 @@ void mysqld_stmt_reset(THD *thd, char *packet) DBUG_ENTER("mysqld_stmt_reset"); /* First of all clear possible warnings from the previous command */ - mysql_reset_thd_for_next_command(thd, opt_userstat_running); + mysql_reset_thd_for_next_command(thd); status_var_increment(thd->status_var.com_stmt_reset); if (!(stmt= find_prepared_statement(thd, stmt_id)))