Patch for Bug 608032 - MySQL timeout and no attempt reconnect

[Geert Janssens]

On Thursday 4 February 2010, Tom Van Braeckel wrote:
> > On Thursday 4 February 2010, Tom Van Braeckel wrote:
> > > Hi guys,
> > >
> > > Here's another patch - my first *code* patch to GnuCash !
> > >
> > > Rationale: When we try to open a database transaction, and the database
> > > reports that the "server has gone away", we try to reconnect before
> >
> > failing
> >
> > > hard.
> >
> > Hi,
> >
> > Thanks you for your patch. It looks like a good start, but to my limited
> > knowledge of the sql backend, it seems incomplete.
> >
> > Here are my thoughts on the patch:
> > * you test for an error by comparing with a string. I think it would
> > probably
> > be safer and definitely be more efficient to simply test for the error
> > number
> > returned by dbi_conn_error. It's possible that the error strings returned
> > by
> > MySQL are translated into other languages on other systems, so your
> > string wouldn't always match.
> 
> Thanks for the feedback !
> You're right - I've corrected this in the attached patch and cleaned it up
>  a bit - hope you like it :-)
> 
> Note that such string-based error checking is also done in other places
> (such as the "mysql_error_fn" function in gnc-backend-dbi.c), perhaps I'll
> rectify that in a future patch.
> 
Hmm, after giving it some more thought, I think I know why Phil used the 
string in the first place: MySQL is not the only database supported by dbi. 
Error code 2006 in MySQL may mean something different in SQLite or Postgresql.

So using the number only, we risk that a Postgres user gets unexpected 
behavior if he hits a 2006 error.

I find your reference to mysql_error_fn very interesting. This is the function 
that is called whenever something goes wrong when accessing the database, and 
it is specific to mysql. I wonder if you can't reuse this function instead to 
handle the reconnection.

> > * what happens if not only the connection had timed out, but something
> > else changed as well (an administrator changed the login password for
> > example) ? I
> > mean you don't check the results of dbi_connect or the second
> > dbi_conn_queryf.
> 
> Good question. When that happens, GnuCash will behave as it did before
> ("Could not save..." error). There's room for improvement there (such as
> asking for the new login password) but I feel like that's a different
>  issue.
> 
I suppose that's acceptable.

> > * I also think there may be more useful places to check for a connection
> > timeout, not only at the beginning of transactions. I found several
> > occurences
> > of dbi_conn_query and dbi_conn_queryf some of which could be called
> > passed a
> > timeout.
> 
> Well, normally these timeouts have high values (8 hours in a default MySQL
> installation). Since a database query normally begins with a "begin
> transaction" (as it should) and a transaction is typically very short
>  lived, I don't think the connection will ever time out between "begin
>  transaction" and an actual statement. Unless I'm missing something...
> 
IMHO this is a wrong assumption. Queries that WRITE to the db (update, insert, 
create table,...) should all be enclosed in a transaction. But queries that 
simply read data (SELECT) should not per se. So there's at least one other 
place where a timeout should be tested for.

Just looking in the code, I find functions such as 
conn_execute_select_statement/conn_execute_nonselect_statement which perform 
database actions and are not tested for server is gone errors.

Geert

> > As said in the beginning, I'm not the dbi expert, so maybe my comments
> > are off
> > mark or plain silly.
> >
> > > I've also attached it to the bug report, and taken the liberty to add
> > >  myself to the AUTHORS file.
> > > I hope that's customary here, and I apologise if it's not...
> >
> > I think this is quite ok. I would just like to ask you to put your name
> > in alpabetical order which I think would put you between Richard
> > -Gilligan- Uschold and Matthew Vanecek.
> 
> Done :-)
> 
> > > Thanks again for this great software,
> >
> > Keep submitting patches !
> >
> > > Tom Van Braeckel
> > > GSM: 0032 (0) 486 63 58 04
> >
> > Geert
> > _______________________________________________
> > gnucash-devel mailing list
> > gnucash-devel at gnucash.org
> > https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>