forked from lsmyrnaios/UrlsController
- Add handling for additional/specific exceptions, when checking the "futures".
- Move common "ExecutionException" handling-code into its own method: "GenericUtils.getSelectedStackTraceForCausedException()". - Avoid a double log. - Code polishing.
This commit is contained in:
parent
b34417dc45
commit
9b0818b535
|
@ -112,22 +112,16 @@ public class ScheduledTasks {
|
|||
if ( ! future.get() ) // Get and see if an exception is thrown. This blocks the current thread, until the task of the future has finished.
|
||||
numFailedTasks ++;
|
||||
} catch (ExecutionException ee) { // These can be serious errors like an "out of memory exception" (Java HEAP).
|
||||
// The stacktrace of the "ExecutionException" is the one of the current code and not the code which ran inside the background-task. Try to get the cause.
|
||||
Throwable throwable = ee.getCause();
|
||||
if ( throwable == null ) {
|
||||
logger.warn("No cause was retrieved for the \"ExecutionException\"!");
|
||||
throwable = ee;
|
||||
}
|
||||
logger.error(GenericUtils.getSelectiveStackTrace(throwable, "Background task_" + i + " failed with: " + throwable.getMessage(), 30));
|
||||
logger.error(GenericUtils.getSelectedStackTraceForCausedException(ee, "Background_task_" + i + " failed with: ", null, 30));
|
||||
numFailedTasks ++;
|
||||
} catch (CancellationException ce) {
|
||||
logger.error("Background task_" + i + " was cancelled: " + ce.getMessage());
|
||||
logger.error("Background_task_" + i + " was cancelled: " + ce.getMessage());
|
||||
numFailedTasks ++;
|
||||
} catch (InterruptedException ie) {
|
||||
logger.error("Background task_" + i + " was interrupted: " + ie.getMessage());
|
||||
logger.error("Background_task_" + i + " was interrupted: " + ie.getMessage());
|
||||
numFailedTasks ++;
|
||||
} catch (IndexOutOfBoundsException ioobe) {
|
||||
logger.error("IOOBE for background task_" + i + " in the futures-list! " + ioobe.getMessage());
|
||||
logger.error("IOOBE for background_task_" + i + " in the futures-list! " + ioobe.getMessage());
|
||||
// Only here, the "future" will be null.
|
||||
} finally {
|
||||
if ( future != null ) // It may be null in case we have a IOBE.
|
||||
|
|
|
@ -164,18 +164,14 @@ public class BulkImportServiceImpl implements BulkImportService {
|
|||
// In case all the files failed to be bulk-imported, then we will detect it in the "numSuccessfulSegments"-check later.
|
||||
// The failed-to-be-imported files, will not be deleted, even if the user specifies that he wants to delete the directory.
|
||||
} catch (ExecutionException ee) { // These can be serious errors like an "out of memory exception" (Java HEAP).
|
||||
// The stacktrace of the "ExecutionException" is the one of the current code and not the code which ran inside the background-task. Try to get the cause.
|
||||
Throwable throwable = ee.getCause();
|
||||
if ( throwable == null ) {
|
||||
logger.warn("No cause was retrieved for the \"ExecutionException\"!");
|
||||
throwable = ee;
|
||||
}
|
||||
String stackTraceMessage = GenericUtils.getSelectiveStackTrace(throwable, "Task_" + i + " failed with: " + throwable.getMessage() + additionalLoggingMsg, 15);
|
||||
numFailedSegments ++;
|
||||
logger.error("Task_" + i + " failed with: " + stackTraceMessage);
|
||||
logger.error(GenericUtils.getSelectedStackTraceForCausedException(ee, "Task_" + i + " failed with: ", additionalLoggingMsg, 15));
|
||||
} catch (CancellationException ce) {
|
||||
numFailedSegments ++;
|
||||
logger.error("Task_" + i + " was cancelled: " + ce.getMessage() + additionalLoggingMsg);
|
||||
} catch (InterruptedException ie) {
|
||||
numFailedSegments ++;
|
||||
logger.error("Task_" + i + " was interrupted: " + ie.getMessage());
|
||||
} catch (IndexOutOfBoundsException ioobe) {
|
||||
logger.error("IOOBE for task_" + i + " in the futures-list! " + ioobe.getMessage() + additionalLoggingMsg);
|
||||
}
|
||||
|
|
|
@ -1,11 +1,16 @@
|
|||
package eu.openaire.urls_controller.util;
|
||||
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import java.text.SimpleDateFormat;
|
||||
import java.util.Date;
|
||||
|
||||
public class GenericUtils {
|
||||
|
||||
private static final Logger logger = LoggerFactory.getLogger(GenericUtils.class);
|
||||
|
||||
|
||||
public static final String endOfLine = "\n";
|
||||
|
||||
|
@ -22,6 +27,20 @@ public class GenericUtils {
|
|||
}
|
||||
|
||||
|
||||
|
||||
public static String getSelectedStackTraceForCausedException(Throwable thr, String firstMessage, String additionalMessage, int numOfLines)
|
||||
{
|
||||
// The stacktrace of the "ExecutionException" is the one of the current code and not the code which ran inside the background-task. Try to get the cause.
|
||||
Throwable causedThrowable = thr.getCause();
|
||||
if ( causedThrowable == null ) {
|
||||
logger.warn("No cause was retrieved for the \"ExecutionException\"!");
|
||||
causedThrowable = thr;
|
||||
}
|
||||
String initialMessage = firstMessage + causedThrowable.getMessage() + ((additionalMessage != null) ? additionalMessage : "");
|
||||
return getSelectiveStackTrace(causedThrowable, initialMessage, numOfLines);
|
||||
}
|
||||
|
||||
|
||||
public static String getSelectiveStackTrace(Throwable thr, String initialMessage, int numOfLines)
|
||||
{
|
||||
StackTraceElement[] stels = thr.getStackTrace();
|
||||
|
|
|
@ -43,6 +43,7 @@ import java.util.ArrayList;
|
|||
import java.util.Base64;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.concurrent.CancellationException;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.stream.Collectors;
|
||||
|
@ -687,11 +688,11 @@ public class ParquetFileUtils {
|
|||
int numOfAllPayloadParquetFileCreations = 0;
|
||||
int numOfFailedPayloadParquetFileCreations = 0;
|
||||
|
||||
for ( Future<ParquetReport> future : futures )
|
||||
int sizeOfFutures = futures.size();
|
||||
for ( int i = 0; i < sizeOfFutures; ++i )
|
||||
{
|
||||
ParquetReport parquetReport = null;
|
||||
try {
|
||||
parquetReport = future.get();
|
||||
ParquetReport parquetReport = futures.get(i).get();
|
||||
boolean hasProblems = (! parquetReport.isSuccessful());
|
||||
ParquetReport.ParquetType parquetType = parquetReport.getParquetType();
|
||||
if ( parquetType.equals(ParquetReport.ParquetType.attempt) ) {
|
||||
|
@ -707,20 +708,17 @@ public class ParquetFileUtils {
|
|||
logger.error(errMsg);
|
||||
return new SumParquetSuccess(false, false, ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(errMsg));
|
||||
}
|
||||
} catch (Exception e) {
|
||||
Throwable throwable = e;
|
||||
if ( e instanceof ExecutionException ) {
|
||||
// The stacktrace of the "ExecutionException" is the one of the current code and not the code which ran inside the background-task. Try to get the cause.
|
||||
throwable = e.getCause();
|
||||
if ( throwable == null ) {
|
||||
logger.warn("No cause was retrieved for the \"ExecutionException\"!");
|
||||
throwable = e;
|
||||
}
|
||||
}
|
||||
logger.error("", throwable);
|
||||
// We do not know if the failed "future" refers to a "payload" or to a "attempt".
|
||||
// So we cannot increase a specific counter. That's ok, the only drawback if that we may try to "load" the non-existent data and get an exception.
|
||||
} catch (ExecutionException ee) { // These can be serious errors like an "out of memory exception" (Java HEAP).
|
||||
logger.error(GenericUtils.getSelectedStackTraceForCausedException(ee, "Parquet_task_" + i + " failed with: ", null, 15));
|
||||
} catch (CancellationException ce) {
|
||||
logger.error("Parquet_task_" + i + " was cancelled: " + ce.getMessage());
|
||||
} catch (InterruptedException ie) {
|
||||
logger.error("Parquet_task_" + i + " was interrupted: " + ie.getMessage());
|
||||
} catch (IndexOutOfBoundsException ioobe) {
|
||||
logger.error("IOOBE for parquet_task_" + i + " in the futures-list! " + ioobe.getMessage());
|
||||
}
|
||||
// We do not know if the failed "future" refers to a "payload" or to a "attempt".
|
||||
// So we cannot increase a specific counter. That's ok, the only drawback if that we may try to "load" the non-existent data and get an exception.
|
||||
} // End-for
|
||||
|
||||
boolean hasAttemptParquetFileProblem = (numOfFailedAttemptParquetFileCreations == numOfAllAttemptParquetFileCreations);
|
||||
|
|
Loading…
Reference in New Issue