Re: Controlling releaseConnection
On Wed, 2016-10-19 at 10:14 -0400, Pellerin, Clement wrote: > I agree with you this feature is strange and need not be added to the library. > As I said before, I need to preserve this feature for backwards compatibility. > It has something to do with delaying the emission of an MDN in an AS2 gateway > so it represents the status of the receiver instead of the man in the middle. > Again, please don't ask me to change the architecture of the solution. > We don't own the application and we can't change it. Nevertheless it must > continue to work after we port to HttpClient 4.5.2 > > As for the work-around, I have something working but it duplicates too much > code for my taste. I think this is a nice opportunity for HttpClient > to learn what it is like to try to subclass its engine. > > I am subclassing HttpClientBuilder to override createMainExec() > to return my own implementation of the ClientExecChain. > This is nice. > > Unfortunately, our subclass of ClientExecChain must be in the > org.apache.http.impl.execchain package because support classes > in that package are all package private. This is annoying. > > I modified the problematic code in ClientExecChain.execute() > but that was not enough. I need to trap the EofSensorWatcher > of the response HttpEntity. That requires duplicating > HttpResponseProxy and ResponseEntityProxy. > Again this must be done in the apache package. > > The release should go through the ConnectionHolder since that's > where the "AtomicBoolean released" field is stored. > I thought of providing my own implementation of ConnectionHolder > with an empty releaseConnection() method, but that does not work > when ResponseEntityProxy calls cleanup(), this becomes > releaseConnection(false) which closes the connection completely. > Those classes are package private so we would not keep them API compatible between minor releases. I intend to re-evaluate the execution pipeline code in 5.0 and attempt to provide a public API based on the chain of responsibility pattern. > I would appreciate if you have better suggestions for the work-around. > It would be nice to make subclassing easier too. > > By the way, should ResponseEntityProxy.enchance() > be called enhance() instead? > Compare this to RequestEntityProxy.enhance() > Yes, it should be. Please fork HttpClient on Github and raise a PR with changes you are proposing. We can add methods to MainClientExec as long as they remain package private. Oleg - To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org For additional commands, e-mail: httpclient-users-h...@hc.apache.org
RE: Controlling releaseConnection
I agree with you this feature is strange and need not be added to the library.
As I said before, I need to preserve this feature for backwards compatibility.
It has something to do with delaying the emission of an MDN in an AS2 gateway
so it represents the status of the receiver instead of the man in the middle.
Again, please don't ask me to change the architecture of the solution.
We don't own the application and we can't change it. Nevertheless it must
continue to work after we port to HttpClient 4.5.2
As for the work-around, I have something working but it duplicates too much
code for my taste. I think this is a nice opportunity for HttpClient
to learn what it is like to try to subclass its engine.
I am subclassing HttpClientBuilder to override createMainExec()
to return my own implementation of the ClientExecChain.
This is nice.
Unfortunately, our subclass of ClientExecChain must be in the
org.apache.http.impl.execchain package because support classes
in that package are all package private. This is annoying.
I modified the problematic code in ClientExecChain.execute()
but that was not enough. I need to trap the EofSensorWatcher
of the response HttpEntity. That requires duplicating
HttpResponseProxy and ResponseEntityProxy.
Again this must be done in the apache package.
The release should go through the ConnectionHolder since that's
where the "AtomicBoolean released" field is stored.
I thought of providing my own implementation of ConnectionHolder
with an empty releaseConnection() method, but that does not work
when ResponseEntityProxy calls cleanup(), this becomes
releaseConnection(false) which closes the connection completely.
I would appreciate if you have better suggestions for the work-around.
It would be nice to make subclassing easier too.
By the way, should ResponseEntityProxy.enchance()
be called enhance() instead?
Compare this to RequestEntityProxy.enhance()
-Original Message-
From: Oleg Kalnichevski [mailto:ol...@apache.org]
Sent: Tuesday, October 18, 2016 10:48 AM
To: HttpClient User Discussion
Subject: Re: Controlling releaseConnection
On Tue, 2016-10-18 at 10:12 -0400, Pellerin, Clement wrote:
> I agree that code is correct. I never said there was a bug.
> I am asking how to override the behavior.
> We are porting our product from HttpClient 4.1 to 4.5.2 and we need to
> preserve that feature because it is used by our customers.
Clement
I'll be happy to suggest a work-around but I am still unable to understand what
it is exactly you are trying to achieve by keeping a connection for messages
without a message body, which is quite uncommon (status 204, status 304,
response to HEAD, and that is it).
What is exactly the point of doing so in th first place?
Oleg
> It would be a shame to duplicate all of MainClientExec because of a
> lack of modularity in that class.
>
> We tried to subclass the PoolingHttpClientConnectionManager
> but that does not work because the connection state is actually stored
> in the ConnectionHolder created directly in MainClientExec.execute().
> final ConnectionHolder connHolder = new
> ConnectionHolder(this.log, this.connManager, managedConn);
>
> Basically, I'm asking if the maintainers could consider a method like
> protected boolean isEarlyRelease(HttpEntity entity) {
> return entity == null || !entity.isStreaming();
> }
>
> Or a release strategy interface if this is a common requirement.
> I am also interested in any other work-around.
>
> -Original Message-
> From: Shawn Heisey [mailto:apa...@elyograg.org]
> Sent: Monday, October 17, 2016 9:24 PM
> To: HttpClient User Discussion
> Subject: Re: Controlling releaseConnection
>
> On 10/17/2016 3:22 PM, Pellerin, Clement wrote:
> > Our customer needs to delay the release of the connection until the
> > response is fully processed.
> > They want to turn off the early automatic release of the connection and do
> > it manually later.
> >
> > This is the problematic code in MainClientExec
> > // check for entity, release connection if possible
> > final HttpEntity entity = response.getEntity();
> > if (entity == null || !entity.isStreaming()) {
> > // connection not needed and (assumed to be) in re-usable
> > state
> > connHolder.releaseConnection();
> > return new HttpResponseProxy(response, null);
> > } else {
> > return new HttpResponseProxy(response, connHolder);
> > }
>
> Mostly an end-user here, with no status to speak of in this project. I do
> have status on another Apache project that utilizes HttpClient, but I don't
> know much about that part of the code. I have written some HttpClient code
Re: Controlling releaseConnection
On Tue, 2016-10-18 at 10:12 -0400, Pellerin, Clement wrote:
> I agree that code is correct. I never said there was a bug.
> I am asking how to override the behavior.
> We are porting our product from HttpClient 4.1 to 4.5.2
> and we need to preserve that feature because it is used by our customers.
Clement
I'll be happy to suggest a work-around but I am still unable to
understand what it is exactly you are trying to achieve by keeping a
connection for messages without a message body, which is quite uncommon
(status 204, status 304, response to HEAD, and that is it).
What is exactly the point of doing so in th first place?
Oleg
> It would be a shame to duplicate all of MainClientExec because
> of a lack of modularity in that class.
>
> We tried to subclass the PoolingHttpClientConnectionManager
> but that does not work because the connection state is actually
> stored in the ConnectionHolder created directly in MainClientExec.execute().
> final ConnectionHolder connHolder = new ConnectionHolder(this.log,
> this.connManager, managedConn);
>
> Basically, I'm asking if the maintainers could consider a method like
> protected boolean isEarlyRelease(HttpEntity entity) {
> return entity == null || !entity.isStreaming();
> }
>
> Or a release strategy interface if this is a common requirement.
> I am also interested in any other work-around.
>
> -Original Message-
> From: Shawn Heisey [mailto:apa...@elyograg.org]
> Sent: Monday, October 17, 2016 9:24 PM
> To: HttpClient User Discussion
> Subject: Re: Controlling releaseConnection
>
> On 10/17/2016 3:22 PM, Pellerin, Clement wrote:
> > Our customer needs to delay the release of the connection until the
> > response is fully processed.
> > They want to turn off the early automatic release of the connection and do
> > it manually later.
> >
> > This is the problematic code in MainClientExec
> > // check for entity, release connection if possible
> > final HttpEntity entity = response.getEntity();
> > if (entity == null || !entity.isStreaming()) {
> > // connection not needed and (assumed to be) in re-usable
> > state
> > connHolder.releaseConnection();
> > return new HttpResponseProxy(response, null);
> > } else {
> > return new HttpResponseProxy(response, connHolder);
> > }
>
> Mostly an end-user here, with no status to speak of in this project. I do
> have status on another Apache project that utilizes HttpClient, but I don't
> know much about that part of the code. I have written some HttpClient code
> for a completely unrelated project of my own, but that code is VERY simple.
>
> When I read the code above, what I see is this: It only releases the
> connection if the entity is nonexistent (null) or the entity is NOT a type
> that uses streaming.
>
> I will fully admit that my experience with HttpClient is limited, but I think
> the chance is very small that the HttpComponents committers have made a
> mistake here. I think this particular code has probably been discussed and
> examined, then ultimately validated as correct. Here's why I think they
> didn't make a mistake:
>
> If the entity object is null, then the response probably doesn't HAVE an
> entity (response body), so it will be entirely self-contained, consisting of
> headers only, and the connection doesn't have anything further to send. If
> the entity exists but doesn't utilize streaming, then I think it's likely
> that the entity was received in its entirety and has been incorporated into
> the response object already, and once again, the connection isn't needed. If
> my limited understanding of non-streaming entities is correct, they have the
> potential to be very dangerous from a memory consumption perspective, and my
> own usage of HttpClient (where I did not set anything related to the entity
> type) suggests that streaming entities are used by default.
>
> Restating in another way: In the first situation that results in a released
> connection, there's nothing to consume, you just need the response object
> that you already have. In the second situation, the entity you will consume
> is probably already available within the response object and doesn't need the
> connection. The comment on the release call in the code quoted above implies
> that this is how things work.
>
> In these situations, why do you need the connection to stick around? I think
> it can't do anything else that's useful for that request. I would imagine
> that if the connection utilizes keepalive/pipelining,
RE: Controlling releaseConnection
I agree that code is correct. I never said there was a bug.
I am asking how to override the behavior.
We are porting our product from HttpClient 4.1 to 4.5.2
and we need to preserve that feature because it is used by our customers.
It would be a shame to duplicate all of MainClientExec because
of a lack of modularity in that class.
We tried to subclass the PoolingHttpClientConnectionManager
but that does not work because the connection state is actually
stored in the ConnectionHolder created directly in MainClientExec.execute().
final ConnectionHolder connHolder = new ConnectionHolder(this.log,
this.connManager, managedConn);
Basically, I'm asking if the maintainers could consider a method like
protected boolean isEarlyRelease(HttpEntity entity) {
return entity == null || !entity.isStreaming();
}
Or a release strategy interface if this is a common requirement.
I am also interested in any other work-around.
-Original Message-
From: Shawn Heisey [mailto:apa...@elyograg.org]
Sent: Monday, October 17, 2016 9:24 PM
To: HttpClient User Discussion
Subject: Re: Controlling releaseConnection
On 10/17/2016 3:22 PM, Pellerin, Clement wrote:
> Our customer needs to delay the release of the connection until the response
> is fully processed.
> They want to turn off the early automatic release of the connection and do it
> manually later.
>
> This is the problematic code in MainClientExec
> // check for entity, release connection if possible
> final HttpEntity entity = response.getEntity();
> if (entity == null || !entity.isStreaming()) {
> // connection not needed and (assumed to be) in re-usable
> state
> connHolder.releaseConnection();
> return new HttpResponseProxy(response, null);
> } else {
> return new HttpResponseProxy(response, connHolder);
> }
Mostly an end-user here, with no status to speak of in this project. I do have
status on another Apache project that utilizes HttpClient, but I don't know
much about that part of the code. I have written some HttpClient code for a
completely unrelated project of my own, but that code is VERY simple.
When I read the code above, what I see is this: It only releases the connection
if the entity is nonexistent (null) or the entity is NOT a type that uses
streaming.
I will fully admit that my experience with HttpClient is limited, but I think
the chance is very small that the HttpComponents committers have made a mistake
here. I think this particular code has probably been discussed and examined,
then ultimately validated as correct. Here's why I think they didn't make a
mistake:
If the entity object is null, then the response probably doesn't HAVE an entity
(response body), so it will be entirely self-contained, consisting of headers
only, and the connection doesn't have anything further to send. If the entity
exists but doesn't utilize streaming, then I think it's likely that the entity
was received in its entirety and has been incorporated into the response object
already, and once again, the connection isn't needed. If my limited
understanding of non-streaming entities is correct, they have the potential to
be very dangerous from a memory consumption perspective, and my own usage of
HttpClient (where I did not set anything related to the entity type) suggests
that streaming entities are used by default.
Restating in another way: In the first situation that results in a released
connection, there's nothing to consume, you just need the response object that
you already have. In the second situation, the entity you will consume is
probably already available within the response object and doesn't need the
connection. The comment on the release call in the code quoted above implies
that this is how things work.
In these situations, why do you need the connection to stick around? I think
it can't do anything else that's useful for that request. I would imagine that
if the connection utilizes keepalive/pipelining, that it will typically remain
open after release and can be utilized again for a different request.
Someone with more direct knowledge of HttpClient's internal implementation will
need to confirm whether or not I'm correct in what I've written. My
understanding could be wrong.
Thanks,
Shawn
-
To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org
For additional commands, e-mail: httpclient-users-h...@hc.apache.org
Re: AW: Controlling releaseConnection
On Tue, 2016-10-18 at 01:46 +0200, e...@zusammenkunft.net wrote: > Hello, > > Can younspecify why you need to delay it? Do wou want to make some kind of > rate limit with this or optimize pipelining? > > Gruss > Bernd I am not quite sure I understand the problem. HttpClient releases connection back to the pool only when done reading content from it. Why would one want to hold onto the connection once the response message has been fully consumed? Am I missing something? Oleg - To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org For additional commands, e-mail: httpclient-users-h...@hc.apache.org
Re: Controlling releaseConnection
On 10/17/2016 3:22 PM, Pellerin, Clement wrote:
> Our customer needs to delay the release of the connection until the response
> is fully processed.
> They want to turn off the early automatic release of the connection and do it
> manually later.
>
> This is the problematic code in MainClientExec
> // check for entity, release connection if possible
> final HttpEntity entity = response.getEntity();
> if (entity == null || !entity.isStreaming()) {
> // connection not needed and (assumed to be) in re-usable
> state
> connHolder.releaseConnection();
> return new HttpResponseProxy(response, null);
> } else {
> return new HttpResponseProxy(response, connHolder);
> }
Mostly an end-user here, with no status to speak of in this project. I
do have status on another Apache project that utilizes HttpClient, but I
don't know much about that part of the code. I have written some
HttpClient code for a completely unrelated project of my own, but that
code is VERY simple.
When I read the code above, what I see is this: It only releases the
connection if the entity is nonexistent (null) or the entity is NOT a
type that uses streaming.
I will fully admit that my experience with HttpClient is limited, but I
think the chance is very small that the HttpComponents committers have
made a mistake here. I think this particular code has probably been
discussed and examined, then ultimately validated as correct. Here's
why I think they didn't make a mistake:
If the entity object is null, then the response probably doesn't HAVE an
entity (response body), so it will be entirely self-contained,
consisting of headers only, and the connection doesn't have anything
further to send. If the entity exists but doesn't utilize streaming,
then I think it's likely that the entity was received in its entirety
and has been incorporated into the response object already, and once
again, the connection isn't needed. If my limited understanding of
non-streaming entities is correct, they have the potential to be very
dangerous from a memory consumption perspective, and my own usage of
HttpClient (where I did not set anything related to the entity type)
suggests that streaming entities are used by default.
Restating in another way: In the first situation that results in a
released connection, there's nothing to consume, you just need the
response object that you already have. In the second situation, the
entity you will consume is probably already available within the
response object and doesn't need the connection. The comment on the
release call in the code quoted above implies that this is how things work.
In these situations, why do you need the connection to stick around? I
think it can't do anything else that's useful for that request. I would
imagine that if the connection utilizes keepalive/pipelining, that it
will typically remain open after release and can be utilized again for a
different request.
Someone with more direct knowledge of HttpClient's internal
implementation will need to confirm whether or not I'm correct in what
I've written. My understanding could be wrong.
Thanks,
Shawn
-
To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org
For additional commands, e-mail: httpclient-users-h...@hc.apache.org
AW: Controlling releaseConnection
Hello,
Can younspecify why you need to delay it? Do wou want to make some kind of rate
limit with this or optimize pipelining?
Gruss
Bernd
--
http://bernd.eckenfels.net
>From Win 10 Mobile
Von: Pellerin, Clement
Gesendet: Montag, 17. Oktober 2016 23:23
An: httpclient-users@hc.apache.org
Betreff: Controlling releaseConnection
We are using HttpClient 4.5.2
Our customer needs to delay the release of the connection until the response is
fully processed.
They want to turn off the early automatic release of the connection and do it
manually later.
This is the problematic code in MainClientExec
// check for entity, release connection if possible
final HttpEntity entity = response.getEntity();
if (entity == null || !entity.isStreaming()) {
// connection not needed and (assumed to be) in re-usable state
connHolder.releaseConnection();
return new HttpResponseProxy(response, null);
} else {
return new HttpResponseProxy(response, connHolder);
}
Can you suggest an approach to do this without duplicating all of
MainClientExec.execute()?
What acceptable changes can we make to the HttpClient source code to make this
easier?
I am willing to implement the change if there is interest.
-
To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org
For additional commands, e-mail: httpclient-users-h...@hc.apache.org
Controlling releaseConnection
We are using HttpClient 4.5.2
Our customer needs to delay the release of the connection until the response is
fully processed.
They want to turn off the early automatic release of the connection and do it
manually later.
This is the problematic code in MainClientExec
// check for entity, release connection if possible
final HttpEntity entity = response.getEntity();
if (entity == null || !entity.isStreaming()) {
// connection not needed and (assumed to be) in re-usable state
connHolder.releaseConnection();
return new HttpResponseProxy(response, null);
} else {
return new HttpResponseProxy(response, connHolder);
}
Can you suggest an approach to do this without duplicating all of
MainClientExec.execute()?
What acceptable changes can we make to the HttpClient source code to make this
easier?
I am willing to implement the change if there is interest.
-
To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org
For additional commands, e-mail: httpclient-users-h...@hc.apache.org
