Re: [Maria-developers] 1ed4683: MDEV-8260 : Issues related to concurrent CTAS
Hi, Nirbhay! On Jun 02, Nirbhay Choubey wrote:
revision-id: 1ed46831ed3a1c842b45f7819edd48a233080ce4 parent(s): 6d5b723bdc3e04978619b9673fca266e0426916f committer: Nirbhay Choubey branch nick: 10.0-galera.ctas timestamp: 2015-06-02 17:59:32 -0400 message:
MDEV-8260 : Issues related to concurrent CTAS
* Wait for aborted thd (victim) to release MDL locks * Skip aborting an already aborted thd * Defer setting OK status in case of CTAS * Minor cosmetic changes * Added a test case
So, I'm only reviewing non-wsrep part, ok?
@@ -3735,24 +3757,16 @@ bool select_insert::send_eof() table->file->print_error(error,MYF(0)); DBUG_RETURN(1); } - char buff[160]; - if (info.ignore) - sprintf(buff, ER(ER_INSERT_INFO), (ulong) info.records, - (ulong) (info.records - info.copied), - (long) thd->get_stmt_da()->current_statement_warn_count()); - else - sprintf(buff, ER(ER_INSERT_INFO), (ulong) info.records, - (ulong) (info.deleted+info.updated), - (long) thd->get_stmt_da()->current_statement_warn_count()); - row_count= info.copied + info.deleted + - ((thd->client_capabilities & CLIENT_FOUND_ROWS) ? - info.touched : info.updated); - id= (thd->first_successful_insert_id_in_cur_stmt > 0) ? - thd->first_successful_insert_id_in_cur_stmt : - (thd->arg_of_last_insert_id_function ? - thd->first_successful_insert_id_in_prev_stmt : - (info.copied ? autoinc_value_of_last_inserted_row : 0)); - ::my_ok(thd, row_count, id, buff); + + /* + Avoid setting OK status for CREATE .. SELECT as it may fail during commit. + */ + if (!((thd->lex->sql_command == SQLCOM_CREATE_TABLE) && + ((&thd->lex->select_lex)->item_list.elements))) { + prepare_ok_info(thd); + ::my_ok(thd, ok.row_count, ok.id, ok.message); + } + DBUG_RETURN(0); }
I think we've discussed a slightly different fix, where the parent class (select_insert) wouldn't need to check thd->lex->sql_command. Like, create two methods select_insert::prepare_eof select_insert::send_ok_packet the first method will have everything of the current select_insert::send_eof but without ::my_ok part. Then select_insert::send_eof() will be just bool select_insert::send_eof() { return prepare_eof() || send_ok_packet(); } and select_create::send_eof() will do prepare_eof(), commit and other stuff, send_ok_packet(). In this case you won't need to test for thd->lex->sql_command in the parent class. And you won't need prepare_ok_info(), it'll be inside send_ok_packet(). Regards, Sergei
Hi Serg, On Mon, Jun 8, 2015 at 11:07 AM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nirbhay!
On Jun 02, Nirbhay Choubey wrote:
revision-id: 1ed46831ed3a1c842b45f7819edd48a233080ce4 parent(s): 6d5b723bdc3e04978619b9673fca266e0426916f committer: Nirbhay Choubey branch nick: 10.0-galera.ctas timestamp: 2015-06-02 17:59:32 -0400 message:
MDEV-8260 : Issues related to concurrent CTAS
* Wait for aborted thd (victim) to release MDL locks * Skip aborting an already aborted thd * Defer setting OK status in case of CTAS * Minor cosmetic changes * Added a test case
So, I'm only reviewing non-wsrep part, ok?
Ok. :) ..skip..
I think we've discussed a slightly different fix,where the parent class (select_insert) wouldn't need to check thd->lex->sql_command. Like, create two methods
select_insert::prepare_eof select_insert::send_ok_packet
the first method will have everything of the current select_insert::send_eof but without ::my_ok part. Then select_insert::send_eof() will be just
bool select_insert::send_eof() { return prepare_eof() || send_ok_packet(); }
and select_create::send_eof() will do prepare_eof(), commit and other stuff, send_ok_packet().
In this case you won't need to test for thd->lex->sql_command in the parent class. And you won't need prepare_ok_info(), it'll be inside send_ok_packet().
I have redone the patch with your suggestions. http://lists.askmonty.org/pipermail/commits/2015-June/008023.html Please have a look. Thanks! -- Nirbhay
Regards, Sergei
participants (2)
-
Nirbhay Choubey
-
Sergei Golubchik