Skip to content
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

Default connection pool is poorly configured in ApacheHttpTransport. Oauth2 library #1060

Open
tfeak opened this issue Aug 7, 2017 · 3 comments
Labels
priority: p4 type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tfeak
Copy link

tfeak commented Aug 7, 2017

As is, the default pool doesn't work well in a situation where the pool is infrequently used and causes NoHttpResponseExceptions.

This is because the pool defaults to an infinite keep alive time on the pooled connections when the server isn't providing a keep alive header. If they aren't used frequently, the connection on the Google server side will be closed. The connections aren't checked before re-use and you end up getting exception stacks like the one posted below.

These are totally avoidable by configuring the ConnectionKeepAliveStrategy on the pool OR providing a connection keep alive header value on the server responses. It seems like you should be able to provide a solid default keep alive configuration with knowledge of how long the server will actually keep connections open.

Stack trace below

Caused by: org.apache.http.NoHttpResponseException: www.googleapis.com:443 failed to respond
	at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:143)
	at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:57)
	at org.apache.http.impl.io.AbstractMessageParser.parse(AbstractMessageParser.java:261)
	at org.apache.http.impl.AbstractHttpClientConnection.receiveResponseHeader(AbstractHttpClientConnection.java:283)
	at org.apache.http.impl.conn.DefaultClientConnection.receiveResponseHeader(DefaultClientConnection.java:259)
	at org.apache.http.impl.conn.AbstractClientConnAdapter.receiveResponseHeader(AbstractClientConnAdapter.java:232)
	at org.apache.http.protocol.HttpRequestExecutor.doReceiveResponse(HttpRequestExecutor.java:272)
	at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:124)
	at org.apache.http.impl.client.DefaultRequestDirector.tryExecute(DefaultRequestDirector.java:686)
	at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:488)
	at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:884)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:107)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:55)
	at com.google.api.client.http.apache.ApacheHttpRequest.execute(ApacheHttpRequest.java:67)
	at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:981)
	at com.google.api.client.googleapis.auth.oauth2.GooglePublicKeysManager.refresh(GooglePublicKeysManager.java:172)
	at com.google.api.client.googleapis.auth.oauth2.GooglePublicKeysManager.getPublicKeys(GooglePublicKeysManager.java:141)
	at com.google.api.client.googleapis.auth.oauth2.GoogleIdTokenVerifier.verify(GoogleIdTokenVerifier.java:174)
	at com.google.api.client.googleapis.auth.oauth2.GoogleIdTokenVerifier.verify(GoogleIdTokenVerifier.java:192)
@bit-twit
Copy link

bit-twit commented Nov 10, 2017

I'm just posting this for other people that encounter this error and need a quick solution.

Example of custom HTTP client implementation that can be used with google api.
We use it to get rid of org.apache.http.NoHttpResponseException.

One mention: this is no optimized for high traffic - for high traffic Apache HC suggests to actually catch error and retry if you consider appropiate.
Link: Apache HTTP Client docs

/**
 * Apache HTTP client implementation with:
 * - custom short Keep Alive policy
 * - supports 'retry once' policy
 * - has low number of connections
 * - has stale checking enabled
 */
public class CustomTimeoutHttpClient {
    public static HttpClient newHttpClient() {
        return newDefaultHttpClient(
            SSLSocketFactory.getSocketFactory(), newDefaultHttpParams(), ProxySelector.getDefault());
    }

    /** Returns a new instance of the default HTTP parameters we use. */
    static HttpParams newDefaultHttpParams() {
        HttpParams params = new BasicHttpParams();
        // Turn off stale checking. Our connections break all the time anyway,
        // and it's not worth it to pay the penalty of checking every time.
        HttpConnectionParams.setStaleCheckingEnabled(params, true);
        HttpConnectionParams.setSocketBufferSize(params, 8192);
        ConnManagerParams.setMaxTotalConnections(params, 10);
        ConnManagerParams.setMaxConnectionsPerRoute(params, new ConnPerRouteBean(5));
        return params;
    }

    /**
     *
     * @param socketFactory SSL socket factory
     * @param params HTTP parameters
     * @param proxySelector HTTP proxy selector to use {@link ProxySelectorRoutePlanner} or
     *        {@code null} for {@link DefaultHttpRoutePlanner}
     * @return new instance of the Apache HTTP client
     */
    static DefaultHttpClient newDefaultHttpClient(
        SSLSocketFactory socketFactory, HttpParams params, ProxySelector proxySelector) {
        // See http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html
        SchemeRegistry registry = new SchemeRegistry();
        registry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
        registry.register(new Scheme("https", socketFactory, 443));
        ClientConnectionManager connectionManager = new ThreadSafeClientConnManager(params, registry);
        DefaultHttpClient defaultHttpClient = new DefaultHttpClient(connectionManager, params);
        defaultHttpClient.setHttpRequestRetryHandler(new DefaultHttpRequestRetryHandler(1, true));
        if (proxySelector != null) {
            defaultHttpClient.setRoutePlanner(new ProxySelectorRoutePlanner(registry, proxySelector));
        }
        defaultHttpClient.setKeepAliveStrategy(new ShortKeepAliveStrategy());
        return defaultHttpClient;
    }

    public static class ShortKeepAliveStrategy implements ConnectionKeepAliveStrategy {

        @Override
        public long getKeepAliveDuration(HttpResponse response, HttpContext context) {
            // Keep alive for 5 seconds only
            return 5 * 1000;
        }
    }
}

@mattwhisenhunt mattwhisenhunt added priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jan 8, 2018
@kubukoz
Copy link

kubukoz commented Feb 14, 2018

Just hit it, confirmed that it happens after a long time of not using the connection.

@chenjianjx
Copy link

When will this be fixed in this open source project? Any ETA ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p4 type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

8 participants