New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Http Client Connection Pool Error #1160

Closed
ruimateus opened this Issue Jul 11, 2015 · 11 comments

Comments

Projects
None yet
7 participants
@ruimateus

ruimateus commented Jul 11, 2015

Hi,
I have a series of Tasks that connect to multiple targets that have an hourly cronjob running them.

The multiple tasks are using the same JerseyClient instance from JerseyClientConfiguration.

After a while, I start getting javax.ws.rs.ProcessingException: java.lang.IllegalStateException: Connection pool shut down

The only way to recover is restarting the application.

This only started to occur in Dropwizard 0.8.2.

Thanks for any help!

@ruimateus

This comment has been minimized.

Show comment
Hide comment
@ruimateus

ruimateus Jul 11, 2015

After further investigation, it seems that InstrumentedHttpClientConnectionManager shuts down the pool for an unknown reason:

DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.conn.HttpClientConnectionOperator: Connecting to ****/****
DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.execchain.MainClientExec: Connection discarded
DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.conn.DefaultManagedHttpClientConnection: http-outgoing-6409: Close connection
DEBUG [2015-07-11 14:12:44,652] com.codahale.metrics.httpclient.InstrumentedHttpClientConnectionManager: Connection released: [id: 6409][route: {s}->****:443][total kept alive: 0; route allocated: 7 of 1024; total allocated: 21 of 1024]
DEBUG [2015-07-11 14:12:44,653] com.codahale.metrics.httpclient.InstrumentedHttpClientConnectionManager: Connection manager is shutting down

After this point, any HttpClient call, returns an exception _java.lang.IllegalStateException: Connection pool shut down_

PoolingHttpClientConnectionManager shuts down on close() and finalize() method.

When does Dropwizard closes the CloseableHttpClient?

Thanks

ruimateus commented Jul 11, 2015

After further investigation, it seems that InstrumentedHttpClientConnectionManager shuts down the pool for an unknown reason:

DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.conn.HttpClientConnectionOperator: Connecting to ****/****
DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.execchain.MainClientExec: Connection discarded
DEBUG [2015-07-11 14:12:44,652] org.apache.http.impl.conn.DefaultManagedHttpClientConnection: http-outgoing-6409: Close connection
DEBUG [2015-07-11 14:12:44,652] com.codahale.metrics.httpclient.InstrumentedHttpClientConnectionManager: Connection released: [id: 6409][route: {s}->****:443][total kept alive: 0; route allocated: 7 of 1024; total allocated: 21 of 1024]
DEBUG [2015-07-11 14:12:44,653] com.codahale.metrics.httpclient.InstrumentedHttpClientConnectionManager: Connection manager is shutting down

After this point, any HttpClient call, returns an exception _java.lang.IllegalStateException: Connection pool shut down_

PoolingHttpClientConnectionManager shuts down on close() and finalize() method.

When does Dropwizard closes the CloseableHttpClient?

Thanks

@mikeycmccarthy

This comment has been minimized.

Show comment
Hide comment
@mikeycmccarthy

mikeycmccarthy Jul 17, 2015

Contributor

This also affected us - we had to rollback to 0.8.1.

Contributor

mikeycmccarthy commented Jul 17, 2015

This also affected us - we had to rollback to 0.8.1.

@joschi joschi added the bug label Jul 17, 2015

@joschi joschi added this to the 0.8.3 milestone Jul 17, 2015

@nickbabcock

This comment has been minimized.

Show comment
Hide comment
@nickbabcock

nickbabcock Jul 21, 2015

Contributor

I tried to reproduce the issue locally and couldn't get the behavior seen. Does anyone have a gist or repo demonstrating the bug?

Contributor

nickbabcock commented Jul 21, 2015

I tried to reproduce the issue locally and couldn't get the behavior seen. Does anyone have a gist or repo demonstrating the bug?

@carlo-rtr

This comment has been minimized.

Show comment
Hide comment
@carlo-rtr

carlo-rtr Jul 21, 2015

Member

This probably not very helpful, but I figured I would list the places I believe close could be called:

  • Jetty shuts down
  • InstrumentedHttpClientConnectionManager gets garbage collected and finalize is triggered
  • DropwizardApacheConnector#close
  • One thread calls close on the Client
Member

carlo-rtr commented Jul 21, 2015

This probably not very helpful, but I figured I would list the places I believe close could be called:

  • Jetty shuts down
  • InstrumentedHttpClientConnectionManager gets garbage collected and finalize is triggered
  • DropwizardApacheConnector#close
  • One thread calls close on the Client
@nickbabcock

This comment has been minimized.

Show comment
Hide comment
@nickbabcock

nickbabcock Jul 21, 2015

Contributor

