From 9af3b6e6d2c2d290045edd9bf470f018901211f6 Mon Sep 17 00:00:00 2001 From: Andrew Zimmer Date: Fri, 24 May 2024 14:57:03 -0400 Subject: [PATCH] Cleaning up logging during shutdown and during liquibase error handling. (#2873) --- .../ddp/appengine/spark/SparkBootUtil.java | 13 ++++++++++- .../ddp/util/LiquibaseUtil.java | 2 +- .../org/broadinstitute/ddp/Housekeeping.java | 23 +++++++++++++++---- .../pubsub/PubSubPublisherInitializer.java | 7 +++++- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/appengine/spark/SparkBootUtil.java b/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/appengine/spark/SparkBootUtil.java index 2f745001ba..0843a590c9 100644 --- a/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/appengine/spark/SparkBootUtil.java +++ b/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/appengine/spark/SparkBootUtil.java @@ -26,6 +26,9 @@ public class SparkBootUtil { private static int numShutdownAttempts = 0; + // set when the shutdown process begins + private static boolean isShuttingDown; + /** * Reads configuration and environment settings common to * DSM, DSS, and Housekeeping and starts a spark server on the appropriate port. @@ -69,6 +72,7 @@ public static void startSparkServer(AppEngineShutdown stopRouteCallback, Config }); Spark.get(RouteConstants.GAE.STOP_ENDPOINT, (request, response) -> { + isShuttingDown = true; log.info("Received GAE stop request [{}] for instance {} deployment {}", request.url(), System.getenv(LogUtil.GAE_INSTANCE), System.getenv(LogUtil.GAE_DEPLOYMENT_ID)); if (stopRouteCallback != null) { @@ -89,10 +93,17 @@ public static void startSparkServer(AppEngineShutdown stopRouteCallback, Config }); if (stopRouteCallback != null) { - Runtime.getRuntime().addShutdownHook(new Thread(() -> stopRouteCallback.onTerminate())); + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + isShuttingDown = true; + stopRouteCallback.onTerminate(); + })); } } + public static boolean isShuttingDown() { + return isShuttingDown; + } + /** * Methods called in response to different lifecycle events * related to shutting down diff --git a/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/util/LiquibaseUtil.java b/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/util/LiquibaseUtil.java index a6363eb669..99b000d60c 100644 --- a/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/util/LiquibaseUtil.java +++ b/pepper-apis/ddp-common/src/main/java/org/broadinstitute/ddp/util/LiquibaseUtil.java @@ -205,7 +205,7 @@ private void runMigrations(String changelogFile, String context) throws Liquibas liquibase.update(new Contexts(context)); } catch (LiquibaseException originalError) { log.error("LiquibaseException: {}", originalError.getMessage()); - if (tag != null) { + if (tag != null && originalError.getCause() != null) { if (originalError.getCause().getClass() == MigrationFailedException.class || originalError.getCause().getMessage().contains("Migration failed for change set " + changelogFile)) { try { diff --git a/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/Housekeeping.java b/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/Housekeeping.java index 128cb49f3a..34cf0de352 100755 --- a/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/Housekeeping.java +++ b/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/Housekeeping.java @@ -113,6 +113,7 @@ import org.broadinstitute.ddp.util.LogbackConfigurationPrinter; import org.jdbi.v3.core.Handle; import org.quartz.Scheduler; +import org.quartz.SchedulerException; import spark.Spark; @Slf4j @@ -468,7 +469,11 @@ public static void start(String[] args, SendGridSupplier sendGridSupplier) { } } catch (Exception e) { - logException(e); + if (!SparkBootUtil.isShuttingDown()) { + logException(e); + } else { + log.info("Exception during shutdown", e); + } } // send a ping to monitoring heartbeatMonitor.addPoint(1, Instant.now().toEpochMilli()); @@ -479,13 +484,17 @@ public static void start(String[] args, SendGridSupplier sendGridSupplier) { log.info("Housekeeping interrupted during sleep", e); } } - shutdownPubSub(); - shutdownQuartz(); log.info("Housekeeping is shutting down"); } private static void shutdownQuartz() { - if (scheduler != null) { + boolean isShutdown = false; + try { + isShutdown = scheduler.isShutdown(); + } catch (SchedulerException e) { + log.info("Could not determine whether schedule is shut down", e); + } + if (scheduler != null && !isShutdown) { JobScheduler.shutdownScheduler(scheduler, true); } } @@ -502,7 +511,11 @@ private static void shutdownPubSub() { try { pubSubTaskConnectionService.destroy(); } catch (PubSubTaskException e) { - log.error("Failed to shutdown PubSubTask API", e); + if (!SparkBootUtil.isShuttingDown()) { + log.error("Failed to shutdown PubSubTask API", e); + } else { + log.info("Exception during shutdown", e); + } } } } diff --git a/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/event/publish/pubsub/PubSubPublisherInitializer.java b/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/event/publish/pubsub/PubSubPublisherInitializer.java index e942821844..256fb9013c 100644 --- a/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/event/publish/pubsub/PubSubPublisherInitializer.java +++ b/pepper-apis/dss-core/src/main/java/org/broadinstitute/ddp/event/publish/pubsub/PubSubPublisherInitializer.java @@ -11,6 +11,7 @@ import com.google.pubsub.v1.ProjectTopicName; import com.typesafe.config.Config; import lombok.extern.slf4j.Slf4j; +import org.broadinstitute.ddp.appengine.spark.SparkBootUtil; import org.broadinstitute.ddp.constants.ConfigFile; import org.broadinstitute.ddp.exception.DDPException; import org.broadinstitute.ddp.housekeeping.PubSubConnectionManager; @@ -58,7 +59,11 @@ public static void shutdownPublishers() { try { publisher.getValue().shutdown(); } catch (Exception e) { - log.error("Error shutting down publisher {}", publisher.getKey(), e); + if (!SparkBootUtil.isShuttingDown()) { + log.error("Error shutting down publisher {}", publisher.getKey(), e); + } else { + log.info("Exception during shutdown", e); + } } } }