What is the right way to implement sync and async methods in a library?

Question

I need to make a library in which I will have synchronous and asynchronous feature.

executeSynchronous() - waits until I have a result, returns the result.
executeAsynchronous() - returns a Future immediately which can be processed after other things are done, if needed.

Core Logic of my Library

The customer will use our library and they will call it by passing DataKey builder object. We will then construct a URL by using that DataKey object and make a HTTP client call to that URL by executing it and after we get the response back as a JSON String, we will send that JSON String back to our customer as it is by creating DataResponse object. Some customer will call executeSynchronous() and some might call executeAsynchronous() method so that's why I need to provide two method separately in my library.

Interface:

public interface Client {

    // for synchronous
    public DataResponse executeSynchronous(DataKey key);

    // for asynchronous
    public Future<DataResponse> executeAsynchronous(DataKey key);
}

And then I have my DataClient which implements the above Client interface:

public class DataClient implements Client {

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

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

        try {
            future = executeAsynchronous(key);
            dataResponse = future.get(key.getTimeout(), TimeUnit.MILLISECONDS);
        } catch (TimeoutException ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.TIMEOUT_ON_CLIENT, key);
            dataResponse = new DataResponse(null, DataErrorEnum.TIMEOUT_ON_CLIENT, DataStatusEnum.ERROR);
        } catch (Exception ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }

    //for asynchronous call
    @Override
    public Future<DataResponse> executeAsynchronous(DataKey key) {
        Future<DataResponse> future = null;

        try {
            Task task = new Task(key, restTemplate);
            future = executor.submit(task); 
        } catch (Exception ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
        }

        return future;
    }
}

Simple class which will perform the actual task:

public class Task implements Callable<DataResponse> {

    private DataKey key;
    private 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);

            // it is a successful response
            dataResponse = new DataResponse(response, DataErrorEnum.NONE, DataStatusEnum.SUCCESS);
        } catch (RestClientException ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.SERVER_DOWN, key);
            dataResponse = new DataResponse(null, DataErrorEnum.SERVER_DOWN, DataStatusEnum.ERROR);
        } catch (Exception ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }

    // create a URL by using key object
    private String createURL() {
        String url = somecode;
        return url;
    }
}

Customer within our company will use my library like this as shown below by using my factory in their code base -

// if they are calling `executeSynchronous()` method
DataResponse response = DataClientFactory.getInstance().executeSynchronous(dataKey);

// and if they want to call `executeAsynchronous()` method
Future<DataResponse> response = DataClientFactory.getInstance().executeAsynchronous(dataKey);

What is the best way to implement sync and async method for my library? Does implementing sync call as async + waiting is a bad idea? Because it will consume one thread from the thread pool per a call with my currrent setup? If yes, then can anyone explain why it's a bad idea and will it have any performance issue?

How will you implement sync and async method given the above criteria? What is the best way to do this? This library will be used under very heavy load and it has to be fast, meaning it should take time whatever my server is taking to respond.

Should I use AsyncRestTemplate in my code base which will be async non-blocking architecture?

score 4 · Answer 1 · answered Mar 27 '15 at 07:03

For synchronous call, executing in a separate thread is definitely not a good idea. You are incurring extra costs and resources for a Thread along with the cost of context switch of threads in this case.

If there are lots of synchronous calls then you will be unnecessarily blocking the threads for asynchronous calls as your executor is of fixed size threads. The total throughput of the system will be less in that case.

For example: If there are 10 clients calling each of the synchronous and asynchronous calls, in your implementation only threads will be actually working. However, if you were to utilize the client threads also and not make synchronous call as asynchronous and wait then all the 20 calls will be processed at the same time.

Nayanjyoti Deka · Accepted Answer · 2015-04-04T18:17:43.617

If you are creating a new thread even in case of synchronous operation(when it is actually not required), it will lead to performance hit. You are basically creating new Thread(read as wasting resources) without even getting any benefits of it. That being said I think better way would be to wrap the HTTP part in a different class. That way you would be re-using the code for HTTP access in both synchronous and asynchronous case.

class HTTPAccess{
    private RestTemplate restTemplate;
    private DataKey key;

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

    }


    public DataResponse performRequest() {
        DataResponse dataResponse = null;        
        try {
            String url = createURL();
            response = restTemplate.getForObject(url, String.class);

            // it is a successful response
            dataResponse = new DataResponse(response, DataErrorEnum.NONE, DataStatusEnum.SUCCESS);
        } catch (RestClientException ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.SERVER_DOWN, key);
            dataResponse = new DataResponse(null, DataErrorEnum.SERVER_DOWN, DataStatusEnum.ERROR);
        } catch (Exception ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }

    // create a URL by using key object
    private String createURL() {
        String url = somecode;
        return url;
    }

}

Now for the Client implementation, simply use this class.

public class DataClient implements Client {

    private ExecutorService executor = Executors.newFixedThreadPool(10);
    private RestTemplate restTemplate;
    private void initRestClient(DataKey key){
    if(restTemplate == null)
        restTemplate = new RestTemplate(clientHttpRequestFactory(key));
    }

