[jira] [Commented] (HTTPCLIENT-1857) HttpClient falsely closes a reusable connection

2017-06-19 Thread Rodolfo Udo Labsch (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCLIENT-1857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16055021#comment-16055021
 ] 

Rodolfo Udo Labsch commented on HTTPCLIENT-1857:


[~olegk],

Suppose we cut reading the InputStream in the middle, the following should 
happend:
- close the response object.
- The pool receives back the connection
- The pool checks if the inputstream was read all the way to the end
- if yes, throw away the not used buffer, i.e. read the stream all the way to 
the end but not using any bytes on it.

If after all of this the server stilll holds any state, we do have a defective 
server since HTTP should not work that way.
And like I said, the unused body should be ignored and thrown away, so that we 
may reuse the connection.
And once again, I do not know if it was implemented this way, but seems the 
most logical operating mode to me.

> HttpClient falsely closes a reusable connection
> ---
>
> Key: HTTPCLIENT-1857
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1857
> Project: HttpComponents HttpClient
>  Issue Type: Bug
>Reporter: Rodolfo Udo Labsch
> Attachments: ConnectionHolder.patch
>
>
> If you create a code with a reusable httpclient and then call execute as in 
> the example with the following code. The http connection will be falsely 
> closed.
> {code:java}
> private PoolingHttpClientConnectionManager connectionManager = new 
> PoolingHttpClientConnectionManager();
> httpClient = HttpClients.custom()
> .setConnectionManager(connectionManager)
> .build();
> try (CloseableHttpResponse response = httpClient.execute(new 
> HttpGet(enetLink), context)) {
> .
> }
> {code}
> The reason being that we have:
> {code:java}
> CloseableHttpResponse:
> public void close() throws IOException {
> if (this.connHolder != null) {
> this.connHolder.close();
> }
> }
> ConnectionHolder:
> public void close() throws IOException {
> releaseConnection(false);
> }
> {code}
> Just created the correction, which is attached as patch.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Comment Edited] (HTTPCLIENT-1857) HttpClient falsely closes a reusable connection

2017-06-19 Thread Rodolfo Udo Labsch (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCLIENT-1857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16055021#comment-16055021
 ] 

Rodolfo Udo Labsch edited comment on HTTPCLIENT-1857 at 6/20/17 1:01 AM:
-

[~olegk],

Suppose we cut reading the InputStream in the middle, the following should 
happend:
- close the response object.
- The pool receives back the connection
- The pool checks if the inputstream was read all the way to the end
- if not, throw away the not used buffer, i.e. read the stream all the way to 
the end but not using any bytes on it.

If after all of this the server stilll holds any state, we do have a defective 
server since HTTP should not work that way.
And like I said, the unused body should be ignored and thrown away, so that we 
may reuse the connection.
And once again, I do not know if it was implemented this way, but seems the 
most logical operating mode to me.


was (Author: devilus):
[~olegk],

Suppose we cut reading the InputStream in the middle, the following should 
happend:
- close the response object.
- The pool receives back the connection
- The pool checks if the inputstream was read all the way to the end
- if yes, throw away the not used buffer, i.e. read the stream all the way to 
the end but not using any bytes on it.

If after all of this the server stilll holds any state, we do have a defective 
server since HTTP should not work that way.
And like I said, the unused body should be ignored and thrown away, so that we 
may reuse the connection.
And once again, I do not know if it was implemented this way, but seems the 
most logical operating mode to me.

> HttpClient falsely closes a reusable connection
> ---
>
> Key: HTTPCLIENT-1857
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1857
> Project: HttpComponents HttpClient
>  Issue Type: Bug
>Reporter: Rodolfo Udo Labsch
> Attachments: ConnectionHolder.patch
>
>
> If you create a code with a reusable httpclient and then call execute as in 
> the example with the following code. The http connection will be falsely 
> closed.
> {code:java}
> private PoolingHttpClientConnectionManager connectionManager = new 
> PoolingHttpClientConnectionManager();
> httpClient = HttpClients.custom()
> .setConnectionManager(connectionManager)
> .build();
> try (CloseableHttpResponse response = httpClient.execute(new 
> HttpGet(enetLink), context)) {
> .
> }
> {code}
> The reason being that we have:
> {code:java}
> CloseableHttpResponse:
> public void close() throws IOException {
> if (this.connHolder != null) {
> this.connHolder.close();
> }
> }
> ConnectionHolder:
> public void close() throws IOException {
> releaseConnection(false);
> }
> {code}
> Just created the correction, which is attached as patch.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Commented] (HTTPCORE-469) Use ReentrantReadWriteLock in AbstractConnPool

