0

Below is my my code implementation using ExecutorService

    public void startDownloading(DownloadInfo downloadInfo) {
    int messageSentToNextQueue = 0;
    ExecutorService downloadExecutor = Executors.newSingleThreadExecutor();
    try {
            Callable<Integer> task = () -> {
                return fileDownloader.downloadFromSourceLink(downloadInfo);
            };
            Future<Integer> future = downloadExecutor.submit(task);
            try {
                if (downloadInfo.getExtension().equals(ScraperConstant.ZIP_EXTENSION)) {
                        
                    messageSentToNextQueue = future.get(11, TimeUnit.HOURS);

                } else if (downloadInfo.getExtension().equals(ScraperConstant.HTML_EXTENSION)) {
                        
                    messageSentToNextQueue = future.get(10, TimeUnit.MINUTES);

                } else {
                    messageSentToNextQueue = future.get(30, TimeUnit.MINUTES);
                }
            } catch (Exception e) {
                log.error(ex);
                downloadExecutor.shutdownNow();
            }
    } catch (Exception ex) {
        log.error(ex);

    } finally {
        if (!downloadExecutor.isShutdown()) {
            downloadExecutor.shutdownNow();
        }
    }
}

Now I'm creating ExecutorService with SingleThreadExecutor on each method call beacuse in the case of timeOut I can shutdown the service to free all the underlying resources and stop any further processing.

If I use ThreadPoolExecutor and intialize it at class level and reuse it, it will definetly a better approach but in that case if time out happen I have to shutdown complete ExecutorService in order to free resources and it will cause other threads while in processing under that ExecutorService to be stop as well and I also have to Initialize ExecutorService after shutdown.

If Someone suggest some better approach it would be great.

Noman Ellahi
  • 45
  • 2
  • 5
  • Note: `downloadExecutor.shutdownNow()` does not wait for the running task to complete. [All it does is](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/ThreadPoolExecutor.html#shutdownNow()) (a) remove any pending tasks (you have none), (b) interrupt any running task, and (c) set a flag to refuse any new tasks. **Q:** What will your `task` do when it is interrupted? – Solomon Slow May 30 '23 at 12:18
  • 1
    Action specific wait times are usually implemented by providing a timeout to the respective I/O operation. For instance, using `socket.setSoTimeout(timeout)` (see [related question](https://stackoverflow.com/questions/4969760/setting-a-timeout-for-socket-operations)). – meriton May 30 '23 at 12:48

1 Answers1

2

Style note:

I would not duplicate the messageSentToNextQueue = future.get(...) call in three places. Nor would I encode the different timeouts in the startDownloading function. I would put them in the downloadInfo instead.

messageSentToNextQueue = future.get(
    downloadInfo.getTimeoutValue(),
    downloadInfo.getTimeoutUnit()
);
Solomon Slow
  • 25,130
  • 5
  • 37
  • 57