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

[Antony T Curtis]

On Tue, 2007-07-24 at 20:56 +0200, Sergei Golubchik wrote:
> Hi!
> 
> On Jul 24, Antony T Curtis wrote:
> > On Tue, 2007-07-24 at 19:15 +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.
> > > 
> > > > diff -Nrup a/sql/ha_federated.cc b/sql/ha_federated.cc
> > > > --- a/sql/ha_federated.cc	2007-06-29 15:14:00 -07:00
> > > > +++ b/sql/ha_federated.cc	2007-07-09 21:54:03 -07:00
> > > ....
> > > > +    if (!mysql_real_connect(mysql,
> > > > +                            share->hostname,
> > > > +                            share->username,
> > > > +                            share->password,
> > > > +                            share->database,
> > > > +                            share->port,
> > > > +                            share->socket, 0))
> > > > +    {
> > > > +      mysql_close(mysql);
> > > > +      mysql= NULL;
> > > > +      sql_query.append("database: '");
> > > > +      sql_query.append(share->database);
> > > > +      sql_query.append("'  username: '");
> > > > +      sql_query.append(share->username);
> > > > +      sql_query.append("'  hostname: '");
> > > > +      sql_query.append(share->hostname);
> > > > +      sql_query.append("'");
> > > > +      my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0),
> sql_query.ptr());
> > > > +      remote_error_number= -1;
> > > 
> > > Why you don't use stash_remote_error(), but generates less informative
> > > generic error message ? 
> > 
> > Using same error message as it used to generate in case where connection
> > has failed.
> 
> But there was stash_remote_error() in ha_federated::open() before.

Hmm... looks like I copied this out from the connection test code which
was used by create()

I will correct this in the updated patch.

Regards,

-- 
Antony T Curtis, Senior Software Developer
MySQL Inc, www.mysql.com
SIP: 4468@stripped