23

I am working on a project in which I need to make a HTTP URL call to my server which is running Restful Service which returns back the response as a JSON String.

Below is my main code which is using the future and callables -

public class TimeoutThreadExample {

    private ExecutorService executor = Executors.newFixedThreadPool(10);

    public String getData() {
        Future<String> future = executor.submit(new Task());
        String response = null;

        try {
            response = future.get(100, TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {
            e.printStackTrace();
        } catch (InterruptedException e) {
            e.printStackTrace();
        } catch (ExecutionException e) {
            e.printStackTrace();
        }

        return response;
    }
}

Below is my Task class which implements the Callable interface and uses the RestTemplate...

class Task implements Callable<String> {

    private RestTemplate restTemplate = new RestTemplate();

    public String call() throws Exception {

        String url = "some_url";
        String response = restTemplate.getForObject(url, String.class);

        return response;
    }
}

And now I have below code in another class DemoTest which calls the getData method in TimeoutThreadExample class 5000 times sequentially -

public class DemoTest { 
   public static void main(String[] args) {

        TimeoutThreadExample bc = new TimeoutThreadExample();

        for (int i = 0; i <= 5000; i++) {
        //  TimerTest timer = TimerTest.getInstance(); // line 1
            bc.getData();
        //  timer.getDuration(); // line 2
        }
    }
}       

So my question is should RestTemplate be static here in my Task class as if I see it correctly, I am recreating the whole connection pool for each request in RestTemplate which is not the right way I guess..

NOTE: If I am making RestTemplate static, then I see better performance end to end as compared to non static RestTemplate after commenting out line1 and line2 in DemoTest class which measures the performance.

In general what is the right way to use RestTemplate in Multithreading environment? Currently I am calling sequentially getData method 5000 times one by one but some customer will call it in a multithreaded way so need to know what is the best way to have RestTemplate in a multithreaded environment..

May be to use ConnectionFactory in the RestTemplate constructor? Any thoughts?

UPDATE:-

public class TimeoutThreadExample {

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

    public String getData() {
        Future<String> future = executor.submit(new Task(restTemplate));
        String response = null;

        try {
            response = future.get(100, TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {
            e.printStackTrace();
        } catch (InterruptedException e) {
            e.printStackTrace();
        } catch (ExecutionException e) {
            e.printStackTrace();
        }

        return response;
    }
}

And below my TaskClass -

class Task implements Callable<String> {

    private RestTemplate restTemplate;

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

    public String call() throws Exception {

        String url = "some_url";
        String response = restTemplate.getForObject(url, String.class);

        return response;
    }
}
AKIWEB
  • 19,008
  • 67
  • 180
  • 294
  • possible duplicate of [How to improve the performance while using ExecutorService with thread timeout capabilities?](http://stackoverflow.com/questions/21241036/how-to-improve-the-performance-while-using-executorservice-with-thread-timeout-c) – Sergey Morozov Jan 20 '14 at 20:01

3 Answers3

26

Correct me if I didn't understand your question. It seems very similar to the previous one here.

There, we determined that RestTemplate is thread-safe. There is therefore no reason not to share it wherever it makes sense to, ie. wherever you are using it in the same way. Your example seems like the perfect place to do so.

As you stated, recreating a new instance of RestTemplate for each Task instance is wasteful.

I would create the RestTemplate in TimeoutThreadExample and pass it to the Task as a constructor argument.

class Task implements Callable<String> {

    private RestTemplate restTemplate;

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

    public String call() throws Exception {

        String url = "some_url";
        String response = restTemplate.getForObject(url, String.class);

        return response;
    }
}

This way you share the RestTemplate instance between all your Task objects.

Note that RestTemplate uses SimpleClientHttpRequestFactory to create its connections.

Community
  • 1
  • 1
Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
  • I see.. So in short they both are same.. making a static in Task class? or using DI by passing around the objects? – AKIWEB Jan 20 '14 at 19:56
  • 2
    RestTemplate is thread-safe but you also have to be sure the underling http connection manager is threadsafe, for e.g. using `org.apache.commons.httpclient.MultiThreadedHttpConnectionManager` (please note I'm using commons-httpclient 3.1, so this may not apply to other versions) – Taylor Jan 20 '14 at 19:57
  • Another close duplicate: http://stackoverflow.com/a/21241233/1103872. Apparently OP has two identities here. – Marko Topolnik Jan 20 '14 at 19:57
  • @AKIWEB In that sense, yes. – Sotirios Delimanolis Jan 20 '14 at 19:58
  • @MarkoTopolnik: I opened this question here since I had similar question here on the RestTemplate which Sotirios pointed out with resttemplate so wanted to be on the same flow.. I am not duplicating any stuff here just following the proper way of not duplicating stuff, otherwise I would have posted it from that accont itself.... – AKIWEB Jan 20 '14 at 20:00
  • @Taylor Can you go into more detail about what you mean? All I see is that `RestTemplate` uses `SimpleClientHttpRequestFactory` to generate its connections and requests. – Sotirios Delimanolis Jan 20 '14 at 20:01
  • Yeah, and that's not thread safe. I'll add an answer with my spring config for this. – Taylor Jan 20 '14 at 20:02
  • @Taylor Why is it not thread safe? It uses `URL#openConnection()`. – Sotirios Delimanolis Jan 20 '14 at 20:02
  • @SotiriosDelimanolis: See my update in the question, my above example is fine and it is along the line as you have stated correct? – AKIWEB Jan 20 '14 at 20:10
  • @AKIWEB That is what I was suggesting, yes. But I still want to see what Taylor has to say about lack of thread-safety. – Sotirios Delimanolis Jan 20 '14 at 20:11
  • Perhaps this has been fixed in some more recent version, but we tried to use this multi threaded and got corrupted responses when concurrent requests were fired. – Taylor Jan 20 '14 at 20:12
  • I would also avoid JDK's URLConnections for anything nontrivial. The implementation is incredibly convoluted (just go ahead and try to keep a bearing while tracing an `openConnection` call through the source). It is also not fully configurable. – Marko Topolnik Jan 20 '14 at 20:19
  • @Taylor I can't find any evidence of what you are suggesting. Also `commons-httpclient-3.1` is outdated and `CommonsClientHttpRequestFactory` has been deprecated in favor of `HttpComponentsClientHttpRequestFactory`. That might be an appropriate alternative but I'm pretty sure it does similar things in the background, except it might be more configurable. Both (all 3) should be thread safe. – Sotirios Delimanolis Jan 20 '14 at 20:21
  • @MarkoTopolnik Don't all HTTP clients use `URLConnection` in their underlying implementation? Do some use sockets directly? I would think that worse. – Sotirios Delimanolis Jan 20 '14 at 20:22
  • Why would it be worse? Of course a competent HTTP implementation should start from a raw socket---in fact, nowadays it shouldn't touch anything related to `java.io`, anyway. – Marko Topolnik Jan 20 '14 at 20:24
  • @Marko I would think it harder to work with. I'll look into some more source. Thanks. – Sotirios Delimanolis Jan 20 '14 at 20:25
  • 1
    [Here's a pointer.](http://grepcode.com/file/repo1.maven.org/maven2/org.apache.httpcomponents/httpclient/4.0.1/org/apache/http/impl/conn/DefaultClientConnectionOperator.java#90) BTW I truly advise you look into the implementation of URLConnection---I think you'll change your mind about "harder to work with" :) – Marko Topolnik Jan 20 '14 at 20:29
  • @SotiriosDelimanolis Yes, we were stuck with the outdated commons-httpclient because at the time (not sure if this is still true) it was what spring rest template supported. I notice now, in digging back through my version control that the httpclient in the version i used used `SimpleHttpConnectionManager` (different from what you described, apologies), which holds a `httpConnection` instance variable. – Taylor Jan 20 '14 at 20:29
  • @SotiriosDelimanolis I have similar question [here](https://stackoverflow.com/questions/25698072/simpleclienthttprequestfactory-vs-httpcomponentsclienthttprequestfactory-for-htt) which uses `RestTemplate`. See if you can help me out? I am stuck on this for a while. – AKIWEB Sep 06 '14 at 22:03
  • @SotiriosDelimanolis hey, I have a similar question on RestTemplate [here](http://stackoverflow.com/questions/30679918/does-resttemplate-reuse-and-pool-connections-by-default). If possible, can you help me out over there. I only see your name who answer most on RestTemplate question so thought to check with you. Any help will be greatly appreciated as I am having some basic confusion how to do it. I apologize for contacting like this. – john Jun 06 '15 at 07:00
5

A RestTemplate is thread safe once constructed, so you can construct one instance and have all your tasks share it. That will be much more efficient, because you eliminate the construction cost from each task, and reduce the load on the garbage collector.

Raedwald
  • 46,613
  • 43
  • 151
  • 237
4

I have my multi-thread-safe singleton REST template wired like this in spring:

<bean class="org.apache.commons.httpclient.params.HttpConnectionManagerParams" id="httpConnectionManagerParams">
    <property name="connectionTimeout" value="10000"/>
</bean>
<bean class="org.apache.commons.httpclient.MultiThreadedHttpConnectionManager" id="httpConnectionManager">
    <property name="params" ref="httpConnectionManagerParams"/>
</bean>
<bean class="org.apache.commons.httpclient.params.HttpClientParams" id="httpClientParams">
    <property name="authenticationPreemptive" value="true"/>
    <property name="soTimeout" value="10000"/>
</bean>
<bean class="org.apache.commons.httpclient.HttpClient" id="httpClient">
    <constructor-arg ref="httpClientParams"/>
    <constructor-arg ref="httpConnectionManager"/>
</bean>
<bean class="org.springframework.http.client.CommonsClientHttpRequestFactory" id="httpClientFactory">
    <constructor-arg ref="httpClient"/>
</bean>
<bean class="org.springframework.security.oauth.consumer.client.OAuthRestTemplate" id="restTemplate">
    <constructor-arg ref="httpClientFactory"/>
    <constructor-arg ref="myResource"/>
    <property name="messageConverters">
        <list>
            <ref bean="marshallingHttpMessageConverter"/>
        </list>
    </property>
</bean>

Please note I'm using an OAuthRestTemplate, and myResource refers to the oauth resource stuff which I've omitted as it's not relevant. Instead of an OAuthRestTemplate you could just as easily use a org.springframework.web.client.RestTemplate http://docs.spring.io/spring/docs/3.2.4.RELEASE/javadoc-api/org/springframework/web/client/RestTemplate.html

Taylor
  • 3,942
  • 2
  • 20
  • 33