You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Roland Weber <ht...@dubioso.net> on 2007/02/18 20:23:51 UTC

[HttpConn] another status update

Hi all,

I've hacked up plenty of connection release code in HttpConn,
plus some use of that code in HttpClient and examples. Since
I was a little bit confused myself when writing that code,
I'll give you a quick intro in case you want to have a look.
There are three levels on which a connection can be released:

(1) Connection Management level
The connection manager has a releaseConnection method that
expects the connection as argument. The connection itself
also has a releaseConnection method and will call back to
it's connection manager if that is called. Actually there
are two methods:
  releaseConnection - gracefully
  abortConnection - enforces shutdown, no connection re-use

(2) Entity level
The new BasicManagedEntity keeps a reference to the connection
that should be released. There are three methods:
  consumeContent - gracefully
  releaseConnection - gracefully
  abortConnection - enforces shutdown, no connection re-use
The entity also installs a level 3 notification handler.

(3) Stream level
EofSensorInputStream replaces AutoCloseInputStream. There
are three methods:
  close - gracefully
  releaseConnection - gracefully
  abortConnection - enforces shutdown, no connection re-use
Actual handling is typically delegated to the watcher.


I know it sounds tricky. It is. But I don't think we could
or should drop any of those levels or methods. The two
methods available on all three levels are defined in
ConnectionReleaseTrigger to facilitate instanceof checks.
The additional methods on some levels are required by the
interfaces implemented.
A release option on the entity level should be available
since HttpEntity.getContent() will return the stream only
once. If a method like EntityUtils.toString(HttpEntity)
accesses the stream and then fails before reading to the
end, level 3 can no longer be used to release the connection.
Dragging level 1 information through your application is
the situation which we have in HttpClient 3, and we've had
requests to drop that requirement since years ago.
It is hard to determine on one level whether the connection
has already been released on another level. The manager is
therefore required to tolerate multiple release of a single
connection.

Another detail I introduced is a flag in the connection
that tells the manager whether it is reusable. That's
meant for tracking the communication states as mentioned
in [1] and [2], but it also came in handy for implementing
abortConnection().

I don't consider that another milestone, but I'm closing
in on one. At the very least, I want to add support for
a connection re-use strategy in the request director.
There will be a little twitch to that one: instead of
remembering the strategy and arguments required to call it
until the connection is released, I'd like to evaluate
the strategy directly after the response header is received
and only remember the boolean result. That will go a long
way towards addressing HTTPCLIENT-589. It shouldn't make a
difference to the strategy, unless that depends on trailers.
Has anybody ever seen trailers being used in the wild?

Once that is addressed, the question will be whether I
should implement a SimpleClientConnectionManager before
or after writing test cases for ThreadSafeClientConnManager
and the rest of HttpConn. What do you think?
Of course I have plenty of ideas for HttpClient, but I
feel it makes more sense to stabilize HttpConn quickly.
Test cases are a prerequisite for cleaning up TSCCM.

I'm signing off for today... Snooker is on :-)

cheers,
  Roland

[1]
http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B333F3.3010706@dubioso.net%3e
[2]
http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B361C0.50509@dubioso.net%3e
[3] https://issues.apache.org/jira/browse/HTTPCLIENT-589

---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,

>> Once that is addressed, the question will be whether I
>> should implement a SimpleClientConnectionManager before
>> or after writing test cases for ThreadSafeClientConnManager
>> and the rest of HttpConn. What do you think?
> 
> I think you should bring your work on HttpConn to some kind of logical
> closure and then start working on adding test coverage.

That's a good criterion. I will implement the SCCM since
that helps me to validate AbstractClientConnectionAdapter.
I should get that done by the week-end. I'm probably
travelling again next week, so that is a good occasion
to round up the API for review.

cheers,
  Roland


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2007-02-18 at 20:23 +0100, Roland Weber wrote:
> Hi all,
> 
> I've hacked up plenty of connection release code in HttpConn,
> plus some use of that code in HttpClient and examples. Since
> I was a little bit confused myself when writing that code,
> I'll give you a quick intro in case you want to have a look.
> There are three levels on which a connection can be released:
> 
> (1) Connection Management level
> The connection manager has a releaseConnection method that
> expects the connection as argument. The connection itself
> also has a releaseConnection method and will call back to
> it's connection manager if that is called. Actually there
> are two methods:
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> 
> (2) Entity level
> The new BasicManagedEntity keeps a reference to the connection
> that should be released. There are three methods:
>   consumeContent - gracefully
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> The entity also installs a level 3 notification handler.
> 
> (3) Stream level
> EofSensorInputStream replaces AutoCloseInputStream. There
> are three methods:
>   close - gracefully
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> Actual handling is typically delegated to the watcher.
> 
> 
> I know it sounds tricky. It is. But I don't think we could
> or should drop any of those levels or methods.

