2296 improve reuse connection strategy #2487
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Improves the brutal no reuse connection strategy initially implemented in #2471.
The default reuse connection strategy was causing ProxyServerTest to fail and I think the reason is:
when a proxy is used,
ProxyRemoteHandler
is called, but it closes the channel when the response is set (See last line inProxyRemoteHandler.channelRead0
).But the apache http client is not notified and tries to re-use the connection, hence the error on subsequent calls.
The new proposed strategy therefore closes the connection when a proxy is used, and delegates to the default strategy in all other cases. It might be possibly to be even smarter and take nonProxyHosts and a few other parameters into account too, but this implementation should be good enough, I believe.
Logs from this PR, using hc5 version 5.4.alpha1 because version 5.3 has an annoying bug causing NPEs in DEBUG:
mvn test -Dtest=ProxyServerTest
Connection is closed after each call. This is on purpose whenever a proxy is used, as ProxyRemoteHandler closes the channel, hence causing SocketException if the connection is re-used.
mvn test -Dtest=FeatureServerTest
the first 6 calls, all within the same Scenario, use the same http-outgoing-0 connection.
The connection is released after the call, but not closed.
Calls #7 and #8, which belong to different Scenarios, will create one connection per Scenario.
mvn test -Dtest=FeatureServerTest
against current master:Connection is closed after each call. So is the connection manager, by the way.