1

In my program I try tro grasp how to use ExecutorService to optimize my program. For some reason, It gets stuck a little on two Urls. The http://sjsu.edu/ and https://paypal.com. When it sits on these two, it does not continue executing other URLS.

Should the other 3 threads available not continue even though the two domains aren't responsing fast enough?

How is this fixed in the best possible manner?

public class SequentialPinger {
    public static void main(String args[]) throws Exception {

        String[] hostList = {"http://crunchify.com", "http://yahoo.com",
            "http://www.ebay.com", "http://google.com",
            "http://www.example.co", "https://paypal.com",
            "http://bing.com/", "http://techcrunch.com/",
            "http://mashable.com/", "http://thenextweb.com/",
            "http://wordpress.com/", "http://cphbusiness.dk/",
            "http://example.com/", "http://sjsu.edu/",
            "http://ebay.co.uk/", "http://google.co.uk/",
            "http://www.wikipedia.org/",
            "http://dr.dk", "http://pol.dk", "https://www.google.dk",
            "http://phoronix.com", "http://www.webupd8.org/",
            "https://studypoint-plaul.rhcloud.com/", "http://stackoverflow.com",
            "http://docs.oracle.com", "https://fronter.com",
            "http://imgur.com/", "http://www.imagemagick.org"
        };

        List<CallableImpl> callList = new ArrayList();
        ExecutorService es = Executors.newFixedThreadPool(4);

        for (String url : hostList) {
            CallableImpl callable = new CallableImpl(url);
            callList.add(callable);
        }
        for (CallableImpl callableImpl : callList) {
            System.out.println("Trying to connect to: " + callableImpl.getUrl());
            Future<String> lol = es.submit(callableImpl);
            System.out.println("status: " + lol.get());
        }

        es.shutdown();

    }
}

My Callable implementation

public class CallableImpl implements Callable<String> {

    private final String url;

    public CallableImpl(String url) {
        this.url = url;
    }

    public String getUrl() {
        return url;
    }

    @Override
    public String call() {
        String result = "Error";

        try {
            URL siteURL = new URL(url);
            HttpURLConnection connection = (HttpURLConnection) siteURL
                    .openConnection();
            connection.setRequestMethod("GET");

            connection.connect();

            int code = connection.getResponseCode();
            if (code == 200) {
                result = "Green";
            }

            if (code == 301) {
                result = "Redirect";
            }

        } catch (IOException e) {
            result = "->Red<-";
        }
        return result;
    }
}
2
  • 1
    calling get on the future right after you submit it turns this into a blocking call, so it stops you from submitting more tasks until the current future completes. Commented Aug 22, 2018 at 12:36
  • From the documentation: Waits if necessary for the computation to complete, and then retrieves its result - So you are basically waiting after submitting each task Commented Aug 22, 2018 at 12:40

4 Answers 4

5

In your code you submit Callable to ExecutorService one by one and immediately call Future.get() which will block until result is ready (or exception is thrown at runtime).

You'd better wrap ExecutorService with CompletionSerivce which provides results as soon as they are ready. And split for-loop into two loops: one to submit all Callables and second to check results.

ExecutorService es = Executors.newFixedThreadPool(4);
ExecutorCompletionService<String> completionService = new ExecutorCompletionService<>(es);

for (CallableImpl callableImpl : callList) {
        System.out.println("Trying to connect to: " + callableImpl.getUrl());
        completionService.submit(callableImpl);
}
for (int i = 0; i < callList.size(); ++i) {
    completionService.take().get();   //fetch next finished Future and check its result
}
Sign up to request clarification or add additional context in comments.

5 Comments

I have looked the ExecutorCompletionService class up, and it seems to be too difficult for me to grasp with my current knowledge of multithreading. Do you mind to specify a bit further?
@JonasGrønbek each call to ExecutorCompletionService.take() either returns Future that already finished (either normally or with exception) or blocks until such Future is available. So if you submitted 10 tasks to CompletionService you need to call take() 10 times to get all results. Results are returned from take() as soon as they are available and this might not be the order in which they were submitted
So it is correct to say that when submitting with a ExecutorCompletionService, it stores the Future and the threads looks for the next task. When calling take(). You ask if it has any futures stored, and waits until a thread completes something if it is currently empty? @Ivan
It starts executing task as usual ExecutoService but also stores finished futures internally and when take() is called it checks if there are finished futures
Thanks a ton! @Ivan
2

Problem

You call get() on the Future directly after creating it, blocking the main thread. Thus you don't have any parallel calls at all, and making the ExecutorService essentially useless. Your code is equivalent to simply calling callableImpl.call() yourself.

Solution

Don't call get() if you want to continue execution and have each CallableImpl run in parallel. Instead you can call es.awaitTermination() after es.shutdown().

Comments

1

I suggest using a CompletableFuture added in Java 8 and add a callback to it.

CompletableFuture.supplyAsync(myCallable::call, es)
        .thenAccept(result -> {
            something(result);
        });

I would suggest making your Callable be a Supplier to make this simpler.

Comments

0

You wrote: "it does not continue executing other URLS" - I believe it does, but your log messages are misleading because are not tightly connected to the actual execution. To fix this, do the following:

  1. Move System.out.println("Trying to connect to: ") and System.out.println("status: ") into the CallableImpl.call() method.
  2. Do not call to lol.get() at all.

This way you will see actual sequence of the start and the end of handling each URL.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.