Re: Fwd: bk commit into 5.1 tree (anozdrin:1.2671) BUG#32148

[Andrei Elkin]

Alexander, hello.

> Hi,
>
> this is a patch.
>
> Thanks.
>
>

The patch is okay.
I'd say it explicit that a reliable test case is impossible to write.

Thanks for fixing!

Andrei


> ----------  Forwarded Message  ----------
>
> Subject: Fwd: bk commit into 5.1 tree (anozdrin:1.2671) BUG#32148
> Date: Thursday 29 November 2007 15:08
> From: Alexander Nozdrin <alik@stripped>
> To: commits@stripped
>
> Below is the list of changes that have just been committed into a local
> 5.1 repository of alik. When alik does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-11-29 14:57:12+03:00, anozdrin@ibm. +2 -0
>   A patch for BUG#32148: killing a query may be ineffective.
>   
>   The problem was that THD::killed was reset after a command was
>   read from the socket, but before it was actually handled. That lead
>   to a race: if another KILL statement was issued for this connection
>   in the middle of reading from the socket and processing a command,
>   THD::killed state would be cleaned.
>   
>   The fix is to move this cleanup into net_send_error() function.
>   
>   A sample test case exists in binlog_killed.test:
>     - connection 1: start a new transaction on table t1;
>     - connection 2: send query to the server (w/o waiting for the
>       result) to update data in table t1 -- this query will be blocked
>       since there is unfinished transaction;
>     - connection 1: kill query in connection 2 and finish the transaction;
>     - connection 2: get result of the previous query -- it should be
>       the "query-killed" error.
>   
>   This test however contains race condition, which can not be fixed
>   with the current protocol: there is no way to guarantee, that the
>   server will receive and start processing the query in connection 2
>   (which is intended to get blocked) before the KILL command (sent in
>   the connection 1) will arrive. In other words, there is no way to
>   ensure that the following sequence will not happen:
>   
>     - connection 1: start a new transaction on table t1;
>     - connection 1: kill query in connection 2 and finish the transaction;
>     - connection 2: send query to the server (w/o waiting for the
>       result) to update data in table t1 -- this query will be blocked
>       since there is unfinished transaction;
>     - connection 2: get result of the previous query -- the query will
>       succeed.
>
>   sql/protocol.cc@stripped, 2007-11-29 14:57:09+03:00, anozdrin@ibm. +6 -0
>     Move thd->killed cleanup from dispatch_command() to net_send_error().
>
>   sql/sql_parse.cc@stripped, 2007-11-29 14:57:09+03:00, anozdrin@ibm. +1 -6
>     Move thd->killed cleanup from dispatch_command() to net_send_error().
>
> diff -Nrup a/sql/protocol.cc b/sql/protocol.cc
> --- a/sql/protocol.cc	2007-11-01 00:10:56 +03:00
> +++ b/sql/protocol.cc	2007-11-29 14:57:09 +03:00
> @@ -76,6 +76,12 @@ void net_send_error(THD *thd, uint sql_e
>  
>    DBUG_ASSERT(!thd->spcont);
>  
> +  if (thd->killed == THD::KILL_QUERY || thd->killed == THD::KILL_BAD_DATA)
> +  {
> +    thd->killed= THD::NOT_KILLED;
> +    thd->mysys_var->abort= 0;
> +  }
> +
>    if (net && net->no_send_error)
>    {
>      thd->clear_error();
> diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
> --- a/sql/sql_parse.cc	2007-11-28 19:08:25 +03:00
> +++ b/sql/sql_parse.cc	2007-11-29 14:57:09 +03:00
> @@ -788,12 +788,7 @@ bool dispatch_command(enum enum_server_c
>    NET *net= &thd->net;
>    bool error= 0;
>    DBUG_ENTER("dispatch_command");
> -
> -  if (thd->killed == THD::KILL_QUERY || thd->killed == THD::KILL_BAD_DATA)
> -  {
> -    thd->killed= THD::NOT_KILLED;
> -    thd->mysys_var->abort= 0;
> -  }
> +  DBUG_PRINT("info",("packet: '%*.s'; command: %d", packet_length, packet,
> command));
>  
>    thd->command=command;
>    /*
>
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1
>
>
>
> -------------------------------------------------------
>
> -- 
> Alexander Nozdrin, Software Developer
> MySQL AB, Moscow, Russia, www.mysql.com