This seems reasonable to me, at least as far as the high level design is
concerned. 

>  The two
> methods available on all three levels are defined in
> ConnectionReleaseTrigger to facilitate instanceof checks.
> The additional methods on some levels are required by the
> interfaces implemented.
> A release option on the entity level should be available
> since HttpEntity.getContent() will return the stream only
> once. If a method like EntityUtils.toString(HttpEntity)
> accesses the stream and then fails before reading to the
> end, level 3 can no longer be used to release the connection.
> Dragging level 1 information through your application is
> the situation which we have in HttpClient 3, and we've had
> requests to drop that requirement since years ago.
> It is hard to determine on one level whether the connection
> has already been released on another level. The manager is
> therefore required to tolerate multiple release of a single
> connection.
> 
> Another detail I introduced is a flag in the connection
> that tells the manager whether it is reusable. That's
> meant for tracking the communication states as mentioned
> in [1] and [2], but it also came in handy for implementing
> abortConnection().
> 
> I don't consider that another milestone, but I'm closing
> in on one. At the very least, I want to add support for
> a connection re-use strategy in the request director.
> There will be a little twitch to that one: instead of
> remembering the strategy and arguments required to call it
> until the connection is released, I'd like to evaluate
> the strategy directly after the response header is received
> and only remember the boolean result. That will go a long
> way towards addressing HTTPCLIENT-589. It shouldn't make a
> difference to the strategy, unless that depends on trailers.
> Has anybody ever seen trailers being used in the wild?
> 

I have not. I am in favor of just ignoring trailers in HttpClient.
Trailers are fully supported in HttpCore, but they appear to be of no
use what so ever for HttpClient.

> Once that is addressed, the question will be whether I
> should implement a SimpleClientConnectionManager before
> or after writing test cases for ThreadSafeClientConnManager
> and the rest of HttpConn. What do you think?

I think you should bring your work on HttpConn to some kind of logical
closure and then start working on adding test coverage.

> Of course I have plenty of ideas for HttpClient, but I
> feel it makes more sense to stabilize HttpConn quickly.
> Test cases are a prerequisite for cleaning up TSCCM.
> 
> I'm signing off for today... Snooker is on :-)
> 

First things first ;-)

Cheers

Oleg

> cheers,
>   Roland
> 
> [1]
> http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B333F3.3010706@dubioso.net%3e
> [2]
> http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B361C0.50509@dubioso.net%3e
> [3] https://issues.apache.org/jira/browse/HTTPCLIENT-589
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Roland Weber <os...@dubioso.net>.
Hi Oleg,

I don't have time for a thorough review, but I had a glimpse.
Looks good to me. If it works (which I assume :-), throw out
the TalkativeSocket stuff and close HTTPCLIENT-497.

One thing though: I hate classes that don't appear in the
default JavaDocs. How about making the Wire class public?
It's in an impl package, that should be enough to tell people
that it is not part of the API.

At one point in time, we have to review all logging classes
and preferably change the static Log objects to non-static.
http://wiki.apache.org/jakarta-commons/Logging/StaticLog
But there's plenty of static ones in the connection classes
I ported, too. So we better do that in one go when we feel
that the code is becoming stable, rather than piece by piece
while we port individual classes.

cheers,
  Roland

---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2007-03-19 at 20:12 +0100, Roland Weber wrote:

...

> > (3) I would like to add logging decorators for HttpDataReceiver and
> > HttpDataTransmitter interfaces, so we could have the convenience of wire
> > logging, which I find very useful when developing new stuff.   
> 
> I put a TalkativeSocketFactory in the HttpClient contrib code to achieve
> a similar result, but I never was satisfied with that plug-in point,
> and it would not work well for layered connections. Please go ahead.
> Btw, when I used that approach, it seemed that some characters in the
> header were being written one by one rather than buffered. I don't
> recall what exactly it was, it might have been a header name.
> 
> cheers,
>   Roland
> 

Hi Roland,

I added wire and headers logging to DefaultClientConnection. Please
review. I also would like to go ahead and close HTTPCLIENT-497 if I hear
no objections.