I believe you are correct (especially because the default is false):

Contributor

nickbabcock commented Jul 21, 2015

I believe you are correct (especially because the default is false):

@carlo-rtr

This comment has been minimized.

Show comment
Hide comment
@carlo-rtr

carlo-rtr Jul 21, 2015

Member

hotness

Member

carlo-rtr commented Jul 21, 2015

hotness

@arteam

This comment has been minimized.

Show comment
Hide comment
@arteam

arteam Jul 22, 2015

Member

I don't think that's relevant.

The problem described in the Jersey issue is applied only for cases, when user provides own connection manager that's shared between clients. We don't provide such ability, because we create a new connection manager every time user creates a new instance through JerseyClientBuilder. Moreover, the current fix leaks InstrumentedHttpClientConnectionManager, which will be closed only during a major GC - a highly unpredictable event. It's really easy to end up with a couple of hundred open TCP connections to a remote server.

I think, we should revert connection manager sharing or at least disable it by default.

Member

arteam commented Jul 22, 2015

I don't think that's relevant.

The problem described in the Jersey issue is applied only for cases, when user provides own connection manager that's shared between clients. We don't provide such ability, because we create a new connection manager every time user creates a new instance through JerseyClientBuilder. Moreover, the current fix leaks InstrumentedHttpClientConnectionManager, which will be closed only during a major GC - a highly unpredictable event. It's really easy to end up with a couple of hundred open TCP connections to a remote server.

I think, we should revert connection manager sharing or at least disable it by default.

@carlo-rtr

This comment has been minimized.

Show comment
Hide comment
@carlo-rtr

carlo-rtr Jul 22, 2015

Member

Okay, I've reverted. Do you have any idea what's causing the issue?

Member

carlo-rtr commented Jul 22, 2015

Okay, I've reverted. Do you have any idea what's causing the issue?

@arteam

This comment has been minimized.

Show comment
Hide comment
@arteam

arteam Jul 22, 2015

Member

I would go with reviewing the changes in Jersey between 2.17 and 2.19, if the issue is reproduced only in 0.8.2. But I have a gut feeling that the issue is connected to the finalizer in some way.

Member

arteam commented Jul 22, 2015

I would go with reviewing the changes in Jersey between 2.17 and 2.19, if the issue is reproduced only in 0.8.2. But I have a gut feeling that the issue is connected to the finalizer in some way.

@krutsko

This comment has been minimized.

Show comment
Hide comment
@krutsko

krutsko Aug 5, 2015

@arteam We faced the same issue with dropwizard 0.8.2.
I noticed, that finalizer was added into ClientRuntime of Jersey 2.18. https://github.com/jersey/jersey/blob/4eece8eeb3947becdf91188a63412b5011eae2b5/core-client/src/main/java/org/glassfish/jersey/client/ClientRuntime.java#L297 which close() the connector.
Then, underlying apache client 4.4.1 http://grepcode.com/file/repo1.maven.org/maven2/org.apache.httpcomponents/httpclient/4.4.1/org/apache/http/impl/client/HttpClientBuilder.java#1184 close() the connection manager, cause it's not shared by default.
So, the patch provided by @carlo-rtr seems valid to me. What do you think, guys?

krutsko commented Aug 5, 2015

@arteam We faced the same issue with dropwizard 0.8.2.
I noticed, that finalizer was added into ClientRuntime of Jersey 2.18. https://github.com/jersey/jersey/blob/4eece8eeb3947becdf91188a63412b5011eae2b5/core-client/src/main/java/org/glassfish/jersey/client/ClientRuntime.java#L297 which close() the connector.
Then, underlying apache client 4.4.1 http://grepcode.com/file/repo1.maven.org/maven2/org.apache.httpcomponents/httpclient/4.4.1/org/apache/http/impl/client/HttpClientBuilder.java#1184 close() the connection manager, cause it's not shared by default.
So, the patch provided by @carlo-rtr seems valid to me. What do you think, guys?

@carlo-rtr carlo-rtr closed this in #1232 Aug 24, 2015

carlo-rtr added a commit to carlo-rtr/dropwizard that referenced this issue Aug 24, 2015

arteam added a commit that referenced this issue Aug 25, 2015

Don't close the client instance in DropwizardApacheConnector
Jersey expects that we create a new instance of the Apache HTTP client
every time, when we create a new instance of the connector.

So, when `ClientRuntime` is garbage collected, the client is closed
and new requests, that work with a new connector, use already closed client.

The fix is to not close the client in the connector. We can rely on the fact,
that the client is managed by Dropwizard environment and it will be closed,
when the application will be shut down.

Also users of Apache HTTP Client keep the ability to close the client manually,
if they want to.

Fix #1160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment