FreeTDS Development Group

[Frediano Ziglio <freddy77 AT gmail.com>]

2014-10-30 19:23 GMT+00:00 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.
>
>
I know... but we don't just call recv, it's wrapped to handle such cases.


> 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...
>
>
Understood. Still the point remains, you are catching corner cases. We are
trying to detect if server is not sending anything. I understood that in
some situations packets could get a lot fragmented and every fragment can
have different reasons to be delayed so the total timeout can be quite big
but we are talking about packets that are mostly 4kb transmitted in network
packets which are more than 1kb.

Good code anyway is always welcome. Remember to use the master branch.

Frediano



> 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
> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds
>