Do you think we should keep TalkativeSocketFactory in the contrib
package?

Cheers,

Oleg

> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2007-03-19 at 20:12 +0100, Roland Weber wrote:
> Hi Oleg,
> 
> as usual, our opinions differ...
> 
> > (1) I tend to dislike static helper methods in examples, because they
> > disrupt the linear control flow and make code more difficult to read
> > IMO. Samples do not have to be very code efficient.
> > 
> > May I inline all private static helper methods in the samples?
> 
> Please do not. The samples for using an operator are meant to show
> how to use an operator. The same goes for the samples on using a
> manager, or the client. I trimmed the main() methods down to contain
> exactly that code, and as little besides as possible. Setting up
> parameters and protocol schemes is necessary ado to make the examples
> executable, but it distracts from what is exemplified. That is also
> why these helper methods are after the main method and not before.
> I would hope that this kind of examples encourages newcomers that
> rely on them to structure their applications, rather than to hack
> everything into one big method because they have no guidance on what
> should reasonably be moved into a helper.
> 
> > (2) May I change the samples to make use of HttpRequestExecutor instead
> > writing and reading stuff directly to and from the connection objects? I
> > think we ought not encourage the direct use of connection objects by the
> > end users.
> 
> Are you talking about the examples that show how to use a connection
> on a low level :-? Assuming that the request executor is prepared in
> a helper (;-), I have no problem with the sample request being executed
> that way. Except maybe that it requires more ado, like initializing the
> context. But if that is consistenly the same code in all examples, it
> shouldn't hurt.
> I have some reservations about sending the tunnelling requests
> through the same executor, although that is just a gut feeling for
> the moment. My understanding of what request and response processors
> should be used for and what not has undergone some changes during
> the work on HttpConn and HttpClient. The uneasiness might pass away.
> If you feel comfortable with it, that's good enough for me.
> 
> > (3) I would like to add logging decorators for HttpDataReceiver and
> > HttpDataTransmitter interfaces, so we could have the convenience of wire
> > logging, which I find very useful when developing new stuff.   
> 
> I put a TalkativeSocketFactory in the HttpClient contrib code to achieve
> a similar result, but I never was satisfied with that plug-in point,
> and it would not work well for layered connections. Please go ahead.
> Btw, when I used that approach, it seemed that some characters in the
> header were being written one by one rather than buffered. I don't
> recall what exactly it was, it might have been a header name.
> 

I'll leave the samples alone.

Oleg

> cheers,
>   Roland
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,

as usual, our opinions differ...

> (1) I tend to dislike static helper methods in examples, because they
> disrupt the linear control flow and make code more difficult to read
> IMO. Samples do not have to be very code efficient.
> 
> May I inline all private static helper methods in the samples?

Please do not. The samples for using an operator are meant to show
how to use an operator. The same goes for the samples on using a
manager, or the client. I trimmed the main() methods down to contain
exactly that code, and as little besides as possible. Setting up
parameters and protocol schemes is necessary ado to make the examples
executable, but it distracts from what is exemplified. That is also
why these helper methods are after the main method and not before.
I would hope that this kind of examples encourages newcomers that
rely on them to structure their applications, rather than to hack
everything into one big method because they have no guidance on what
should reasonably be moved into a helper.

> (2) May I change the samples to make use of HttpRequestExecutor instead
> writing and reading stuff directly to and from the connection objects? I
> think we ought not encourage the direct use of connection objects by the
> end users.

Are you talking about the examples that show how to use a connection
on a low level :-? Assuming that the request executor is prepared in
a helper (;-), I have no problem with the sample request being executed
that way. Except maybe that it requires more ado, like initializing the
context. But if that is consistenly the same code in all examples, it
shouldn't hurt.
I have some reservations about sending the tunnelling requests
through the same executor, although that is just a gut feeling for
the moment. My understanding of what request and response processors
should be used for and what not has undergone some changes during
the work on HttpConn and HttpClient. The uneasiness might pass away.
If you feel comfortable with it, that's good enough for me.

> (3) I would like to add logging decorators for HttpDataReceiver and
> HttpDataTransmitter interfaces, so we could have the convenience of wire
> logging, which I find very useful when developing new stuff.   

I put a TalkativeSocketFactory in the HttpClient contrib code to achieve
a similar result, but I never was satisfied with that plug-in point,
and it would not work well for layered connections. Please go ahead.
Btw, when I used that approach, it seemed that some characters in the
header were being written one by one rather than buffered. I don't
recall what exactly it was, it might have been a header name.

