cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH 2/7] pipelining: Fix connection handling under timeouts.

From: Carlo Wood <carlo_at_alinoe.com>
Date: Tue, 11 Nov 2014 18:03:51 +0100

On Tue, 11 Nov 2014 00:20:40 +0100 (CET)
Daniel Stenberg <daniel_at_haxx.se> wrote:

> On Mon, 10 Nov 2014, Carlo Wood wrote:
>
> > If you agree with that, then what we need is to detect is if a
> > request has already sent data to the server or not. Not if it
> > already reached the DONE state. The added multi_do_connection_id is
> > for this purpose:
>
> That's not exactly what it detects though and hence my question.

Your question was "Isn't the multi_do_connection_id check basically
another way of checking that the state hasn't reached DONE?".

To quote my previous mail, the conclusion from the elaborate example
was:

 "The point is however, that these requests did not reach the DONE state
  yet. The reason we want to close a connection is therefore not if a
  request reached the DONE state yet, but whether or not we already sent
  the request off to the server over that connection."

It is unclear to me if I answered your question or not, or if you still
wonder the same thing.

> First, the call to Curl_do() is not guaranteed to send any data (even
> if it is likely).

This doesn't really matter that much, what is most important to me is
to fix the 32 connect()/close() floods. Hence, to not close fresh
connections that haven't been used at all yet. If it keep more
connections alive then that's just a plus.

> (It is interesting to recall that there are many more protocols than
> HTTP...)

If this needs tweaking for that reason then I'm gonna have to rely on
you to fix it; I am not familiar with other protocols - or how libcurl
works in that regard.

> The check later seems to check that the timeout happens for the
> transfer that issued the most recent request on the connection (even
> the last one done on the multi handle which may use any amount of
> pipelines and connections), and *only* *then* you set the
> bits.timedout bit.

Which check are you referring to? I assume the one where timedout is
set since you say "and *only* *then* you set the bits.timedout bit",
thus:

+ if(data->multi_do_connection_id == data->easy_conn->connection_id) {
+ connclose(data->easy_conn, "Disconnected with pending data");
+ /* Make sure this connection is no longer considered for
+ pipelining. */
+ data->easy_conn->bits.timedout = TRUE;
+ }

I don't understand what you mean with "seems to check that the timeout
happens for the transfer that issued the most recent request on the
connection". Why? This checks if the request 'data' did send data over
its current connection or not. That is what we want no?

[...]
> I say you can't know (using this method) if data truly has been sent.

Fair enough, but as I said above - it is good enough in all practical
cases as it certainly stops freshly created connections to be closed
again. If you want I can come up with a change though to be more
precise with regard to the "data has been sent".

> I think using the fact that we've not reached DONE as a
> data-has-been-sent signal
[..snip..]

Heh? Are we even talking about the same thing? EXACTLY which state are
you refering to?

When you type 'DONE' I assume(d) that you mean CURLM_STATE_DONE, and
not CURLM_STATE_DO_DONE. Are you actually talking about 'DO_DONE'?
If so then I can start all over again answering your original mail :/

> could be used for simplicity as it avoids
> that weird check. There's a writebytecount that _can_ be read if we
> really wanted to know exactly, but I think that's an optmization we
> can wait with.

-- 
Carlo Wood <carlo_at_alinoe.com>
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2014-11-11