[FEATURE]ApiClient should have an overloaded constructor to accept OkHttpClient object #482
Description
Is your feature request related to a problem? Please describe.
- With Okhttp version 3, each object of OkHttpClient creates its own:
- Dispatcher → backed by a thread pool (executorService)
- ConnectionPool → manages keep-alive sockets
- Cache (if configured)
-
The class software.amazon.spapi.ApiClient initializes a default OkHttpClient object in its constructor.
-
By creating a fresh OkHttpClient per ingestion, we’re also creating a new dispatcher thread pool. This creates following problems:
- Thread exhaustion: OS can’t handle that many live threads, so we hit OutOfMemoryError
- Memory bloat: Each client carries thread stacks, pools, and internal structures.
- Shutdown overhead: Constantly creating and tearing down pools adds latency.
- OkHttp is designed for client reuse:
- OkHttpClient is thread-safe and intended to be a singleton for the lifetime of the app.
- We should not create one per request/ingestion. Instead:
- Create one shared OkHttpClient bean (singleton in Spring, for example).
- Use it across all the Api instances.
Describe the solution you'd like
The ApiClient class should have an overloaded constructor that excepts OkHttpClient object. We will then be able to create a singleton bean of OkHttpClient and inject the same bean for every ApiClient object.
public class ApiClient {
...
private OkHttpClient httpClient;
...
/*
* Default Constructor for ApiClient
*/
public ApiClient() {
httpClient = new OkHttpClient();
...
}
/*
* Overloaded Constructor for ApiClient
*/
public ApiClient(OkHttpClient httpClient) {
this.httpClient = httpClient;
...
}
...
}
With this constructor in place, each time when we have to create an Api object, we will be able to pass the singleton bean:
ApiClient client = new ApiClient(okHttpClientBean)
.setBasePath(region.getSellingPartnerEndPoint())
.setLWAAuthorizationSigner(new LWAAuthorizationSigner(lwaAuthorizationCredentials));
CatalogApi catalogApi = new CatalogApi(client);
Describe alternatives you've considered
We tried injecting the OkHttpClient singleton bean after creating ApiClient object and kill the resources of the default httpClient object:
CatalogApi catalogApi = new CatalogApi.Builder()
.lwaAuthorizationCredentials(lwaAuthorizationCredentials)
.endpoint(region.getSellingPartnerEndPoint())
.build();
// capture the temp client before replacing
OkHttpClient tmp = catalogApi.getApiClient().getHttpClient();
// Use shared OkHttpClient to prevent thread proliferation
catalogApi.getApiClient().setHttpClient(okHttpClientBean);
// shutdown the throwaway client
tmp.dispatcher().executorService().shutdown();
tmp.connectionPool().evictAll();
With this implementation there is some improvement in the memory usage but still lot of threads are created and terminated.
Additional context
We are doing ingestions for multiple customers, so we cannot create a single Api object and use it each time. The credentials change for each customer.
In OkHttpClient version 2 (com.squareup.okhttp) the resources were shared between multiple instances of OkHttpClient so this issue was not faced.