FreeTDS Development Group

[Tolga Ceylan <tolga.ceylan AT gmail.com>]

In other words, (except maybe in encrypted case), a TCP read can
return 1 byte. The underlying
call is recv(). There's no guarantee that arrivals will always be >= 8
bytes or whole packets. It's TCP which
is a stream of data where application (freetds) is responsible with
assembling application level
data. These tiny reads could happen if the client slows down and
adjusts its receive window size.

For the 'tds packet timeout', yes, I was basically suggesting to move
the timeout logic to higher level from
tds_goodread() to tds_read_packet().  Full packet timeout basically...

On Thu, Oct 30, 2014 at 12:01 PM, Frediano Ziglio <freddy77 AT gmail.com> wrote:
> 2014-10-30 17:56 GMT+00:00 Tolga Ceylan <tolga.ceylan AT gmail.com>:
>
>> I mostly agree with this. I think app level timeouts is easy to add
>> since callbacks are firing
>> and the app code can keep track of the overall time there.
>>
>> However, 'query_timeout' is misleading. This really is a
>> 'socket_activity_timeout'
>> since it's based on arriving bits of data from the socket.
>>
>> Let say 'query_timeout' is 300 secs. If the TDS packet can be
>> assembled after 5 calls to socket
>> read, then this could get blown up to 5*300 secs. This means even if
>> we rename 'query_timeout', it's not
>> really meaningful.
>>
>>
> yes, probably query_timeout is quite misleading.
>
>
>> I also see that the TDS header (8 bytes) is expected to arrive in one
>> read. If the header
>> arrives in more than one read, then the connection gets closed. This
>> is a bug. TCP code
>> needs to handle tiny read cases even if this is rare.
>>
>>
> I don't think so. The only case tds_goodread returns < 8 is when user
> requested to close connection or on a socket error. Never for less bytes
> readed. And if crypted all packets are crypted so is impossible it does not
> return at least 8 bytes.
>
>
>> I think 'query_timeout' can be renamed to 'tds packet timeout.'
>> Instead of tds_select(), the higher
>> level goodread() function can keep track of this timer and decide if
>> there is a packet timeout or not.
>>
>> I can put together a patch if you like.
>>
>> Cheers,
>> Tolga
>>
>>
> Would be useful. I didn't actually understand what you want to achieve
> however. A kind of full packet timeout? Server tends to send packet
> together so you would catch some network delay or problem. If you remove
> the timeout from tds_select how can it return if server does not send data?
>
> Frediano
>
>
>> On Thu, Oct 30, 2014 at 3:02 AM, Frediano Ziglio <freddy77 AT gmail.com>
>> wrote:
>> > 2014-10-30 4:10 GMT+00:00 Tolga Ceylan <tolga.ceylan AT gmail.com>:
>> >
>> >> Hi All,
>> >>
>> >> Recently, I've stumbled on a potential issue with the timeout handling
>> >> in ct_lib.
>> >>
>> >> Given these two cases of SQL executing;
>> >>
>> >> DECLARE @counter int
>> >> WHILE (@counter < 99999999)
>> >> BEGIN
>> >>     PRINT @counter
>> >>     SET @counter = @counter + 1
>> >> END
>> >>
>> >> versus;
>> >>
>> >> DECLARE @counter int
>> >> WHILE (@counter < 99999999)
>> >> BEGIN
>> >>     SET @counter = @counter + 1
>> >> END
>> >>
>> >> (only difference is print or no-print basically)
>> >>
>> >> seems to affect timeout handling since this trips the ::select() (aka
>> >> tds_select) based query timeout.
>> >>
>> >> In other words, continuously feeding such server messages back to the
>> >> client bypasses
>> >> the query timeout. The issue is tricky because there has to be enough
>> >> 'print' executions
>> >> to actually trigger this. (Probably due to Nagle or other buffering.)
>> >>
>> >> Has anybody seen this before? Any work around for it(*) ?
>> >>
>> >> (*) the fix probably is to consider beginning/end of the actual
>> >> queries as opposed to blocking on select
>> >> with query timeout.
>> >>
>> >> I'm using 0.82 on linux centos 6.5 although I couldn't see changes
>> >> related to this in 0.91.
>> >>
>> >> Cheers,
>> >> Tolga
>> >>
>> >
>> >
>> > Hi,
>> >   It's not a bug, it's a feature :)
>> >
>> > Really, this is the intention. Also considering that you should handle
>> > results (print IS a result) the library should return so your timeout
>> > should be managed by application.
>> >
>> > Frediano
>> > _______________________________________________
>> > FreeTDS mailing list
>> > FreeTDS AT lists.ibiblio.org
>> > http://lists.ibiblio.org/mailman/listinfo/freetds
>> _______________________________________________
>> FreeTDS mailing list
>> FreeTDS AT lists.ibiblio.org
>> http://lists.ibiblio.org/mailman/listinfo/freetds
>>
> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds