Skip to content

Conversation

@nicktindall
Copy link
Contributor

@nicktindall nicktindall commented Feb 18, 2025

POC to use the Apache HTTP client implementation in the GCS repository client (considered as an approach to work around some limitations of the GCS interceptor)

Clients share a connection pool but nothing else. We could perhaps have the GCS clients share Apache HTTP clients once we start making client-per-purpose.

This change seems to struggle with the tests where the server doesn't respond. Apache seems to throw an exception under those circumstances which doesn't get retried. It also seems to have issues with stale connections in the pool when the server doesn't respond.

I'm not sure whether this is

  • An issue with the apache GCS storage implementation (it is marked as Beta)
  • Something I've misconfigured
  • Something we can work around

I tried setting connectionReuseStrategy to NoConnectionReuseStrategy to see if it was an issue with the pooling, which seemed to improve things, but the errors persisted. I suspect the problem is not due to the pooling.

But I'm going to do what I can with the existing client and leave this for later.

final PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
connectionManager.setMaxTotal(maxOpenConnections);
connectionManager.setDefaultMaxPerRoute(maxOpenConnections);
return connectionManager;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no setting to purge idle connections from the pool (see "Connection Eviction Policy" here).

We would have to schedule a task to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.apache.http.impl.client.HttpClientBuilder#evictExpiredConnections doesn't work if we want to share a thread pool.

Please note this method has no effect if the instance of HttpClient is configuted[sic] to use a shared connection manager.

@nicktindall nicktindall requested a review from mhl-b February 18, 2025 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants