FreeTDS Development Group

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

On Sun, 11 Jan 2004, Alex Kiesel <alex.kiesel AT document-root.de> wrote:
> 
> attached is another patch which fixes freetds' behaviour when cancelling
> a query. At the moment, when cancelling a query, a cancel packet is
> being sent (that is, an empty packet with the headers out_flag set to
> 0x06 = attention packet). After this, all packets are read until an
> acknowledgement of the cancel has been received.
> 
> Now, the wire does not contain more data - but as the cancel has been
> sent asynchroneously, a client can (and e.g. sqsh often does) call one
> of ct_results, ct_fetch and probably more. Thus, this all comes down to
> goodread(); freetds tries to read data from the wire where there is no
> data available. Due to the timeout handling code - this is being done
> over and over again.
> 
> My patch changes this to return 0 when the socket is not ready for read
> operations. This lets the client leave the result-handling code and
> return to "normal" operation. It can start a new query.
> 
> After applying this patch, all ctlib- and tds-unittests succeed.
> dblib-tests t0013 and t0014 fail; this is a pure timeout problem.
> Setting the sockets read timeout to something higher than 1 second fixes
> both tests - and all tests pass. I have not enough knowledge to judge
> whether this is critical for dblib.
> 
> Could someone have a look at this? It would be great if this could be
> included into 0.62 if the unittests can be fixed by someone...

Alex, thank you for the patch.  

Cancel and timeout handling is some of the trickiest logic in FreeTDS, at
least in my estimation.  I cannot both include it in 0.62 *and* release
0.62 today.  It's a question of delaying the release or excluding the
patch.  Issuing the release is more than simply making sure the unit tests
all pass, after all.  

I branched the code on Saturday, and am working on the release notes and
announcements.  I think the safest thing to do, at this stage in the game,
is to apply your patch to the trunk (not 0.62).  We could include your
patch (and message) as a separate file in the release, to be applied at
the user's discretion.  I'm open to suggestions on that score.  

I think there will be other ramifications to your patch, simple as it is. 
  Certainly, error handling is rickety, and there is some interest in
fixing query timeouts.  And it may well turn out that this area sees some
substantial change, anyway, in 0.63. 

That's my PHB answer.  If more knowledgeable people tell me we should hold
up and include this -- and make the other changes it implies or uncovers
-- in 0.62, then OK, I yield to them.  It will mean a two week delay,
however; I cannot promise the Release Engineering team will be available
again until then.  They have something like 12 hours to express their
opinions.  

--jkl