From 887c8d4f4ca0cb4373a64444a85eef3444e7140b Mon Sep 17 00:00:00 2001 From: Rsl1122 Date: Thu, 5 Dec 2019 14:00:48 +0200 Subject: [PATCH] Changed Session shutdown save execution Save was preventing unnecessary transactions from being skipped, leading to hang during session shutdown save. Fixed by moving the wait to only JVM shutdown save, and trusting on database shutdown transaction execution to do the job. Affects issues: - Fixed #1241 --- .../plan/gathering/ServerShutdownSave.java | 41 +++++++++---------- .../plan/gathering/ShutdownHook.java | 16 +++++++- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java index 051ab89d9..38654278d 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java @@ -18,7 +18,6 @@ package com.djrapitops.plan.gathering; import com.djrapitops.plan.delivery.domain.keys.SessionKeys; import com.djrapitops.plan.exceptions.database.DBInitException; -import com.djrapitops.plan.exceptions.database.DBOpException; import com.djrapitops.plan.gathering.cache.SessionCache; import com.djrapitops.plan.gathering.domain.Session; import com.djrapitops.plan.settings.locale.Locale; @@ -33,7 +32,7 @@ import com.djrapitops.plugin.logging.error.ErrorHandler; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; /** * Class in charge of performing save operations when the server shuts down. @@ -48,6 +47,8 @@ public abstract class ServerShutdownSave { private final ErrorHandler errorHandler; private boolean shuttingDown = false; + private boolean startedDatabase = false; + public ServerShutdownSave( Locale locale, DBSystem dbSystem, @@ -66,14 +67,14 @@ public abstract class ServerShutdownSave { shuttingDown = true; } - public void performSave() { + public Optional> performSave() { if (!checkServerShuttingDownStatus() && !shuttingDown) { - return; + return Optional.empty(); } Map activeSessions = SessionCache.getActiveSessions(); if (activeSessions.isEmpty()) { - return; + return Optional.empty(); } // This check ensures that logging is not attempted on JVM shutdown. @@ -81,32 +82,33 @@ public abstract class ServerShutdownSave { if (!shuttingDown) { logger.info(locale.getString(PluginLang.DISABLED_UNSAVED_SESSIONS)); } - attemptSave(activeSessions); - - SessionCache.clear(); + return attemptSave(activeSessions); } - private void attemptSave(Map activeSessions) { + private Optional> attemptSave(Map activeSessions) { try { prepareSessionsForStorage(activeSessions, System.currentTimeMillis()); - saveActiveSessions(activeSessions); + return Optional.of(saveActiveSessions(activeSessions)); } catch (DBInitException e) { errorHandler.log(L.ERROR, this.getClass(), e); + return Optional.empty(); } catch (IllegalStateException ignored) { /* Database is not initialized */ + return Optional.empty(); } finally { closeDatabase(dbSystem.getDatabase()); } } - private void saveActiveSessions(Map activeSessions) { + private Future saveActiveSessions(Map activeSessions) { Database database = dbSystem.getDatabase(); if (database.getState() == Database.State.CLOSED) { // Ensure that database is not closed when performing the transaction. + startedDatabase = true; database.init(); } - saveSessions(activeSessions, database); + return saveSessions(activeSessions, database); } private void prepareSessionsForStorage(Map activeSessions, long now) { @@ -118,18 +120,13 @@ public abstract class ServerShutdownSave { } } - private void saveSessions(Map activeSessions, Database database) { - try { - database.executeTransaction(new ServerShutdownTransaction(activeSessions.values())) - .get(); // Ensure that the transaction is executed before shutdown. - } catch (ExecutionException | DBOpException e) { - errorHandler.log(L.ERROR, this.getClass(), e); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } + private Future saveSessions(Map activeSessions, Database database) { + return database.executeTransaction(new ServerShutdownTransaction(activeSessions.values())); } private void closeDatabase(Database database) { - database.close(); + if (startedDatabase) { + database.close(); + } } } \ No newline at end of file diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ShutdownHook.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ShutdownHook.java index 2a1f0ba3a..e7a53865a 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ShutdownHook.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ShutdownHook.java @@ -18,6 +18,10 @@ package com.djrapitops.plan.gathering; import javax.inject.Inject; import javax.inject.Singleton; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Thread that is run when JVM shuts down. @@ -62,6 +66,16 @@ public class ShutdownHook extends Thread { @Override public void run() { serverShutdownSave.serverIsKnownToBeShuttingDown(); - serverShutdownSave.performSave(); + serverShutdownSave.performSave().ifPresent(this::waitForSave); + } + + private void waitForSave(Future sessionsAreSavedFuture) { + try { + sessionsAreSavedFuture.get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } catch (ExecutionException e) { + Logger.getGlobal().log(Level.SEVERE, "Plan failed to save sessions on JVM shutdown.", e); + } } }