From 9ea2ed371a345f53657a194469127451ce4d48e7 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Wed, 8 May 2019 15:32:20 +0200 Subject: [PATCH] added exception logging for all ExecutorService.invokeAll calls Change-Id: I86e24978feb6d47bd9d4856d7b5b2e76a6bf7792 --- .../MarkPassingCalculator.java | 17 +++----- .../impl/CandidateFinderImpl.java | 7 ++-- .../impl/TrackFileExporterImpl.java | 33 +--------------- .../src/com/sap/sse/util/ThreadPoolUtil.java | 25 ++++++++++++ .../sap/sse/util/impl/ThreadPoolUtilImpl.java | 39 +++++++++++++++++++ 5 files changed, 74 insertions(+), 47 deletions(-) diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/MarkPassingCalculator.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/MarkPassingCalculator.java index 86cfaa61e78..9207ebfdd46 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/MarkPassingCalculator.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/MarkPassingCalculator.java @@ -120,11 +120,8 @@ public class MarkPassingCalculator { return null; }); } - try { - executor.invokeAll(tasks); - } catch (Exception e) { - logger.log(Level.SEVERE, "Error trying to compute initial set of mark passings for race "+race.getRace().getName(), e); - } + ThreadPoolUtil.INSTANCE.invokeAllAndLogExceptions(executor, Level.SEVERE, + "Error trying to compute initial set of mark passings for race "+race.getRace().getName()+": %s", tasks); if (listener != null) { synchronized (MarkPassingCalculator.this) { if (listenerThread == null) { @@ -326,7 +323,8 @@ public class MarkPassingCalculator { }); } logger.finer(()->"Calculating mark passing deltas after course change in executor"); - executor.invokeAll(tasks); + ThreadPoolUtil.INSTANCE.invokeAllAndLogExceptions(executor, Level.SEVERE, + "Error calculating mark passing deltas after course change in executor: %s", tasks); logger.finer(()->"Done calculating mark passing deltas after course change"); } updateManuallySetMarkPassings(fixedMarkPassings, removedFixedMarkPassings, suppressedMarkPassings, unsuppressedMarkPassings); @@ -424,11 +422,8 @@ public class MarkPassingCalculator { } }); } - try { - executor.invokeAll(tasks); - } catch (InterruptedException e) { - logger.log(Level.INFO, "mark passing calculation interrupted", e); - } + ThreadPoolUtil.INSTANCE.invokeAllAndLogExceptions(executor, Level.INFO, + "Error during mark passing calculation: %s", tasks); } private class ComputeMarkPassings implements Runnable { diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateFinderImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateFinderImpl.java index 57d4b71d3fa..c638283e4e7 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateFinderImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateFinderImpl.java @@ -422,10 +422,9 @@ public class CandidateFinderImpl implements CandidateFinder { } }); } - executor.invokeAll(tasks); - } catch (InterruptedException e) { - logger.log(Level.SEVERE, "Problem trying to update competitor candidate sets after waypoints starting at zero-based index "+ - zeroBasedIndexOfWaypointChanged+" have changed", e); + ThreadPoolUtil.INSTANCE.invokeAllAndLogExceptions(executor, Level.SEVERE, + "Problem trying to update competitor candidate sets after waypoints starting at zero-based index "+ + zeroBasedIndexOfWaypointChanged+" have changed: %s", tasks); } finally { course.unlockAfterRead(); } diff --git a/java/com.sap.sailing.server.trackfiles/src/com/sap/sailing/server/trackfiles/impl/TrackFileExporterImpl.java b/java/com.sap.sailing.server.trackfiles/src/com/sap/sailing/server/trackfiles/impl/TrackFileExporterImpl.java index 0c11fe98d55..046c09b75b8 100644 --- a/java/com.sap.sailing.server.trackfiles/src/com/sap/sailing/server/trackfiles/impl/TrackFileExporterImpl.java +++ b/java/com.sap.sailing.server.trackfiles/src/com/sap/sailing/server/trackfiles/impl/TrackFileExporterImpl.java @@ -2,11 +2,8 @@ package com.sap.sailing.server.trackfiles.impl; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; import java.util.logging.Level; import java.util.logging.Logger; import java.util.zip.ZipEntry; @@ -44,7 +41,6 @@ public class TrackFileExporterImpl implements TrackFileExporter { @Override public void writeAllData(List data, TrackFilesFormat format, List races, boolean dataBeforeAfter, boolean rawFixes, final ZipOutputStream out) throws IOException { - WriteZipCallback callback = new WriteZipCallback() { @Override public synchronized void write(ZipEntry entry, byte[] data) throws IOException { @@ -53,7 +49,6 @@ public class TrackFileExporterImpl implements TrackFileExporter { out.closeEntry(); } }; - List callables = new ArrayList<>(); ExecutorService executor = ThreadPoolUtil.INSTANCE.getDefaultForegroundTaskThreadPoolExecutor(); List errors = new ArrayList<>(); @@ -64,32 +59,7 @@ public class TrackFileExporterImpl implements TrackFileExporter { callables.add(callable); } } - - List> results = Collections.> emptyList(); - - try { - results = executor.invokeAll(callables); - } catch (InterruptedException e) { - errors.add(e.getMessage()); - log.log(Level.WARNING, "Error exporting race: " + e.getMessage()); - e.printStackTrace(); - } - - for (Future result : results) { - try { - result.get(); - } catch (ExecutionException e) { - Throwable t = e.getCause(); - errors.add(t.getMessage()); - log.log(Level.WARNING, "Error exporting race: " + t.getMessage()); - t.printStackTrace(); - } catch (InterruptedException e) { - errors.add(e.getMessage()); - log.log(Level.WARNING, "Error exporting race: " + e.getMessage()); - e.printStackTrace(); - } - } - + ThreadPoolUtil.INSTANCE.invokeAllAndLogExceptions(executor, Level.WARNING, "Error exporting race: %s", callables); if (errors.size() > 0) { StringBuilder sb = new StringBuilder(); for (String error : errors) { @@ -100,7 +70,6 @@ public class TrackFileExporterImpl implements TrackFileExporter { callback.write(new ZipEntry("ERRORS"), sb.toString().getBytes()); } catch (Exception e) { log.log(Level.WARNING, "Error exporting race: " + e.getMessage()); - e.printStackTrace(); } } diff --git a/java/com.sap.sse/src/com/sap/sse/util/ThreadPoolUtil.java b/java/com.sap.sse/src/com/sap/sse/util/ThreadPoolUtil.java index d257f585c2c..0427222e25d 100644 --- a/java/com.sap.sse/src/com/sap/sse/util/ThreadPoolUtil.java +++ b/java/com.sap.sse/src/com/sap/sse/util/ThreadPoolUtil.java @@ -1,6 +1,11 @@ package com.sap.sse.util; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.logging.Level; import com.sap.sse.util.impl.ThreadPoolUtilImpl; @@ -79,4 +84,24 @@ public interface ThreadPoolUtil { ScheduledExecutorService createForegroundTaskThreadPoolExecutor(String name); ScheduledExecutorService createForegroundTaskThreadPoolExecutor(int size, String name); + + /** + * Logs all exceptions that occurred during the execution of the {@code futures} provided. + * + * @param messageTemplate + * a message string that must contain a single {@code %s} parameter placeholder that will be substituted + * by the exception's message. + */ + void logExceptionsFromFutures(Level logLevel, String messageTemplate, Iterable> futures); + + /** + * Uses the {@code executor}'s {@link ExecutorService#invokeAll(java.util.Collection) invokeAll} method to execute + * all {@code tasks}. All exceptions that occur are logged, using the {@code logLevel} provided, using as the log + * message the message template, parameterized with the exception message. + * + * @param messageTemplate + * a message string that must contain a single {@code %s} parameter placeholder that will be substituted + * by the exception's message. + */ + List> invokeAllAndLogExceptions(ExecutorService executor, Level logLevel, String messageTemplate, Iterable> tasks); } diff --git a/java/com.sap.sse/src/com/sap/sse/util/impl/ThreadPoolUtilImpl.java b/java/com.sap.sse/src/com/sap/sse/util/impl/ThreadPoolUtilImpl.java index 4c6b046a7a3..0fb175c8495 100644 --- a/java/com.sap.sse/src/com/sap/sse/util/impl/ThreadPoolUtilImpl.java +++ b/java/com.sap.sse/src/com/sap/sse/util/impl/ThreadPoolUtilImpl.java @@ -1,10 +1,20 @@ package com.sap.sse.util.impl; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.logging.Level; +import java.util.logging.Logger; +import com.sap.sse.common.Util; import com.sap.sse.util.ThreadPoolUtil; public class ThreadPoolUtilImpl implements ThreadPoolUtil { + private static final Logger logger = Logger.getLogger(ThreadPoolUtilImpl.class.getName()); private static final int REASONABLE_THREAD_POOL_SIZE = Math.max(Runtime.getRuntime().availableProcessors()-1, 3); private final ScheduledExecutorService defaultBackgroundTaskThreadPoolExecutor; @@ -58,4 +68,33 @@ public class ThreadPoolUtilImpl implements ThreadPoolUtil { public int getReasonableThreadPoolSize() { return REASONABLE_THREAD_POOL_SIZE; } + + @Override + public void logExceptionsFromFutures(Level logLevel, String messageTemplate, Iterable> futures) { + for (final Future result : futures) { + try { + result.get(); + } catch (ExecutionException e) { + Throwable t = e.getCause(); + logger.log(logLevel, String.format(messageTemplate, t.getMessage()), t); + } catch (InterruptedException e) { + logger.log(logLevel, String.format(messageTemplate, e.getMessage()), e); + } + } + } + + @Override + public List> invokeAllAndLogExceptions(ExecutorService executor, Level logLevel, String messageTemplate, + Iterable> tasks) { + final List> tasksAsList = new ArrayList<>(); + Util.addAll(tasks, tasksAsList); + List> result = null; + try { + result = executor.invokeAll(tasksAsList); + logExceptionsFromFutures(logLevel, messageTemplate, result); + } catch (InterruptedException e) { + logger.log(logLevel, String.format(messageTemplate, e.getMessage()), e); + } + return result; + } }