2017-06-19 Thread Matt Nelson (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCORE-469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16054222#comment-16054222
 ] 

Matt Nelson commented on HTTPCORE-469:
--

I'm ok with waiting for the lockless connection pool. I have not noticed any 
performance issues with the current implementation for my use cases, but 
thought I'd put out the recommendation for the lock refactoring.

> Use ReentrantReadWriteLock in AbstractConnPool
> --
>
> Key: HTTPCORE-469
> URL: https://issues.apache.org/jira/browse/HTTPCORE-469
> Project: HttpComponents HttpCore
>  Issue Type: Improvement
>  Components: HttpCore
>Reporter: Matt Nelson
>Assignee: Oleg Kalnichevski
>Priority: Minor
>  Labels: stuck, volunteers-wanted
> Fix For: Future
>
>
> AbstractConnPool is currently using a {{ReentrantLock}} which has to lock for 
> read and write operations. Switching to {{ReentrantReadWriteLock}}[1] and 
> read locks for the stats/getters methods would reduce the possibility for 
> instrumentation[1] to cause contention. Another option would be 
> {{StampedLock}}[3] if the compile target is 1.8.
> [1] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
> [2] 
> https://github.com/dropwizard/metrics/blob/v3.2.2/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java#L63-L95
> [3] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/StampedLock.html



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Commented] (HTTPCORE-469) Use ReentrantReadWriteLock in AbstractConnPool

2017-06-19 Thread Gary Gregory (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCORE-469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16054184#comment-16054184
 ] 

Gary Gregory commented on HTTPCORE-469:
---

I agree. If someone wants to step in and do the works, great, otherwise let's 
leave it. It seems tricky to get this kind of code right and trickier to prove 
you've done it right with unit tests.

> Use ReentrantReadWriteLock in AbstractConnPool
> --
>
> Key: HTTPCORE-469
> URL: https://issues.apache.org/jira/browse/HTTPCORE-469
> Project: HttpComponents HttpCore
>  Issue Type: Improvement
>  Components: HttpCore
>Reporter: Matt Nelson
>Assignee: Oleg Kalnichevski
>Priority: Minor
>  Labels: stuck, volunteers-wanted
> Fix For: Future
>
>
> AbstractConnPool is currently using a {{ReentrantLock}} which has to lock for 
> read and write operations. Switching to {{ReentrantReadWriteLock}}[1] and 
> read locks for the stats/getters methods would reduce the possibility for 
> instrumentation[1] to cause contention. Another option would be 
> {{StampedLock}}[3] if the compile target is 1.8.
> [1] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
> [2] 
> https://github.com/dropwizard/metrics/blob/v3.2.2/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java#L63-L95
> [3] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/StampedLock.html



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Commented] (HTTPCORE-459) Make request available to ResponseHandlers

2017-06-19 Thread Oleg Kalnichevski (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCORE-459?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16054048#comment-16054048
 ] 

Oleg Kalnichevski commented on HTTPCORE-459:


Tobias,

Because I do not see a problem here. There are multiple ways of making request 
(or any other contextual attributes) available to {{ResponseHandler}} s.

Oleg

> Make request available to ResponseHandlers
> --
>
> Key: HTTPCORE-459
> URL: https://issues.apache.org/jira/browse/HTTPCORE-459
> Project: HttpComponents HttpCore
>  Issue Type: Improvement
>Affects Versions: 5.0-alpha3
>Reporter: Tobias Oberlies
>
> We use HttpClients for a system test of a REST API. We are using 
> ResponseHandlers to directly convert the response entity to a data structure 
> that is suitable for assertions.
> This works very well, except for the occasional case where the system under 
> test responds with an unexpected status code. In this case, the response 
> handler throws an exception. For a good error message, it would be useful to 
> also include the request URL. However the request object is not available in 
> the ResponseHandler.handleResponse method.
> So this is a request to also make the HttpRequest object available in the 
> ResponseHandler.handleResponse method. This could be done by adding a getter 
> in the HttpResponse class, or by creating a new interface (e.g. 
> HttpResponseHandler2) with a two-parameter handleResponse method.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Commented] (HTTPCORE-459) Make request available to ResponseHandlers

2017-06-19 Thread Tobias Oberlies (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCORE-459?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16054018#comment-16054018
 ] 

Tobias Oberlies commented on HTTPCORE-459:
--

Oleg, why do you consider this "Not a problem"?

