List:Commits« Previous MessageNext Message »
From:Ingo Strüwing Date:May 6 2009 8:13am
Subject:Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780
View as plain text  
Hi Konstantin,

Konstantin Osipov, 05.05.2009 22:21:

> * Ingo Strüwing <Ingo.Struewing@stripped> [09/05/05 23:35]:
...
> your patch is bound to solve the present day trouble only unless
> you write an instruction to developers that they can follow
> when writing new code: an instruction how to write "killable" and
> race-free code.


Where shall this instruction got to? Where is the old instruction?

I tried to include the instruction at every wait condition. I think that
a developer, who wants to write "killable" code, will look at other
places where similar things are done. So I hope, he will find one of the
repeated comments. But I've already received the comment that these many
repetitions don't seem useful. Now I'm gathering opinions, which place
might be optimal for a single copy.

> 
> When you move the checks for thd->killed into critical section,
> you change the conceptual model of thd->enter_cond().
> According to the old conceptual model it was unnecessary, since 
> thd->awake cancels the wait on the condition (at least such was
> the pitch).


It does still cancel the wait on the condition. I don't understand what
you mean with me to "change the conceptual model of thd->enter_cond()".
As far as I understand the "conceptual model of thd->enter_cond()", it
is not affected in its main feature: Making it possible to kick a thread
out of a wait condition in an exceptional way. thd->killed is just one
way to transport the exception itself. The way, how to check it, may be
seen as a peripheral recommendation. If you think, I'm wrong, please
explain in detail, what is the conceptual model is and why the
(additional) check for thd->killed changes the model.

> 
> thd->killed  has never been too reliable. It wasn't designed to
> be. In real life a user can send the signal twice without too much 
> trouble. But the way it worked was simple and easy to understand.


My aim is to make thd->killed more reliable. If only to support
regression testing.

I'm sorry to hear that you don't find it simple and easy to understand
that the check for thd->killed should not (only) be done before
enter_cond(), but (also) after it.

> 
> So, before we proceed, can we zoom out from the code for a minute
> and discuss:
> 
>  - what was the old model for thd->killed. What was wrong with it?


I didn't know that we had a "model" for thd->killed. Can you please
point me at its written instruction/documentation?

The way thd->killed was examined, was unnecessary unreliable. Checking
it before enter_cond() only allows for a race condition.

>  - what is the new model. How is it better?

Checking thd->killed (also) after enter_cond() makes it reliable
regarding wait conditions. IMHO this is a simple and inexpensive
addition, which improves reliability of thd->killed.

I agree that this may not be desirable for real-life applications as
they (the user) can repeat the kill request.

But it improves the testability of KILL. Without this change we probably
need to live with a failing kill.test, or not (regression-)testing KILL
at all.

Regards
Ingo
-- 
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder,   Wolfgang Engels,   Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring   HRB München 161028
Thread
bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Struewing25 Feb
  • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Davi Arnaut5 May
    • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing5 May
      • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Konstantin Osipov5 May
        • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing6 May
          • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Konstantin Osipov6 May
            • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing6 May
              • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Konstantin Osipov6 May
                • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing7 May
                  • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing12 May
              • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Davi Arnaut6 May
      • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Davi Arnaut6 May
        • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing6 May
          • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Konstantin Osipov6 May
            • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Ingo Strüwing7 May
          • Re: bzr commit into mysql-6.0 branch (ingo.struewing:2779) Bug#37780Davi Arnaut7 May