2

I am using RestTemplate as my HttpClient in one of my library. I am not sure whether I am using it correctly in multithreading environment as my library will be used under very heavy load in multithreading environment so it has to be very fast.

Below is my DataClient class:

public class DataClient implements Client {

    private RestTemplate restTemplate = new RestTemplate(clientHttpRequestFactory());
    private ExecutorService executor = Executors.newFixedThreadPool(10);

    // for synchronous call
    @Override
    public DataResponse executeSync(DataKey key) {
        DataResponse dataResponse = null;
        Future<DataResponse> future = null;

        try {
            future = executeAsync(key);
            dataResponse = future.get(key.getTimeout(), TimeUnit.MILLISECONDS);
        } catch (TimeoutException ex) {
            dataResponse = new DataResponse(null, DataErrorEnum.TIMEOUT, DataStatusEnum.ERROR);
            future.cancel(true);
        } catch (Exception ex) {
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }

    //for asynchronous call
    @Override
    public Future<DataResponse> executeAsync(DataKey key) {
        Future<DataResponse> future = null;
        Task task = new Task(key, restTemplate);
        future = executor.submit(task);

        return future;
    }

    // does this looks right?
    private ClientHttpRequestFactory clientHttpRequestFactory() {
        HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory();
        // setting 2000 ms as the default timeout for each Http Request
        RequestConfig requestConfig = RequestConfig.custom().setConnectionRequestTimeout(2000).setConnectTimeout(2000)
                .setSocketTimeout(2000).setStaleConnectionCheckEnabled(false).build();
        SocketConfig socketConfig = SocketConfig.custom().setSoKeepAlive(true).setTcpNoDelay(true).build();

        PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = new PoolingHttpClientConnectionManager();
        poolingHttpClientConnectionManager.setMaxTotal(800);
        poolingHttpClientConnectionManager.setDefaultMaxPerRoute(700);

        CloseableHttpClient httpClientBuilder = HttpClientBuilder.create()
                .setConnectionManager(poolingHttpClientConnectionManager).setDefaultRequestConfig(requestConfig)
                .setDefaultSocketConfig(socketConfig).build();

        requestFactory.setHttpClient(httpClientBuilder);
        return requestFactory;
    }
}

Simple class which will perform the actual task:

public class Task implements Callable<DataResponse> {

    private final DataKey key;
    private final RestTemplate restTemplate;

    public Task(DataKey key, RestTemplate restTemplate) {
        this.key = key;
        this.restTemplate = restTemplate;
    }

    @Override
    public DataResponse call() {
        DataResponse dataResponse = null;
        String response = null;

        try {
            String url = createURL();
            response = restTemplate.getForObject(url, String.class);

            dataResponse = new DataResponse(response, DataErrorEnum.OK, DataStatusEnum.SUCCESS);
        } catch (RestClientException ex) {
            dataResponse = new DataResponse(null, DataErrorEnum.SERVER_DOWN, DataStatusEnum.ERROR);
        } catch (Exception ex) {
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }
}

And below is my factory which I am using to create a single instance of DataClient which means, it will also have single instance of RestTemplate.

public class DataClientFactory {

    private DataClientFactory() {}

    private static class ClientHolder {
        private static final DataClient INSTANCE = new DataClient();
    }

    public static Client getInstance() {
        return ClientHolder.INSTANCE;
    }
}

And this is the way I will make a call to get the data:

DataResponse response = DataClientFactory.getInstance().executeSync(dataKey);

Now my question is - I am not sure whether I am using RestTemplate correctly with HttpComponentsClientHttpRequestFactory. Do I need PoolingHttpClientConnectionManager here along with RestTemplate at all or not?

My main goal is to use RestTemplate efficiently in multithreading environment. As my library will be used under very heavy load so it has to be very fast. As under heavy load, I was seeing lot of TIME_WAIT connections so that's why I added clientHttpRequestFactory() method to use with RestTemplate.

john
  • 9,493
  • 34
  • 111
  • 210

3 Answers3

1

RestTemplate is thread safe in Spring. So what you may want to do is create only one instance of RestTemplate in your application and share it across multiple threads. This is of course assuming that you will use the same HTTP properties(like timeout,set alive etc) for all. In case you need to change the connection properties you can create a pool at app start of RestTemplate objects and use this to inject the RestTemplate instance to the caller class.

Nayanjyoti Deka
  • 369
  • 2
  • 15
  • I am already creating one instance of `RestTemplate` right in my question? I have updated it slightly which tell us how I am using DataClient class. so I am using same HTTP properties for all calls. Can you take a look again into my question? – john Jul 01 '15 at 20:28
  • I see that you are creating a new thread even for the synchronous calls. basically wasting the cost of creating a new thread when its not required. Please look at http://stackoverflow.com/a/29447961/2776345 . – Nayanjyoti Deka Jul 02 '15 at 05:24
0

If all the requests you do on restTemplate will be via the executor ExecutorService executor = Executors.newFixedThreadPool(10);, then this way you manage your restTemplate connection pool yourself. No need for other connection managers.

Yet, it is better if you use the PoolingHttpClientConnectionManager with all the necessary configuration (timeouts, connection number, etc).

In result, you write much less code as you don't need your Fixed Thread Pool Executor anymore, because every request you do on restTemplate will get as (what u did above):

final Future<CPoolEntry> future = this.pool.lease(..)

Finally, if you need asynchronous calls, maybe it is worth trying http://hc.apache.org/httpcomponents-asyncclient-4.1.x/index.html.

DayaMoon
  • 327
  • 2
  • 7
0

RestTemplate itself is thread-safe. However, I note that your private RestTemplate restTemplate is not final. It is therefore not clear that it is safely published from the constructor of DataClient. The reference is never changed, so you can simply change it to final to be sure. Fortunately, the reference will be safely published before any of your tasks try to use it, because ExecutorService makes such a guarantee, so I believe your code is thread safe.

Community
  • 1
  • 1
Raedwald
  • 40,290
  • 35
  • 127
  • 207