You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Usman Ismail <0x...@gmail.com> on 2011/04/26 16:43:13 UTC
HttpClient Connection close bugs
Hi,
I had a look at the Objective C and C++ http client code and noticed
that it does not supply the Connection: close header. This means that
webservers keep the connection alive assuming its a persistent
connection. This slows down client side request processing
significantly and also wastes server resources. I created issues 1153
and 1154 for c++ and objective c respectively it would be great to
have then in the next release.
I also noticed a similar issue in java although I am not sure whether
the the change is needed in java or whether underlying layers take
care of it.
--regards
Usman Ismail
Re: HttpClient Connection close bugs
Posted by Seth Hitchings <se...@evernote.com>.
When we developed the Objective-C HTTP client code we spent some time
messing around with this and found that the Cocoa networking stack
automatically implemented keep-alive, essentially doing the right thing
without us having to worry about it. I can't speak to the default C++
client, we have an in-house version based on WinINet that we use in our Win
app.
Seth
On Tue, Apr 26, 2011 at 8:13 PM, Usman Ismail <0x...@gmail.com> wrote:
> Observed in the c++ code but I also saw that objective c does not have the
> header
>
> Sent from my iPhone
>
> On 2011-04-26, at 8:09 PM, Seth Hitchings <se...@evernote.com> wrote:
>
> > What language was that? We use the Objective-C client in our Mac app and
> > don't have any issues.
> >
> > Seth
> >
> > On Tue, Apr 26, 2011 at 6:56 PM, Kerr, Rowan <rk...@ea.com> wrote:
> >
> >> Observed behavior was that reading an object from response would not
> >> return the object until connection closed.
> >>
> >>
> >> On 11-04-26 2:32 PM, "Mark Slee" <ms...@fb.com> wrote:
> >>
> >>> This should definitely be a configurable option, but I think the
> default
> >>> should definitely be *not* to send the close header. As far as I
> >>> remember, the Thrift HTTP clients were intentionally written to support
> >>> keep-alive, which is definitely the best way to use them if making
> >>> repeated requests.
> >>>
> >>> Philosophically, I think we should err on making it easier to get
> things
> >>> right in the high-volume, high-performance case. IMO, keep-alive should
> >>> be something that "just works" if you do the naïve thing (construct a
> >>> THttpClient and issue multiple subsequent RPC calls on it with no extra
> >>> lines of code in between).
> >>>
> >>> If you're closing your connections after every unique request, your
> perf
> >>> requirements are probably less stringent than someone who's pushing
> many
> >>> requests over shared connections.
> >>>
> >>> Cheers,
> >>> mcslee
> >>>
> >>>
> >>> -----Original Message-----
> >>> From: Usman Ismail [mailto:0xfffffff@gmail.com]
> >>> Sent: Tuesday, April 26, 2011 11:22 AM
> >>> To: dev@thrift.apache.org
> >>> Subject: Re: HttpClient Connection close bugs
> >>>
> >>> As far as I can tell for c++ and objective c we are not actually using
> >>> persistent http, i.e. we do not reuse open connections. Hence not
> >>> setting the close header just means it takes longer to close the
> >>> connection. In the long run I agree we should explore use of
> >>> persistent http connections as well as user specified headers.
> >>>
> >>> --usman
> >>>
> >>> On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com>
> >> wrote:
> >>>> Using keep-alive should improve performance, not degrade it. Adding
> >>>> Connection: close would significantly degrade performance, especially
> >>>> for
> >>>> HTTPS connections.
> >>>>
> >>>> In general, it would be great if all of the HTTP client transports
> >>>> exposed
> >>>> the ability to customize headers, so that an app could override
> >>>> defaults or
> >>>> add custom headers without modifying the client itself.
> >>>>
> >>>> Seth
> >>>>
> >>>> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> I had a look at the Objective C and C++ http client code and noticed
> >>>>> that it does not supply the Connection: close header. This means that
> >>>>> webservers keep the connection alive assuming its a persistent
> >>>>> connection. This slows down client side request processing
> >>>>> significantly and also wastes server resources. I created issues 1153
> >>>>> and 1154 for c++ and objective c respectively it would be great to
> >>>>> have then in the next release.
> >>>>>
> >>>>> I also noticed a similar issue in java although I am not sure whether
> >>>>> the the change is needed in java or whether underlying layers take
> >>>>> care of it.
> >>>>>
> >>>>> --regards
> >>>>> Usman Ismail
> >>>>>
> >>>>
> >>>
> >>
> >>
>
Re: HttpClient Connection close bugs
Posted by Usman Ismail <0x...@gmail.com>.
Observed in the c++ code but I also saw that objective c does not have the header
Sent from my iPhone
On 2011-04-26, at 8:09 PM, Seth Hitchings <se...@evernote.com> wrote:
> What language was that? We use the Objective-C client in our Mac app and
> don't have any issues.
>
> Seth
>
> On Tue, Apr 26, 2011 at 6:56 PM, Kerr, Rowan <rk...@ea.com> wrote:
>
>> Observed behavior was that reading an object from response would not
>> return the object until connection closed.
>>
>>
>> On 11-04-26 2:32 PM, "Mark Slee" <ms...@fb.com> wrote:
>>
>>> This should definitely be a configurable option, but I think the default
>>> should definitely be *not* to send the close header. As far as I
>>> remember, the Thrift HTTP clients were intentionally written to support
>>> keep-alive, which is definitely the best way to use them if making
>>> repeated requests.
>>>
>>> Philosophically, I think we should err on making it easier to get things
>>> right in the high-volume, high-performance case. IMO, keep-alive should
>>> be something that "just works" if you do the naïve thing (construct a
>>> THttpClient and issue multiple subsequent RPC calls on it with no extra
>>> lines of code in between).
>>>
>>> If you're closing your connections after every unique request, your perf
>>> requirements are probably less stringent than someone who's pushing many
>>> requests over shared connections.
>>>
>>> Cheers,
>>> mcslee
>>>
>>>
>>> -----Original Message-----
>>> From: Usman Ismail [mailto:0xfffffff@gmail.com]
>>> Sent: Tuesday, April 26, 2011 11:22 AM
>>> To: dev@thrift.apache.org
>>> Subject: Re: HttpClient Connection close bugs
>>>
>>> As far as I can tell for c++ and objective c we are not actually using
>>> persistent http, i.e. we do not reuse open connections. Hence not
>>> setting the close header just means it takes longer to close the
>>> connection. In the long run I agree we should explore use of
>>> persistent http connections as well as user specified headers.
>>>
>>> --usman
>>>
>>> On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com>
>> wrote:
>>>> Using keep-alive should improve performance, not degrade it. Adding
>>>> Connection: close would significantly degrade performance, especially
>>>> for
>>>> HTTPS connections.
>>>>
>>>> In general, it would be great if all of the HTTP client transports
>>>> exposed
>>>> the ability to customize headers, so that an app could override
>>>> defaults or
>>>> add custom headers without modifying the client itself.
>>>>
>>>> Seth
>>>>
>>>> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I had a look at the Objective C and C++ http client code and noticed
>>>>> that it does not supply the Connection: close header. This means that
>>>>> webservers keep the connection alive assuming its a persistent
>>>>> connection. This slows down client side request processing
>>>>> significantly and also wastes server resources. I created issues 1153
>>>>> and 1154 for c++ and objective c respectively it would be great to
>>>>> have then in the next release.
>>>>>
>>>>> I also noticed a similar issue in java although I am not sure whether
>>>>> the the change is needed in java or whether underlying layers take
>>>>> care of it.
>>>>>
>>>>> --regards
>>>>> Usman Ismail
>>>>>
>>>>
>>>
>>
>>
Re: HttpClient Connection close bugs
Posted by Seth Hitchings <se...@evernote.com>.
What language was that? We use the Objective-C client in our Mac app and
don't have any issues.
Seth
On Tue, Apr 26, 2011 at 6:56 PM, Kerr, Rowan <rk...@ea.com> wrote:
> Observed behavior was that reading an object from response would not
> return the object until connection closed.
>
>
> On 11-04-26 2:32 PM, "Mark Slee" <ms...@fb.com> wrote:
>
> >This should definitely be a configurable option, but I think the default
> >should definitely be *not* to send the close header. As far as I
> >remember, the Thrift HTTP clients were intentionally written to support
> >keep-alive, which is definitely the best way to use them if making
> >repeated requests.
> >
> >Philosophically, I think we should err on making it easier to get things
> >right in the high-volume, high-performance case. IMO, keep-alive should
> >be something that "just works" if you do the naïve thing (construct a
> >THttpClient and issue multiple subsequent RPC calls on it with no extra
> >lines of code in between).
> >
> >If you're closing your connections after every unique request, your perf
> >requirements are probably less stringent than someone who's pushing many
> >requests over shared connections.
> >
> >Cheers,
> >mcslee
> >
> >
> >-----Original Message-----
> >From: Usman Ismail [mailto:0xfffffff@gmail.com]
> >Sent: Tuesday, April 26, 2011 11:22 AM
> >To: dev@thrift.apache.org
> >Subject: Re: HttpClient Connection close bugs
> >
> >As far as I can tell for c++ and objective c we are not actually using
> >persistent http, i.e. we do not reuse open connections. Hence not
> >setting the close header just means it takes longer to close the
> >connection. In the long run I agree we should explore use of
> >persistent http connections as well as user specified headers.
> >
> >--usman
> >
> >On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com>
> wrote:
> >> Using keep-alive should improve performance, not degrade it. Adding
> >> Connection: close would significantly degrade performance, especially
> >>for
> >> HTTPS connections.
> >>
> >> In general, it would be great if all of the HTTP client transports
> >>exposed
> >> the ability to customize headers, so that an app could override
> >>defaults or
> >> add custom headers without modifying the client itself.
> >>
> >> Seth
> >>
> >> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com>
> >>wrote:
> >>
> >>> Hi,
> >>>
> >>> I had a look at the Objective C and C++ http client code and noticed
> >>> that it does not supply the Connection: close header. This means that
> >>> webservers keep the connection alive assuming its a persistent
> >>> connection. This slows down client side request processing
> >>> significantly and also wastes server resources. I created issues 1153
> >>> and 1154 for c++ and objective c respectively it would be great to
> >>> have then in the next release.
> >>>
> >>> I also noticed a similar issue in java although I am not sure whether
> >>> the the change is needed in java or whether underlying layers take
> >>> care of it.
> >>>
> >>> --regards
> >>> Usman Ismail
> >>>
> >>
> >
>
>
RE: HttpClient Connection close bugs
Posted by Mark Slee <ms...@fb.com>.
Hmm, that sounds like a bug to me then. Not familiar with the specifics, but a "correct" HTTP client implementation should definitely not block on the connection closing to complete a read. Maybe it is being too greedy about filling up a buffer or something?
I haven't been in this code for quite some time, so hard to comment definitively. Just my philosophical opinion on this one -- moving to always close the connection seems like a workable but suboptimal fix.
-----Original Message-----
From: Kerr, Rowan [mailto:rkerr@ea.com]
Sent: Tuesday, April 26, 2011 3:56 PM
To: dev@thrift.apache.org
Subject: Re: HttpClient Connection close bugs
Observed behavior was that reading an object from response would not
return the object until connection closed.
On 11-04-26 2:32 PM, "Mark Slee" <ms...@fb.com> wrote:
>This should definitely be a configurable option, but I think the default
>should definitely be *not* to send the close header. As far as I
>remember, the Thrift HTTP clients were intentionally written to support
>keep-alive, which is definitely the best way to use them if making
>repeated requests.
>
>Philosophically, I think we should err on making it easier to get things
>right in the high-volume, high-performance case. IMO, keep-alive should
>be something that "just works" if you do the naïve thing (construct a
>THttpClient and issue multiple subsequent RPC calls on it with no extra
>lines of code in between).
>
>If you're closing your connections after every unique request, your perf
>requirements are probably less stringent than someone who's pushing many
>requests over shared connections.
>
>Cheers,
>mcslee
>
>
>-----Original Message-----
>From: Usman Ismail [mailto:0xfffffff@gmail.com]
>Sent: Tuesday, April 26, 2011 11:22 AM
>To: dev@thrift.apache.org
>Subject: Re: HttpClient Connection close bugs
>
>As far as I can tell for c++ and objective c we are not actually using
>persistent http, i.e. we do not reuse open connections. Hence not
>setting the close header just means it takes longer to close the
>connection. In the long run I agree we should explore use of
>persistent http connections as well as user specified headers.
>
>--usman
>
>On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com> wrote:
>> Using keep-alive should improve performance, not degrade it. Adding
>> Connection: close would significantly degrade performance, especially
>>for
>> HTTPS connections.
>>
>> In general, it would be great if all of the HTTP client transports
>>exposed
>> the ability to customize headers, so that an app could override
>>defaults or
>> add custom headers without modifying the client itself.
>>
>> Seth
>>
>> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com>
>>wrote:
>>
>>> Hi,
>>>
>>> I had a look at the Objective C and C++ http client code and noticed
>>> that it does not supply the Connection: close header. This means that
>>> webservers keep the connection alive assuming its a persistent
>>> connection. This slows down client side request processing
>>> significantly and also wastes server resources. I created issues 1153
>>> and 1154 for c++ and objective c respectively it would be great to
>>> have then in the next release.
>>>
>>> I also noticed a similar issue in java although I am not sure whether
>>> the the change is needed in java or whether underlying layers take
>>> care of it.
>>>
>>> --regards
>>> Usman Ismail
>>>
>>
>
Re: HttpClient Connection close bugs
Posted by "Kerr, Rowan" <rk...@ea.com>.
Observed behavior was that reading an object from response would not
return the object until connection closed.
On 11-04-26 2:32 PM, "Mark Slee" <ms...@fb.com> wrote:
>This should definitely be a configurable option, but I think the default
>should definitely be *not* to send the close header. As far as I
>remember, the Thrift HTTP clients were intentionally written to support
>keep-alive, which is definitely the best way to use them if making
>repeated requests.
>
>Philosophically, I think we should err on making it easier to get things
>right in the high-volume, high-performance case. IMO, keep-alive should
>be something that "just works" if you do the naïve thing (construct a
>THttpClient and issue multiple subsequent RPC calls on it with no extra
>lines of code in between).
>
>If you're closing your connections after every unique request, your perf
>requirements are probably less stringent than someone who's pushing many
>requests over shared connections.
>
>Cheers,
>mcslee
>
>
>-----Original Message-----
>From: Usman Ismail [mailto:0xfffffff@gmail.com]
>Sent: Tuesday, April 26, 2011 11:22 AM
>To: dev@thrift.apache.org
>Subject: Re: HttpClient Connection close bugs
>
>As far as I can tell for c++ and objective c we are not actually using
>persistent http, i.e. we do not reuse open connections. Hence not
>setting the close header just means it takes longer to close the
>connection. In the long run I agree we should explore use of
>persistent http connections as well as user specified headers.
>
>--usman
>
>On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com> wrote:
>> Using keep-alive should improve performance, not degrade it. Adding
>> Connection: close would significantly degrade performance, especially
>>for
>> HTTPS connections.
>>
>> In general, it would be great if all of the HTTP client transports
>>exposed
>> the ability to customize headers, so that an app could override
>>defaults or
>> add custom headers without modifying the client itself.
>>
>> Seth
>>
>> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com>
>>wrote:
>>
>>> Hi,
>>>
>>> I had a look at the Objective C and C++ http client code and noticed
>>> that it does not supply the Connection: close header. This means that
>>> webservers keep the connection alive assuming its a persistent
>>> connection. This slows down client side request processing
>>> significantly and also wastes server resources. I created issues 1153
>>> and 1154 for c++ and objective c respectively it would be great to
>>> have then in the next release.
>>>
>>> I also noticed a similar issue in java although I am not sure whether
>>> the the change is needed in java or whether underlying layers take
>>> care of it.
>>>
>>> --regards
>>> Usman Ismail
>>>
>>
>
RE: HttpClient Connection close bugs
Posted by Mark Slee <ms...@fb.com>.
This should definitely be a configurable option, but I think the default should definitely be *not* to send the close header. As far as I remember, the Thrift HTTP clients were intentionally written to support keep-alive, which is definitely the best way to use them if making repeated requests.
Philosophically, I think we should err on making it easier to get things right in the high-volume, high-performance case. IMO, keep-alive should be something that "just works" if you do the naïve thing (construct a THttpClient and issue multiple subsequent RPC calls on it with no extra lines of code in between).
If you're closing your connections after every unique request, your perf requirements are probably less stringent than someone who's pushing many requests over shared connections.
Cheers,
mcslee
-----Original Message-----
From: Usman Ismail [mailto:0xfffffff@gmail.com]
Sent: Tuesday, April 26, 2011 11:22 AM
To: dev@thrift.apache.org
Subject: Re: HttpClient Connection close bugs
As far as I can tell for c++ and objective c we are not actually using
persistent http, i.e. we do not reuse open connections. Hence not
setting the close header just means it takes longer to close the
connection. In the long run I agree we should explore use of
persistent http connections as well as user specified headers.
--usman
On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com> wrote:
> Using keep-alive should improve performance, not degrade it. Adding
> Connection: close would significantly degrade performance, especially for
> HTTPS connections.
>
> In general, it would be great if all of the HTTP client transports exposed
> the ability to customize headers, so that an app could override defaults or
> add custom headers without modifying the client itself.
>
> Seth
>
> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com> wrote:
>
>> Hi,
>>
>> I had a look at the Objective C and C++ http client code and noticed
>> that it does not supply the Connection: close header. This means that
>> webservers keep the connection alive assuming its a persistent
>> connection. This slows down client side request processing
>> significantly and also wastes server resources. I created issues 1153
>> and 1154 for c++ and objective c respectively it would be great to
>> have then in the next release.
>>
>> I also noticed a similar issue in java although I am not sure whether
>> the the change is needed in java or whether underlying layers take
>> care of it.
>>
>> --regards
>> Usman Ismail
>>
>
Re: HttpClient Connection close bugs
Posted by Usman Ismail <0x...@gmail.com>.
As far as I can tell for c++ and objective c we are not actually using
persistent http, i.e. we do not reuse open connections. Hence not
setting the close header just means it takes longer to close the
connection. In the long run I agree we should explore use of
persistent http connections as well as user specified headers.
--usman
On Tue, Apr 26, 2011 at 2:12 PM, Seth Hitchings <se...@evernote.com> wrote:
> Using keep-alive should improve performance, not degrade it. Adding
> Connection: close would significantly degrade performance, especially for
> HTTPS connections.
>
> In general, it would be great if all of the HTTP client transports exposed
> the ability to customize headers, so that an app could override defaults or
> add custom headers without modifying the client itself.
>
> Seth
>
> On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com> wrote:
>
>> Hi,
>>
>> I had a look at the Objective C and C++ http client code and noticed
>> that it does not supply the Connection: close header. This means that
>> webservers keep the connection alive assuming its a persistent
>> connection. This slows down client side request processing
>> significantly and also wastes server resources. I created issues 1153
>> and 1154 for c++ and objective c respectively it would be great to
>> have then in the next release.
>>
>> I also noticed a similar issue in java although I am not sure whether
>> the the change is needed in java or whether underlying layers take
>> care of it.
>>
>> --regards
>> Usman Ismail
>>
>
Re: HttpClient Connection close bugs
Posted by Seth Hitchings <se...@evernote.com>.
Using keep-alive should improve performance, not degrade it. Adding
Connection: close would significantly degrade performance, especially for
HTTPS connections.
In general, it would be great if all of the HTTP client transports exposed
the ability to customize headers, so that an app could override defaults or
add custom headers without modifying the client itself.
Seth
On Tue, Apr 26, 2011 at 10:43 AM, Usman Ismail <0x...@gmail.com> wrote:
> Hi,
>
> I had a look at the Objective C and C++ http client code and noticed
> that it does not supply the Connection: close header. This means that
> webservers keep the connection alive assuming its a persistent
> connection. This slows down client side request processing
> significantly and also wastes server resources. I created issues 1153
> and 1154 for c++ and objective c respectively it would be great to
> have then in the next release.
>
> I also noticed a similar issue in java although I am not sure whether
> the the change is needed in java or whether underlying layers take
> care of it.
>
> --regards
> Usman Ismail
>