Re: bk commit into 5.0 tree (antony:1.2511) BUG#25679

[Sergei Golubchik]

Hi!

On Jul 16, Antony T Curtis wrote:
> On Mon, 2007-07-16 at 22:22 +0200, Sergei Golubchik wrote:
> > On Jul 09, antony@stripped wrote:
> > > ChangeSet@stripped, 2007-07-09 21:54:16-07:00, antony@stripped +4 -0
> > >   Bug#25679
> > >     "Federated Denial of Service"
> > >     Federated storage engine used to attempt to open connections within
> > >     the ::create() and ::open() methods which are invoked while LOCK_open
> > >     mutex is being held by mysqld. As a result, no other connection can
> > >     open tables during the network processing. Long DNS lookup times
> > >     would stall mysqld's operation and a rogue connection string which
> > >     connects to a remote server which simply stalls during handshake can
> > >     stall mysqld for much longer periods of time.
> > >     This patch moves the connection-opening much later to when LOCK_open
> > >     mutex is not being held.
> > > 
> > >   sql/ha_federated.cc@stripped, 2007-07-09 21:54:03-07:00, antony@stripped
> +136 -156
> > >     bug25679
> > >       remove function check_foreign_data_source()
> > >       ha_federated::open() no longer opens the federated connection.
> > >       ha_federated::create() no longer opens and tests connection.
> > >       ha_federated::real_query() opens and tests presence of table.
> > 
> > Uhm, I don't know.
> > 
> > We have enough troubles with MERGE tables that don't check the
> > correctness until the first SELECT.
> > 
> > I personally would be ok with such a behaviour, but apparently many
> > users aren't, and I think it'd be unwise to implement this unpopular
> > approach.
> 
> If we must check the connection within ::open() or ::create() and that
> they are called while LOCK_open is held, then this bug must be marked
> as "Won't fix" because we need to know the results of a network
> operation which takes a non-trivial amount of time to complete.

No, not that. (see below)
 
> If we intend to have ::open() or ::create() called without LOCK_open
> being held, then the earlier patch should be reconsidered even though
> it in effect introduces a change in API... although, a change towards
> a situation which most people would have already expected to be in
> place.  Not many people currently realise that LOCK_open is held
> during these operations and the simple fact that the coding guidelines
> have had storage engine implementors create their own mutex for use
> during calls to ::open() would indicate that reentrant behaviour was
> originally intended.

Yes, this is what I would prefer.

> Until there is some resolution as to which avenue will be used to
> solve this bug, perhaps it should simply be marked as "To be fixed
> later"?

We discussed that on irc, and seems like the resolution was - your
latest idea is ok, push it in 5.0 (when approved), and when Dmitry will
fix locking in 5.2 (WL#3983) the server will be fixed using the first
approach (don't keep LOCK_open while calling the handler methods).

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Radlkoferstr. 2, D-81373 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140