FreeTDS Development Group

["James K. Lowden" <jklowden AT schemamania.org>]

I thought I'd try to implement dbsettime().  It's not simple, for a few
reasons.  One, I'm not all that familiar with network programming a la
Unix.  Two, it doesn't take a dbproc as an argument: the documentation
says 

        "dbsettime can be called at any time during the application--before or
after a call to dbopen. It takes effect immediately upon being called."  

Because we maintain per-connection timeouts (in TDSSOCKET), that implies
that dbsettime() has to:
        
1.  set a new default for future connections.  
2.  iterate over the open connections, adjusting each one's timeout.  

(a bit tricky in a multithreaded setting).  

That brings me to the third reason it's not simple: the number of timeouts
in TDSCONNECTION.  We have:

$ sed -ne'/tds_connection$/,/TDSCONNECTION/p' include/tds.h |grep timeout 

        TDS_INT connect_timeout;
        TDS_INT query_timeout;
        TDS_INT longquery_timeout;
        int timeout;

I think longquery_timeout is an ODBC thing that we can ignore for now
(although I haven't found anything in the ODBC spec about a "longquery"
timeout, either).  

The real confusion is in how the others interact, not least because there
are 3 variables to control 2 things.  

To be absolutely clear, let us define our terms:

1.  Connection timeout.  How long we'll wait for the server to acknowledge
the formation of a connection.  E.g, how long dbopen() will wait for an
answer.   
2.  Query timeout.  How long we'll wait for a response to a query.  

The code
--------

For starters, in config.c, the TDSCONNECTION members are set thus:

        } else if (!strcmp(option, TDS_STR_TIMEOUT)) {
                if (atoi(value))
                        connection->timeout = atoi(value);
        } else if (!strcmp(option, TDS_STR_CONNTIMEOUT)) {
                if (atoi(value))
                        connection->connect_timeout = atoi(value);

So, 'query_timeout' is not initialized from freetds.conf, but 'timeout'
is.  

In login.c, we have this weird line:

        tds->timeout = (connect_timeout) ? connection->query_timeout : 0;

Weird, first, because nowhere that I know of do we say "if you indicate a
query timeout, it won't be honored unless you indicate a connection
timeout, too.  But, if you set a timeout in freetds.conf and *don't* set a
connection timeout, then the query timeout will will work."  

Weird, second, because afaict that's the only place TDSSOCKET::timeout is
set, and that's the value read.c::goodread() relies on.   

Weird, third, because again afaict connection->query_timeout is used
otherwise only in odbc.c::_SQLExecute to set tds->longquery_timeout.  I
don't see it referenced anywhere else in libtds.  

Also in login.c, we have a function tds_set_timeouts().  No one calls it. 


Proposal
--------

I like the name 'query_timeout' better than 'timeout'.  It's pretty clear,
though, that 'timeout' is older and is used more extensively (but not
quite correctly).  I suggest we:

1.  Delete 'query_timeout' from the structures, shifting all references to
use 'timeout' instead.   It looks to me like that's the intent anyway.  

2.  Fix login.c.  Use 'connect_timeout' for the initial connection, and
initialize 'timeout' according to the default from freetds.conf or e.g.
dbsettime().  

3.  Rename 'timeout' to 'query_timeout'.  Add "query timeout" as a
freetds.conf entry, while retaining "timeout" as an undocumented (but
working) alias.  

4.  Delete tds_set_timeouts().  Add dbsettime().  

For the record, Microsoft defines DBSETLTIME(), a per-login connect
timeout.  We don't support it, and no one's asked for it.  To support it,
tdsdbopen() would need to know to preserve the TDSLOGIN::timeout when
reading freetds.conf.  

I'm especially interested in any corrections to my understanding.  All
suggestions welcome, naturally.  

--jkl