    private ClientHttpRequestFactory clientHttpRequestFactory(DataKey key) {
        HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory();
        factory.setReadTimeout(key.getTimeout());
        factory.setConnectTimeout(key.getTimeout());
        //if you need to set otherparams this is the place we can do it extracting from DataKey obj
        return factory;
    }

    // for synchronous call
    @Override
    public DataResponse executeSynchronous(DataKey key) {
        initRestClient(key);
        DataResponse dataResponse = new HTTPAccess(key).performRequest();
        return dataResponse;
    }

    //for asynchronous call
    @Override
    public Future<DataResponse> executeAsynchronous(final DataKey key) {
        return executor.submit(new Callable<DataResponse>() {
            @Override
            public DataResponse call() throws Exception {
                return executeSynchronous(key);
            }
        });
    }
}

This way your HTTP implementation is completely separate and in future if you need to change the way to receive DataResponse (maybe from DB call ), then you have to change the HTTPAccess class only and other part will not be affected.

Please note that timeout will be caught as RestClientException in HTTPAccess class. So maybe you would want to change the code where we caught RestClientException with something more specific to your need. — Nayanjyoti Deka, Apr 04 '15 at 15:11
Thanks for suggestion. Appreciated your help. I have few doubts. With this approach, are we not creating `RestTemplate` everytime for each request here? And also I guess, we are creating factory for every request and each factory has connection and thread pool and pretty heavy object I guess. Is there any way to reuse them? I guess bcoz of that reason I was passing RestTemplate from my DataClien class using DI. — john, Apr 04 '15 at 16:11
yes your suggestion is great. we can reuse the `RestTemplate` object. Have edited the code to reflect the changes. — Nayanjyoti Deka, Apr 04 '15 at 18:03

score 1 · Answer 3 · edited May 23 '17 at 11:59

1

I think this is better:

@Override
public DataResponse executeSynchronous(DataKey key) {
    Task task = new Task(key, restTemplate);
    return task.call();
}

It performs the same job, is clear, shorter, and has no overhead.

Notice that his also cleans up the duplicate Exception handling you currently have.

If the timeout is a must, an option is to use the underlying timeouts for the RestTemplate class, as explained in Spring RestTemplate timeout

Then the timeout will cause a RestClientException that you or the library client can handle.

edited May 23 '17 at 11:59

Community

1
1

answered Mar 30 '15 at 23:02

wzurita

56
4

I did thought about that. Then how will I implement the timeout stuff? I still need to do that right so that I can timeout if server is taking too much time to respond. – john Mar 30 '15 at 23:11
I noticed the timeout after I gave my answer. I also made an edition to the answer without reading your comment. I don't think (based on the code you show) the timeout should be on the *DataKey*, and, if the timeout is a *must* then this is actually a good implementation. To implement a timeout you will need a second thread that counts the time and kills the original, slow thread. – wzurita Mar 30 '15 at 23:24
BUT take a look at: http://stackoverflow.com/questions/13837012/spring-resttemplate-timeout. It is better to properly use the settings of the underlying library and write your code as simple as possible, as in my original answer (I will edit it to include this bit) – wzurita Mar 30 '15 at 23:37

score 1 · Answer 4 · answered Apr 02 '15 at 15:02

I wouldn't bother with that Task class. Just make your synchronous method do all the work and call it asynchronously from the asynchronous method.

public class DataClient implements Client {

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

    // for synchronous call
    @Override
    public DataResponse executeSynchronous(DataKey key) {
        DataResponse dataResponse = null;
        String response = null;

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

            // it is a successful response
            dataResponse = new DataResponse(response, DataErrorEnum.NONE, DataStatusEnum.SUCCESS);
        } catch (RestClientException ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.SERVER_DOWN, key);
            dataResponse = new DataResponse(null, DataErrorEnum.SERVER_DOWN, DataStatusEnum.ERROR);
        } catch (Exception ex) {
            PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
            dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
        }

        return dataResponse;
    }

    //for asynchronous call
    @Override
    public Future<DataResponse> executeAsynchronous(final DataKey key) {
        return executor.submit(new Callable<DataResponse>() {
            @Override
            public DataResponse call() throws Exception {
                return executeSynchronous(key);
            }
        });
    }
}

I only wrote the above code that way because I thought it looked prettier and simpler. But your version is absolutely fine if what you want to do is to throttle the number of simultaneous request to the rest service. I can't tell from the information you have given which would perform better. It depends on how the REST service works and on what the clients of your library do. The best thing would be to try one then the other and measure which performs better. I don't know anything about AsyncRestTemplate, sorry. — mikeyreilly, Apr 02 '15 at 15:36

score 1 · Answer 5 · answered Apr 03 '15 at 15:52

The above code of executing synchronous task via async is same as having everything as async. If this is the requirement then I would suggest you to use google guava's ListenableFuture. I am not an advocate but it has methods to manage task timeout, well written callbacks to handle onSuccess, onFailure scenarios. https://code.google.com/p/guava-libraries/wiki/ListenableFutureExplained

What is the right way to implement sync and async methods in a library?

5 Answers5

Linked