diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/MySQLDB.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/MySQLDB.java index 817b3f066e..33b38d8d9c 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/MySQLDB.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/MySQLDB.java @@ -17,7 +17,6 @@ package com.djrapitops.plan.storage.database; import com.djrapitops.plan.exceptions.database.DBInitException; -import com.djrapitops.plan.exceptions.database.DBOpException; import com.djrapitops.plan.exceptions.database.MariaDB11Exception; import com.djrapitops.plan.identification.ServerInfo; import com.djrapitops.plan.settings.config.PlanConfig; @@ -223,11 +222,7 @@ public synchronized Connection getConnection() throws SQLException { Connection connection = dataSource.getConnection(); if (!connection.isValid(5)) { connection.close(); - try { - return getConnection(); - } catch (StackOverflowError databaseHasGoneDown) { - throw new DBOpException("Valid connection could not be fetched (Is MySQL down?) - attempted until StackOverflowError occurred.", databaseHasGoneDown); - } + throw new SQLTransientConnectionException("Connection validation failed"); } if (connection.getAutoCommit()) connection.setAutoCommit(false); setTimezoneToUTC(connection); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java index 89e3990558..dccd02893f 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java @@ -51,6 +51,9 @@ import java.sql.Connection; import java.sql.SQLException; +import java.sql.SQLNonTransientConnectionException; +import java.sql.SQLRecoverableException; +import java.sql.SQLTransientConnectionException; import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; @@ -65,6 +68,8 @@ */ public abstract class SQLDB extends AbstractDatabase { + private static final long INITIAL_CONNECTION_RETRY_DELAY_MS = TimeUnit.SECONDS.toMillis(1L); + private static final long MAX_CONNECTION_RETRY_DELAY_MS = TimeUnit.SECONDS.toMillis(30L); private static final List DRIVER_REPOSITORIES = Arrays.asList( new MavenRepository("https://repo.papermc.io/repository/maven-public"), new MavenRepository("https://repo1.maven.org/maven2") @@ -324,7 +329,7 @@ public CompletableFuture executeTransaction(Transaction transaction) { if (getState() == State.CLOSED) return CompletableFuture.completedFuture(null); accessLock.performDatabaseOperation(() -> { - if (!ranIntoFatalError.get()) {transaction.executeTransaction(this);} + if (!ranIntoFatalError.get()) executeTransactionWithConnectionRetry(transaction); }, transaction); return CompletableFuture.completedFuture(null); } finally { @@ -334,6 +339,59 @@ public CompletableFuture executeTransaction(Transaction transaction) { }, getTransactionExecutor()).exceptionally(errorHandler(transaction, origin)); } + private void executeTransactionWithConnectionRetry(Transaction transaction) { + long retryDelayMs = INITIAL_CONNECTION_RETRY_DELAY_MS; + boolean connectionWasUnavailable = false; + + while (getState() != State.CLOSED && getState() != State.CLOSING) { + try { + transaction.executeTransaction(this); + if (connectionWasUnavailable) { + logger.info("Database connection restored, resuming queued transactions."); + } + return; + } catch (DBOpException failure) { + if (!isConnectionFailure(failure)) throw failure; + + if (!connectionWasUnavailable) { + logger.warn("Database connection is unavailable. Transactions will remain queued until it recovers."); + connectionWasUnavailable = true; + } + if (!waitForConnectionRetry(retryDelayMs)) return; + retryDelayMs = Math.min(retryDelayMs * 2L, MAX_CONNECTION_RETRY_DELAY_MS); + } + } + } + + private boolean waitForConnectionRetry(long retryDelayMs) { + try { + if (retryDelayMs > 0L) Thread.sleep(retryDelayMs); + return getState() != State.CLOSED && getState() != State.CLOSING; + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + return false; + } + } + + static boolean isConnectionFailure(Throwable failure) { + Set visited = Collections.newSetFromMap(new IdentityHashMap<>()); + Throwable cause = failure; + while (cause != null && visited.add(cause)) { + if (cause instanceof FatalDBException || cause instanceof SQLNonTransientConnectionException) { + return false; + } + if (cause instanceof SQLTransientConnectionException || cause instanceof SQLRecoverableException) { + return true; + } + if (cause instanceof SQLException) { + String sqlState = ((SQLException) cause).getSQLState(); + if (sqlState != null && sqlState.startsWith("08")) return true; + } + cause = cause.getCause(); + } + return false; + } + private boolean determineIfShouldDropUnimportantTransactions(int queueSize) { if (getState() == State.CLOSING) { return true; diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java index 344e948c3d..e24be38d66 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java @@ -21,6 +21,7 @@ import com.djrapitops.plan.delivery.domain.container.PlayerContainer; import com.djrapitops.plan.delivery.domain.keys.Key; import com.djrapitops.plan.delivery.domain.keys.PlayerKeys; +import com.djrapitops.plan.exceptions.database.DBOpException; import com.djrapitops.plan.gathering.domain.*; import com.djrapitops.plan.gathering.domain.event.JoinAddress; import com.djrapitops.plan.identification.Server; @@ -57,11 +58,14 @@ import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.SQLTransientConnectionException; import java.time.LocalDateTime; import java.time.ZoneOffset; import java.util.*; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import static com.djrapitops.plan.storage.database.sql.building.Sql.*; @@ -224,6 +228,48 @@ default void indexCreationWorksWithoutErrors() throws Exception { assertTrue(transaction.wasSuccessful()); } + @Test + default void transactionsStayOrderedDuringTemporaryConnectionFailure() { + List executionOrder = Collections.synchronizedList(new ArrayList<>()); + AtomicInteger retryAttempts = new AtomicInteger(); + Transaction temporarilyFailing = new Transaction() { + @Override + public void executeTransaction(SQLDB db) { + if (retryAttempts.incrementAndGet() == 1) { + throw new DBOpException( + "Connection unavailable", + new SQLTransientConnectionException("Connection unavailable", "08001") + ); + } + executionOrder.add(1); + } + + @Override + protected void performOperations() { + // executeTransaction is overridden to simulate a connection outage. + } + }; + Transaction queuedAfterFailure = new Transaction() { + @Override + public void executeTransaction(SQLDB db) { + executionOrder.add(2); + } + + @Override + protected void performOperations() { + // executeTransaction is overridden to observe queue order. + } + }; + + CompletableFuture first = db().executeTransaction(temporarilyFailing); + CompletableFuture second = db().executeTransaction(queuedAfterFailure); + CompletableFuture.allOf(first, second).join(); + + assertEquals(2, retryAttempts.get()); + assertEquals(List.of(1, 2), executionOrder); + assertEquals(0, db().getTransactionQueueSize()); + } + @Test default void playerCountForServersIsCorrect() { Map expected = Collections.singletonMap(serverUUID(), 1); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLDBTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLDBTest.java new file mode 100644 index 0000000000..8c93868282 --- /dev/null +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLDBTest.java @@ -0,0 +1,67 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan.storage.database; + +import com.djrapitops.plan.identification.ServerInfo; +import com.djrapitops.plan.settings.config.PlanConfig; +import com.djrapitops.plan.settings.locale.Locale; +import com.djrapitops.plan.storage.file.PlanFiles; +import com.djrapitops.plan.utilities.logging.ErrorLogger; +import com.zaxxer.hikari.HikariDataSource; +import dagger.Lazy; +import dev.vankka.dependencydownload.ApplicationDependencyManager; +import net.playeranalytics.plugin.scheduling.RunnableFactory; +import net.playeranalytics.plugin.server.PluginLogger; +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.SQLTransientConnectionException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class MySQLDBTest { + + @Test + @SuppressWarnings("unchecked") + void invalidConnectionIsRejectedWithoutRecursiveLookup() throws Exception { + Lazy serverInfo = mock(Lazy.class); + MySQLDB database = new MySQLDB( + mock(Locale.class), + mock(PlanConfig.class), + mock(PlanFiles.class), + serverInfo, + mock(RunnableFactory.class), + mock(PluginLogger.class), + mock(ErrorLogger.class), + mock(ApplicationDependencyManager.class) + ); + HikariDataSource dataSource = mock(HikariDataSource.class); + Connection connection = mock(Connection.class); + database.dataSource = dataSource; + when(dataSource.getConnection()).thenReturn(connection); + when(connection.isValid(5)).thenReturn(false); + + assertThrows(SQLTransientConnectionException.class, database::getConnection); + + verify(dataSource, times(1)).getConnection(); + verify(connection).close(); + } +} diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/SQLDBConnectionRetryTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/SQLDBConnectionRetryTest.java new file mode 100644 index 0000000000..f1e49c5d1f --- /dev/null +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/SQLDBConnectionRetryTest.java @@ -0,0 +1,84 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan.storage.database; + +import com.djrapitops.plan.exceptions.database.DBOpException; +import com.djrapitops.plan.exceptions.database.FatalDBException; +import org.junit.jupiter.api.Test; + +import java.sql.SQLException; +import java.sql.SQLNonTransientConnectionException; +import java.sql.SQLTransientConnectionException; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SQLDBConnectionRetryTest { + + @Test + void recognizesConnectionExceptionInCauseChain() { + DBOpException failure = new DBOpException( + "Connection unavailable", + new IllegalStateException(new SQLTransientConnectionException("Connection unavailable")) + ); + + assertTrue(SQLDB.isConnectionFailure(failure)); + } + + @Test + void recognizesConnectionSqlState() { + DBOpException failure = new DBOpException( + "Connection unavailable", + new SQLException("Connection unavailable", "08006") + ); + + assertTrue(SQLDB.isConnectionFailure(failure)); + } + + @Test + void doesNotRetryStatementFailure() { + DBOpException failure = new DBOpException( + "Statement failed", + new SQLException("Constraint violation", "23000", 1062) + ); + + assertFalse(SQLDB.isConnectionFailure(failure)); + } + + @Test + void doesNotRetryNonTransientConnectionFailure() { + DBOpException failure = new DBOpException( + "Connection rejected", + new SQLNonTransientConnectionException("Connection rejected", "08004") + ); + + assertFalse(SQLDB.isConnectionFailure(failure)); + } + + @Test + void doesNotRetryFatalDatabaseFailure() { + FatalDBException failure = new FatalDBException( + "Database initialization failed: ", + new DBOpException( + "Connection unavailable", + new SQLTransientConnectionException("Connection unavailable", "08001") + ) + ); + + assertFalse(SQLDB.isConnectionFailure(failure)); + } +}