Bug 929198 - RFE: [as7] Use Commons HttpClient in ASConnection instead of JDK's HttpUrlConnection
Summary: RFE: [as7] Use Commons HttpClient in ASConnection instead of JDK's HttpUrlCon...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Plugins
Version: 4.6
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ---
: RHQ 4.7
Assignee: Thomas Segismont
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On:
Blocks: 929200
TreeView+ depends on / blocked
 
Reported: 2013-03-29 13:21 UTC by Thomas Segismont
Modified: 2013-09-03 14:44 UTC (History)
1 user (show)

Fixed In Version:
Clone Of:
: 929200 (view as bug list)
Environment:
Last Closed: 2013-09-03 14:44:33 UTC
Embargoed:


Attachments (Terms of Use)

Description Thomas Segismont 2013-03-29 13:21:39 UTC
In AS7 plugin, ASConnection class currently uses JDK's HttpUrlConnection to communicate with an AS7 instance.

This class should be updated to use Commons HttpClient, like ASUploadConnection class does since release 4.6 (see BZ887320)

Commons HttpClient gives us more control on the communication process and has proven to be more reliable.

Comment 1 Thomas Segismont 2013-04-08 21:36:43 UTC
Fixed in master - 21ec5e2

AS7 plugin itests passed succesfully

Comment 2 Thomas Segismont 2013-04-09 10:07:45 UTC
Reworked to increased delay in AS7 itests to discover all resources
master - 3b18013

Comment 3 Thomas Segismont 2013-04-10 12:17:39 UTC
All as7 plugin unit and integration tests pass on my box and on Jenkins:
https://jenkins.mw.lab.eng.bos.redhat.com/hudson/view/RHQ/job/rhq-master/org.rhq$rhq-jboss-as-7-plugin/

Comment 4 Thomas Segismont 2013-04-25 10:03:44 UTC
Additional commit pushed to master on 2013-04-17 - 915ab3d

Keepalive timeout paramater was too high. The server could close connection server-side and the agent would still try to reuse it. Now HttpClient uses the same value as HttpUrlConnection did to expire connections (5 seconds if the server does not send its keepalive timeout in reponse headers).

------------------

Comments on implementation choices:

* Stale connection check

As advised in HttpClient documentation, stale connection check was disabled. Stale connection check adds a 10-30ms overhead per request and is not 100% reliable. It is advised to use an appropriate keepalive strategy instead.

While things like simple avail checks would not be too much affected, the discovery of a fully configured EAP server would suffer performance problems (thousands of requests issued).

See
http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html#d5e399
And 
http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html#d5e652

* Keep alive strategy

ASConnection's keepalive strategy extends HttpClient's DefaultConnectionKeepAliveStrategy.

A default keepalive timeout is applied if the server does not indicate its value in response header. AS7 does not indicate its value in responses. The default value was chosen to reproduce the behavior of the previous implementation based on JDK's HttpUrlConnection.

See http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html#d5e659

It has been raised that the keepalive timeout could be configured to something different than the default value server side thus possibly breaking ASConnection keepalive strategy.

It's not easy to do this. JBoss Web 7.x no longer allows to configure keepAliveTimeout at the http connector level (see https://access.redhat.com/site/solutions/110003). One has to set a system property, org.apache.coyote.http11.DEFAULT_KEEP_ALIVE_TIMEOUT, via the CLI (see http://docs.jboss.org/jbossweb/7.0.x/sysprops.html). I did not find any documentation on customer portal describing how to do that.

In case this would really happen, BZ955014 has been created. So the keepAliveTimeout will be configurable as a plugin config property.

Comment 5 Heiko W. Rupp 2013-09-03 14:44:33 UTC
Bulk closing of issues in old RHQ releases that are in production for a while now.

Please open a new issue when running into an issue.


Note You need to log in before you can comment on or make changes to this bug.