cheers,
  Roland


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Re: [HttpConn] another status update

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2007-02-18 at 20:23 +0100, Roland Weber wrote:
> Hi all,
> 
> I've hacked up plenty of connection release code in HttpConn,
> plus some use of that code in HttpClient and examples.

Hi Roland,

I started reviewing the latest HttpConn and HttpClient changes. I tend
to start the review process by taking a look at the examples first just
to get a feel about the API and then gradually work my way deeper into
the implementation classes.  

I have a few immediate questions

(1) I tend to dislike static helper methods in examples, because they
disrupt the linear control flow and make code more difficult to read
IMO. Samples do not have to be very code efficient.

May I inline all private static helper methods in the samples?

(2) May I change the samples to make use of HttpRequestExecutor instead
writing and reading stuff directly to and from the connection objects? I
think we ought not encourage the direct use of connection objects by the
end users.

(3) I would like to add logging decorators for HttpDataReceiver and
HttpDataTransmitter interfaces, so we could have the convenience of wire
logging, which I find very useful when developing new stuff.   

Cheers

Oleg


>  Since
> I was a little bit confused myself when writing that code,
> I'll give you a quick intro in case you want to have a look.
> There are three levels on which a connection can be released:
> 
> (1) Connection Management level
> The connection manager has a releaseConnection method that
> expects the connection as argument. The connection itself
> also has a releaseConnection method and will call back to
> it's connection manager if that is called. Actually there
> are two methods:
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> 
> (2) Entity level
> The new BasicManagedEntity keeps a reference to the connection
> that should be released. There are three methods:
>   consumeContent - gracefully
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> The entity also installs a level 3 notification handler.
> 
> (3) Stream level
> EofSensorInputStream replaces AutoCloseInputStream. There
> are three methods:
>   close - gracefully
>   releaseConnection - gracefully
>   abortConnection - enforces shutdown, no connection re-use
> Actual handling is typically delegated to the watcher.
> 
> 
> I know it sounds tricky. It is. But I don't think we could
> or should drop any of those levels or methods. The two
> methods available on all three levels are defined in
> ConnectionReleaseTrigger to facilitate instanceof checks.
> The additional methods on some levels are required by the
> interfaces implemented.
> A release option on the entity level should be available
> since HttpEntity.getContent() will return the stream only
> once. If a method like EntityUtils.toString(HttpEntity)
> accesses the stream and then fails before reading to the
> end, level 3 can no longer be used to release the connection.
> Dragging level 1 information through your application is
> the situation which we have in HttpClient 3, and we've had
> requests to drop that requirement since years ago.
> It is hard to determine on one level whether the connection
> has already been released on another level. The manager is
> therefore required to tolerate multiple release of a single
> connection.
> 
> Another detail I introduced is a flag in the connection
> that tells the manager whether it is reusable. That's
> meant for tracking the communication states as mentioned
> in [1] and [2], but it also came in handy for implementing
> abortConnection().
> 
> I don't consider that another milestone, but I'm closing
> in on one. At the very least, I want to add support for
> a connection re-use strategy in the request director.
> There will be a little twitch to that one: instead of
> remembering the strategy and arguments required to call it
> until the connection is released, I'd like to evaluate
> the strategy directly after the response header is received
> and only remember the boolean result. That will go a long
> way towards addressing HTTPCLIENT-589. It shouldn't make a
> difference to the strategy, unless that depends on trailers.
> Has anybody ever seen trailers being used in the wild?
> 
> Once that is addressed, the question will be whether I
> should implement a SimpleClientConnectionManager before
> or after writing test cases for ThreadSafeClientConnManager
> and the rest of HttpConn. What do you think?
> Of course I have plenty of ideas for HttpClient, but I
> feel it makes more sense to stabilize HttpConn quickly.
> Test cases are a prerequisite for cleaning up TSCCM.
> 
> I'm signing off for today... Snooker is on :-)
> 
> cheers,
>   Roland
> 
> [1]
> http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B333F3.3010706@dubioso.net%3e
> [2]
> http://mail-archives.apache.org/mod_mbox/jakarta-httpcomponents-dev/200701.mbox/%3c45B361C0.50509@dubioso.net%3e
> [3] https://issues.apache.org/jira/browse/HTTPCLIENT-589
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org