4
\$\begingroup\$

I am new to Multi-threading. I have had a go at writing some code that loops through an ArrayList, gets a hostname and then pings the hostname. If the hostname is not pingable it adds the hostname to a list returned by the method.

I am very new to multi-threading so I was just wondering if someone could give me a hand with improving this. For example I've heard that you can pass an ArrayList to Runnable but I can't find any good examples on how to do this yet.

Please note that I am using Java 5.

private ArrayList<String> pingNonReplicatingRegisters (
    Collection<RegisterReplicationSynchTime> nonReplicatingRegisters) throws IOException {

    final ArrayList<String> nonPingableRegisters = new ArrayList<String>();    

    int nThreads = 15;
    final ExecutorService es = Executors.newFixedThreadPool(nThreads);

    for(RegisterReplicationSynchTime nonReplicatingRegister : nonReplicatingRegisters){

        final String registerHostName = nonReplicatingRegister.getRegisterName();

        es.execute(new Runnable(){
            public void run(){
                boolean status = false;
                Socket socket = null;
                PingBean pngBean = new PingBean();

                try {   
                    SocketAddress socketAddress = new InetSocketAddress(registerHostName, 139);
                    socket = new Socket();
                    socket.connect(socketAddress, 1000); //1secs
                    status = true;
                    socket.close();
                } catch (IOException ex) {

                }
                finally {
                    if (socket != null) {
                        try {
                            socket.close();
                        } catch (PortUnreachableException e) {
                            socket = null;
                            System.err.println("Got an " + e);
                        } catch (UnknownHostException e) {
                            // TODO Auto-generated catch block
                            socket = null;
                            System.err.println("Got an " + e);
                        } catch (IOException e) {
                            socket = null;
                        }
                    }
               }// finally
               /* return status */
               if (!status) {
                   nonPingableRegisters.add(registerHostName);
               }
           } 
        });
   }// for
   try {
       es.shutdown();
       es.awaitTermination(60, TimeUnit.SECONDS);
       List<Runnable> pending = es.shutdownNow();

       if (!pending.isEmpty()) {
           System.err.println("Pending threads: " + pending);
       }

    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

    return nonPingableRegisters;
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I hope you get some great answers! \$\endgroup\$ – Phrancis Feb 4 '16 at 23:19
  • 1
    \$\begingroup\$ Any particular reason why Java 5? \$\endgroup\$ – h.j.k. Feb 5 '16 at 1:22
1
\$\begingroup\$

I think what you are looking for is the Callable<T> interface:

A task that returns a result and may throw an exception. Implementors define a single method with no arguments called call.

Instead of having to pass around a thread-safe Collection to gather your non-pingable servers, you can call ExecutorService.invokeAll(Collection) to get a List<Future<T>>, which you can then retrieve the results.

First, I'll suggest having an implementation of Callable<T>:

public class Ping implements Callable<String> {

    private final String hostname;

    public Ping(String hostname) {
        this.hostname = hostname;
    }

    public String call() {
        // ... try to ping server
        // assume the results are stored in a boolean
        return isPingable ? "" : hostname;
    }

}

And then you can construct your Collection<Ping> to submit to the ExecutionService:

List<Ping> pingTasks = new ArrayList<Ping>(nonReplicatingRegisters.size());
for (RegisterReplicationSynchTime nonReplicatingRegister : nonReplicatingRegisters) {
    pingTasks.add(new Ping(nonReplicatingRegister.getRegisterName()));
}
// or use invokeAll(Collection, long, TimeUnit) for an explicit timeout
List<Future<String>> taskResults = es.invokeAll(pingTasks);
List<String> results = new ArrayList<String>();
for (Future<String> taskResult : taskResults) {
    try {
        String output = taskResult.get();
        if (!output.isEmpty()) {
            results.add(output);
        }
    } catch (InterruptedException e) {
        // handle accordingly
    } catch (ExecutionException e) {
        // handle accordingly
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.