FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> 
> > - timeout. New timeout in TDSSOCKET. I'm very again this. There are
> 
> Perhaps I don't fully understand the intricacies of the TDS protocol
> but the reason I included it is that from my perspective the caller
> may want to wait an hour for a query to finish, but they probably
> don't want to wait an hour to be notified if there is a network
> timeout or error? Are the two ideas mutually exclusive? 
> 
> Does anything come down the wire when a very long query is being
> executed? I would expect something needs to in order to stop the
> stack from closing the connection?
> 

Do not confuse errors. If read/select fails it means a connection
problem or a system problem. In either condition we should just close
connection, no timeout involved. If server do not return any data for
longer than query_timeout caller are informed and can select the
behavior (cancel, exit or whatever it want)

> > - dblib threading... sorry but I'm still again the big lock around
> > tds_ctx... the problem seems only dbexit so I still think 
> that tds_ctx
> > don't suffer thread problems...
> 
> I agree with you but other than putting a TDS_CONTEXT in each 
> DBPROCESS 
> there isn't any other solution. I don't have the time right now to
> rewrite that much code but I can look into it later.
> 
> >From my point of view, the big mutex solution is better than 
> what's on 
> offer today to threaded users of dblib which is data corruption and 
> application crashes.
> 
> The problem is not just dbexit(). The major problem is the 
> connection_list 
> and other data in g_dblib_ctx. This should be removed in favour of a 
> TDS_CONTEXT in each DBPROCESS.
> 
> You also have data visibility issues unless you use mutual exclusion.
> 
> I have multi-threaded application that uses dblib/FreeTDS to do 1M+
> transactions per day. I can tell you, it crashes spectacularly without
> mutual exclusion in dblib.
> 

Are you using current CVS? Do you call dbexit while other thread are
processing data? Try to remove dbexit call with current CVS. Where does
your application hang inside dblib? I think that a solution is to use a
reference counter for tds_ctx. Increment while creating TDSSOCKET or
dbinit and decrement while destroying TDSSOCKET or dbexit.

> > Sorry, I forgot to add some comments. Why do you add a 
> fixed 60 seconds
> > timeout and a 3600 query_timeout? I think that is up to 
> developer if it
> > wants an infinite wait... 
> 
> I think an infinite wait is inappropriate. If the developer 
> wants a very 
> long wait then they can set timeout = 86400 or greater in 
> .freetds.conf.
> 
> As to what the default values should be, that's up for discussion.
> 

IMO default should be up to upper layer, not to network layer.

> > connection close). You removed code to close connection if 
> we are not
> > able to read header... however if we read partially header 
> we go to an
> > invalid state... much better to close connection... Note 
> that goodread
> > do not exit till if read all data... it CANNOT exit on 
> timeout (as it do
> > now... this is the reason timeout test do not works). You can change
> > this behavior however you cannot lose sync with server...
> 
> You know what the right behaviour is here from a TDS protocol 
> perspective, 
> so let's go with your suggestion. If you can produce a patch, 
> I'll test it
> against what my perception of the appropriate behaviour should be and
> hopefully we'll meet in the middle.
> 

Well, this is just what upper layer expect (in this case
tds_read_packet). Not that easy to understand but it works so... someone
could note that in tds_read_packet we still check for completeness... it
assure that it works using SSL however is not that clear...

> Finally, can you remove the double slash commented macros in 
> tdsthread.h? 
> No point having them there.
> 

Just a sort of TODO... I'll remove comment or line when we came to the
final solution.

Patch attached for net.c (from current CVS). I inlined again read_t and
fixed partial packet problem.

freddy77

Attachment: post64.diff.gz
Description: post64.diff.gz