> Make request available to ResponseHandlers
> --
>
> Key: HTTPCORE-459
> URL: https://issues.apache.org/jira/browse/HTTPCORE-459
> Project: HttpComponents HttpCore
>  Issue Type: Improvement
>Affects Versions: 5.0-alpha3
>Reporter: Tobias Oberlies
>
> We use HttpClients for a system test of a REST API. We are using 
> ResponseHandlers to directly convert the response entity to a data structure 
> that is suitable for assertions.
> This works very well, except for the occasional case where the system under 
> test responds with an unexpected status code. In this case, the response 
> handler throws an exception. For a good error message, it would be useful to 
> also include the request URL. However the request object is not available in 
> the ResponseHandler.handleResponse method.
> So this is a request to also make the HttpRequest object available in the 
> ResponseHandler.handleResponse method. This could be done by adding a getter 
> in the HttpResponse class, or by creating a new interface (e.g. 
> HttpResponseHandler2) with a two-parameter handleResponse method.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Updated] (HTTPCORE-469) Use ReentrantReadWriteLock in AbstractConnPool

2017-06-19 Thread Oleg Kalnichevski (JIRA)

 [ 
https://issues.apache.org/jira/browse/HTTPCORE-469?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oleg Kalnichevski updated HTTPCORE-469:
---
   Labels: stuck volunteers-wanted  (was: )
Fix Version/s: Future

I looked at the possibility of replacing {{ReentrantLock}} with 
{{ReentrantReadWriteLock}}. Most of connection pool methods would require write 
access. Only very few methods could be potentially considered read-only and 
would benefit from a separate read lock. I also suspect write lock acquisition 
with {{ReentrantReadWriteLock}} might be somewhat more expensive than with 
{{ReentrantLock}} thus making the pool slower when leasing / releasing 
connection. 

I am more inclined to put my efforts into HTTPCORE-390. Unless someone else 
steps in and volunteers to do the necessary research and development I am 
leaning toward closing this issue as WONTFIX.

Oleg

> Use ReentrantReadWriteLock in AbstractConnPool
> --
>
> Key: HTTPCORE-469
> URL: https://issues.apache.org/jira/browse/HTTPCORE-469
> Project: HttpComponents HttpCore
>  Issue Type: Improvement
>  Components: HttpCore
>Reporter: Matt Nelson
>Assignee: Oleg Kalnichevski
>Priority: Minor
>  Labels: stuck, volunteers-wanted
> Fix For: Future
>
>
> AbstractConnPool is currently using a {{ReentrantLock}} which has to lock for 
> read and write operations. Switching to {{ReentrantReadWriteLock}}[1] and 
> read locks for the stats/getters methods would reduce the possibility for 
> instrumentation[1] to cause contention. Another option would be 
> {{StampedLock}}[3] if the compile target is 1.8.
> [1] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
> [2] 
> https://github.com/dropwizard/metrics/blob/v3.2.2/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java#L63-L95
> [3] 
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/StampedLock.html



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org



[jira] [Commented] (HTTPCLIENT-1857) HttpClient falsely closes a reusable connection

2017-06-19 Thread Oleg Kalnichevski (JIRA)

[ 
https://issues.apache.org/jira/browse/HTTPCLIENT-1857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16053555#comment-16053555
 ] 

Oleg Kalnichevski commented on HTTPCLIENT-1857:
---

bq. the content should actually be ignored but the connection should still be 
reusable

If the remaining content is simply ignored the next message request / response 
exchange will not be properly delineated as the connection will still hold bits 
of the previous response body.

Oleg

> HttpClient falsely closes a reusable connection
> ---
>
> Key: HTTPCLIENT-1857
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1857
> Project: HttpComponents HttpClient
>  Issue Type: Bug
>Reporter: Rodolfo Udo Labsch
> Attachments: ConnectionHolder.patch
>
>
> If you create a code with a reusable httpclient and then call execute as in 
> the example with the following code. The http connection will be falsely 
> closed.
> {code:java}
> private PoolingHttpClientConnectionManager connectionManager = new 
> PoolingHttpClientConnectionManager();
> httpClient = HttpClients.custom()
> .setConnectionManager(connectionManager)
> .build();
> try (CloseableHttpResponse response = httpClient.execute(new 
> HttpGet(enetLink), context)) {
> .
> }
> {code}
> The reason being that we have:
> {code:java}
> CloseableHttpResponse:
> public void close() throws IOException {
> if (this.connHolder != null) {
> this.connHolder.close();
> }
> }
> ConnectionHolder:
> public void close() throws IOException {
> releaseConnection(false);
> }
> {code}
> Just created the correction, which is attached as patch.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org