From 76fa1d4381586ca55ddfb9869c04a0ef794a92a6 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 12 Sep 2006 15:42:13 +0200 Subject: Fixing problems I identified in my auto_increment work pushed in July (as part of the auto_increment cleanup of WL#3146; let's not be sad, that monster push still removed serious bugs): one problem with INSERT DELAYED (unexpected interval releases), one with stored functions (wrong auto_inc binlogging). These bugs were not released. mysql-test/extra/binlog_tests/binlog_insert_delayed.test: more tests of binlogging of INSERT DELAYED: with multi-row INSERTs. I identified why sleeps are needed to get a repeatable row-based binlogged: because without sleeps rows sometimes get groupped and so generate different row based events. mysql-test/extra/rpl_tests/rpl_foreign_key.test: don't forget to drop tables on slave too, otherwise it leaves an orphan innodb table leading to rpl_insert_id failing sometimes (like in pushbuild "sapsrv2 -max"). mysql-test/extra/rpl_tests/rpl_insert_id.test: testing that if some statement does not update any row, it does not pollute the auto_inc binlog variables of the next statement; the test has to use stored procedures because with plain statements, mysql_reset_thd_for_next_command() does the resetting (and thus there is no problem); mysql_reset_thd_for_next_command() is not called inside routines. mysql-test/r/binlog_row_binlog.result: result additions mysql-test/r/binlog_statement_insert_delayed.result: result additions mysql-test/r/binlog_stm_binlog.result: result additions mysql-test/r/rpl_insert_id.result: result additions mysql-test/r/rpl_loaddata.result: With the change to log.cc reverted, the result changes and is better: the change to log.cc had caused some INSERT_ID events to disappear though they were necessary (but testsuite could not catch that because it's single-threaded). mysql-test/r/rpl_ndb_insert_ignore.result: NDB is now like other engines regarding INSERT IGNORE: autoincrement values which caused a duplicate key are re-used for next row, not lost. rpl_ndb_insert_ignore.result is now identical to rpl_insert_ignore.result. sql/log.cc: LOAD DATA INFILE is binlogged as several events, and the last of them must have the auto_inc id. So it's wrong to reset the auto_inc id after every binlog write (because then it's lost after the first event of LOAD DATA INFILE and so missing for the last one)/ Another problem: MYSQL_LOG::write() is not always called (for example if no row was updated), so we were missing reset in some cases. sql/sp_head.cc: SELECT func1(),func2() generates two binlog events, so needs to clear auto_increment binlog variables after each binlog event (it would be more natural to clear them in the log write code, but LOAD DATA INFILE would suffer from this see the cset comment for log.cc). Without the clearing, the problem is: > exec func1() >> call cleanup_after_query() (which does not clear our vars here) >> binlog SELECT func1() < > exec func2() and so SELECT func2() is binlogged with the auto_inc of SELECT func1(). sql/sql_class.cc: after every statement we should clear auto_inc variables used for binlogging, except if this was a function/trigger (in which case it may be "INSERT SELECT func()", where the cleanup_after_query() executed in func() should not reset the auto_inc binlog variables as they'll be necessary when binlogging the INSERT SELECT later). sql/sql_insert.cc: - as INSERT DELAYED uses the same TABLE object as the delayed_insert system thread, we should not call ha_release_auto_increment() from INSERT DELAYED (and btw it's logical as we reserve nothing as we don't perform the insert). Calling the function caused us to release values being used by the delayed_insert thread. So I do the call only if this is a non-DELAYED INSERT. - Assuming two INSERT DELAYED which get grouped by the delayed_insert thread, the second may use values reserved by the first, which is ok per se, but is a problem in statement-based binlogging: the 2nd INSERT gets binlogged with the "interval start" value of the first INSERT (=> duplicate error in slave). - no reason to ha_release_auto_increment() after every inserted row in INSERT SELECT; more efficient to do it only when the statement ends sql/sql_parse.cc: a comment --- sql/log.cc | 3 --- sql/sp_head.cc | 13 ++++++++++++- sql/sql_class.cc | 6 ++++++ sql/sql_insert.cc | 21 ++++++++++++++++++--- sql/sql_parse.cc | 9 +++++++-- 5 files changed, 43 insertions(+), 9 deletions(-) (limited to 'sql') diff --git a/sql/log.cc b/sql/log.cc index 0336a11d3ad..d15a23de51e 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -3416,9 +3416,6 @@ bool MYSQL_BIN_LOG::write(Log_event *event_info) } } } - /* Forget those values, for next binlogger: */ - thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; - thd->auto_inc_intervals_in_cur_stmt_for_binlog.empty(); } /* diff --git a/sql/sp_head.cc b/sql/sp_head.cc index fc4aa5e26d6..f9c4cc8c68f 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -794,7 +794,7 @@ int cmp_splocal_locations(Item_splocal * const *a, Item_splocal * const *b) This set is produced by tracking user variable reads during statement execution. - Fo SPs, this has the following implications: + For SPs, this has the following implications: 1) thd->user_var_events may contain events from several SP statements and needs to be valid after exection of these statements was finished. In order to achieve that, we @@ -807,6 +807,14 @@ int cmp_splocal_locations(Item_splocal * const *a, Item_splocal * const *b) reset_dynamic(&thd->user_var_events); calls in several different places. (TODO cosider moving this into mysql_bin_log.write() function) + + 4.2 Auto_increment storage in binlog + + As we may write two statements to binlog from one single logical statement + (case of "SELECT func1(),func2()": it is binlogged as "SELECT func1()" and + then "SELECT func2()"), we need to reset auto_increment binlog variables + after each binlogged SELECT. Otherwise, the auto_increment value of the + first SELECT would be used for the second too. */ @@ -1526,6 +1534,9 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, "failed to reflect this change in the binary log"); } reset_dynamic(&thd->user_var_events); + /* Forget those values, in case more function calls are binlogged: */ + thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; + thd->auto_inc_intervals_in_cur_stmt_for_binlog.empty(); } } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 838d91f9c31..35b527584dc 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -634,6 +634,12 @@ bool THD::store_globals() void THD::cleanup_after_query() { + if (!in_sub_stmt) /* stored functions and triggers are a special case */ + { + /* Forget those values, for next binlogger: */ + stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; + auto_inc_intervals_in_cur_stmt_for_binlog.empty(); + } if (first_successful_insert_id_in_cur_stmt > 0) { /* set what LAST_INSERT_ID() will return */ diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 92cad471cf2..8b51ccef9f9 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -563,7 +563,6 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, free_underlaid_joins(thd, &thd->lex->select_lex); joins_freed= TRUE; - table->file->ha_release_auto_increment(); /* Now all rows are inserted. Time to update logs and sends response to @@ -582,6 +581,11 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, else #endif { + /* + Do not do this release if this is a delayed insert, it would steal + auto_inc values from the delayed_insert thread as they share TABLE. + */ + table->file->ha_release_auto_increment(); if (!thd->prelocked_mode && table->file->ha_end_bulk_insert() && !error) { table->file->print_error(my_errno,MYF(0)); @@ -2106,6 +2110,16 @@ bool delayed_insert::handle_inserts(void) thd.start_time=row->start_time; thd.query_start_used=row->query_start_used; + /* + To get the exact auto_inc interval to store in the binlog we must not + use values from the previous interval (of the previous rows). + */ + bool log_query= (row->log_query && row->query.str != NULL); + if (log_query) + { + table->file->ha_release_auto_increment(); + thd.auto_inc_intervals_in_cur_stmt_for_binlog.empty(); + } thd.first_successful_insert_id_in_prev_stmt= row->first_successful_insert_id_in_prev_stmt; thd.stmt_depends_on_first_successful_insert_id_in_prev_stmt= @@ -2146,7 +2160,7 @@ bool delayed_insert::handle_inserts(void) table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); } - if (row->log_query && row->query.str != NULL && mysql_bin_log.is_open()) + if (log_query && mysql_bin_log.is_open()) { /* If the query has several rows to insert, only the first row will come @@ -2542,7 +2556,6 @@ bool select_insert::send_data(List &values) table->next_number_field->reset(); } } - table->file->ha_release_auto_increment(); DBUG_RETURN(error); } @@ -2616,6 +2629,7 @@ void select_insert::send_error(uint errcode,const char *err) } } ha_rollback_stmt(thd); + table->file->ha_release_auto_increment(); DBUG_VOID_RETURN; } @@ -2666,6 +2680,7 @@ bool select_insert::send_eof() } if ((error2=ha_autocommit_or_rollback(thd,error)) && ! error) error=error2; + table->file->ha_release_auto_increment(); if (error) { table->file->print_error(error,MYF(0)); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2aceccd1c92..0bee8fa0c53 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5861,9 +5861,14 @@ void mysql_reset_thd_for_next_command(THD *thd) DBUG_ASSERT(!thd->spcont); /* not for substatements of routines */ thd->free_list= 0; thd->select_number= 1; + /* + Those two lines below are theoretically unneeded as + THD::cleanup_after_query() should take care of this already. + */ thd->auto_inc_intervals_in_cur_stmt_for_binlog.empty(); - thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= - thd->query_start_used= 0; + thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; + + thd->query_start_used= 0; thd->is_fatal_error= thd->time_zone_used= 0; thd->server_status&= ~ (SERVER_MORE_RESULTS_EXISTS | SERVER_QUERY_NO_INDEX_USED | -- cgit v1.2.1