Hi Jon,
On 11/20/09 8:31 AM, Jon Olav Hauglid wrote:
> #At file:///export/home/z/mysql-6.0-codebase-bugfixing-bug48724/ based on
> revid:jon.hauglid@stripped
>
> 3714 Jon Olav Hauglid 2009-11-20
> Bug #48724 Deadlock between INSERT DELAYED and FLUSH TABLES
>
> The problem was that if the handler thread was killed at the wrong moment
> (using e.g. FLUSH TABLES), this would not be properly noticed by the
> INSERT DELAYED connection thread. It would be stuck waiting for the handler
> thread to lock its table, while the handler thread would be looping, trying
> to get the connection thread to notice the error.
>
> The root of the problem was that insert delayed had an extra variable "dead"
> used to indicate if the handler thread had been killed. This in addition to
> the usual "thd->killed". Most places both were set, but some only set
> "thd->killed". And Delayed_insert::get_local_table() only checked "dead"
> while waiting for the table to be locked.
This does not quite explain what is leading to the bug.
From what I could gather, the DELAYED INSERT thread (handler thread?)
used to notify the session thread via the 'dead' flag. This notification
would happen after a failure to lock the table (due to being killed).
Later down the road, the patch for Bug#45949 introduced a check that
(somewhat rightly) skipped attempts to lock the table if the session was
already killed. Unfortunately, this also skipped the setting of the
'dead' flag and /killed/ the notification schema.
BTW, this need to be based upon the tree that Bug#45949 was pushed to.
> {
> /* Don't copy over "Server shutdown in progress". */
> - if (thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
> + if (!thd.is_error() ||
Why the is_error check is necessary? Please add a comment.
> + thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
> my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0));
> else
> my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(),
> MYF(0));
> @@ -2433,7 +2432,7 @@ static void handle_delayed_insert_impl(T
> break; // Time to die
> }
>
> - if (!di->status&& !di->stacked_inserts)
> + if (!thd->killed&& !di->status&&
> !di->stacked_inserts)
> {
> struct timespec abstime;
> set_timespec(abstime, delayed_insert_timeout);
> @@ -2444,7 +2443,7 @@ static void handle_delayed_insert_impl(T
> thd_proc_info(&(di->thd), "Waiting for INSERT");
>
> DBUG_PRINT("info",("Waiting for someone to insert rows"));
> - while (!thd->killed)
> + while (!thd->killed && !di->status)
Why check Delayed_insert::status? Please add a comment.
Regards,
--
Davi Arnaut
