Added a wait for SQLite to finish queries before closing the connection

Also:
- Transactions now execute as much as possible on the same connection instead
  of getting a new connection
- More shutdown messages when waiting for things (Like SQLite queries, or db transactions).

Affects issues:
- Possibly fixed #1814
This commit is contained in:
Risto Lahtela 2021-03-24 14:00:08 +02:00
parent a44c90f479
commit 2c8bbc80d8
13 changed files with 118 additions and 19 deletions

View File

@ -71,7 +71,6 @@ public class ActiveCookieStore implements SubSystem {
}
public static void removeUserCookie(String username) {
System.out.println(USERS_BY_COOKIE);
USERS_BY_COOKIE.entrySet().stream().filter(entry -> entry.getValue().getUsername().equals(username))
.findAny()
.map(Map.Entry::getKey)

View File

@ -58,6 +58,10 @@ public enum PluginLang implements Lang {
DISABLED_PROCESSING_COMPLETE("Disable - Processing Complete", "Processing complete."),
DISABLED_UNSAVED_SESSIONS("Disable - Unsaved Session Save", "Saving unfinished sessions.."),
DISABLED_UNSAVED_SESSIONS_TIMEOUT("Disable - Unsaved Session Save Timeout", "Timeout hit, storing the unfinished sessions on next enable instead."),
DISABLED_WAITING_SQLITE("Disable - Waiting SQLite", "Waiting queries to finish to avoid SQLite crashing JVM.."),
DISABLED_WAITING_SQLITE_COMPLETE("Disable - Waiting SQLite Complete", "Closed SQLite connection."),
DISABLED_WAITING_TRANSACTIONS("Disable - Waiting Transactions", "Waiting for unfinished transactions to avoid data loss.."),
DISABLED_WAITING_TRANSACTIONS_COMPLETE("Disable - Waiting Transactions Complete", "Transaction queue closed."),
VERSION_NEWEST("Version - Latest", "You're using the latest version."),
VERSION_AVAILABLE("Version - New", "New Release (${0}) is available ${1}"),

View File

@ -24,6 +24,7 @@ import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.PluginSettings;
import com.djrapitops.plan.settings.config.paths.TimeSettings;
import com.djrapitops.plan.settings.locale.Locale;
import com.djrapitops.plan.settings.locale.lang.PluginLang;
import com.djrapitops.plan.storage.database.queries.Query;
import com.djrapitops.plan.storage.database.transactions.Transaction;
import com.djrapitops.plan.storage.database.transactions.init.CreateIndexTransaction;
@ -123,6 +124,7 @@ public abstract class SQLDB extends AbstractDatabase {
}
transactionExecutor.shutdown();
try {
logger.info(locale.getString(PluginLang.DISABLED_WAITING_TRANSACTIONS));
Long waitMs = config.getOrDefault(TimeSettings.DB_TRANSACTION_FINISH_WAIT_DELAY, TimeUnit.SECONDS.toMillis(20L));
if (waitMs > TimeUnit.MINUTES.toMillis(5L)) {
logger.warn(TimeSettings.DB_TRANSACTION_FINISH_WAIT_DELAY.getPath() + " was set to over 5 minutes, using 5 min instead.");
@ -138,6 +140,8 @@ public abstract class SQLDB extends AbstractDatabase {
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} finally {
logger.info(locale.getString(PluginLang.DISABLED_WAITING_TRANSACTIONS_COMPLETE));
}
return Collections.emptyList();
}

View File

@ -24,6 +24,7 @@ import com.djrapitops.plan.settings.locale.lang.PluginLang;
import com.djrapitops.plan.storage.file.PlanFiles;
import com.djrapitops.plan.storage.upkeep.DBKeepAliveTask;
import com.djrapitops.plan.utilities.MiscUtils;
import com.djrapitops.plan.utilities.SemaphoreAccessCounter;
import com.djrapitops.plan.utilities.logging.ErrorContext;
import com.djrapitops.plan.utilities.logging.ErrorLogger;
import dagger.Lazy;
@ -49,6 +50,13 @@ public class SQLiteDB extends SQLDB {
private Connection connection;
private Task connectionPingTask;
/*
* In charge of keeping a single thread in control of the connection to avoid
* one thread closing the connection while another is executing a statement as
* that might lead to a SIGSEGV signal JVM crash.
*/
private final SemaphoreAccessCounter connectionLock = new SemaphoreAccessCounter();
private SQLiteDB(
File databaseFile,
Locale locale,
@ -132,6 +140,7 @@ public class SQLiteDB extends SQLDB {
if (connection == null) {
connection = getNewConnection(databaseFile);
}
connectionLock.enter();
return connection;
}
@ -140,14 +149,17 @@ public class SQLiteDB extends SQLDB {
super.close();
stopConnectionPingTask();
logger.info(locale.getString(PluginLang.DISABLED_WAITING_SQLITE));
connectionLock.waitUntilNothingAccessing();
if (connection != null) {
MiscUtils.close(connection);
}
logger.info(locale.getString(PluginLang.DISABLED_WAITING_SQLITE_COMPLETE));
}
@Override
public void returnToPool(Connection connection) {
// Connection pool not in use, no action required.
connectionLock.exit();
}
@Override

View File

@ -42,13 +42,19 @@ public class QueryAPIQuery<T> implements Query<T> {
Connection connection = null;
try {
connection = db.getConnection();
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
return performQuery.apply(preparedStatement);
}
return executeWithConnection(connection);
} catch (SQLException e) {
throw DBOpException.forCause(sql, e);
} finally {
db.returnToPool(connection);
}
}
public T executeWithConnection(Connection connection) {
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
return performQuery.apply(preparedStatement);
} catch (SQLException e) {
throw DBOpException.forCause(sql, e);
}
}
}

View File

@ -48,9 +48,7 @@ public abstract class QueryStatement<T> implements Query<T> {
Connection connection = null;
try {
connection = db.getConnection();
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
return executeQuery(preparedStatement);
}
return executeWithConnection(connection);
} catch (SQLException e) {
throw DBOpException.forCause(sql, e);
} finally {
@ -58,6 +56,14 @@ public abstract class QueryStatement<T> implements Query<T> {
}
}
public T executeWithConnection(Connection connection) {
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
return executeQuery(preparedStatement);
} catch (SQLException e) {
throw DBOpException.forCause(sql, e);
}
}
public T executeQuery(PreparedStatement statement) throws SQLException {
try {
statement.setFetchSize(fetchSize);

View File

@ -22,6 +22,8 @@ import com.djrapitops.plan.storage.database.DBType;
import com.djrapitops.plan.storage.database.Database;
import com.djrapitops.plan.storage.database.SQLDB;
import com.djrapitops.plan.storage.database.queries.Query;
import com.djrapitops.plan.storage.database.queries.QueryAPIQuery;
import com.djrapitops.plan.storage.database.queries.QueryStatement;
import com.djrapitops.plan.utilities.logging.ErrorContext;
import net.playeranalytics.plugin.scheduling.TimeAmount;
@ -194,7 +196,13 @@ public abstract class Transaction {
}
protected <T> T query(Query<T> query) {
return query.executeQuery(db);
if (query instanceof QueryStatement) {
return ((QueryStatement<T>) query).executeWithConnection(connection);
} else if (query instanceof QueryAPIQuery) {
return ((QueryAPIQuery<T>) query).executeWithConnection(connection);
} else {
return query.executeQuery(db);
}
}
protected boolean execute(Executable executable) {
@ -226,7 +234,9 @@ public abstract class Transaction {
transaction.db = db;
transaction.dbType = dbType;
transaction.connection = this.connection;
transaction.performOperations();
if (transaction.shouldBeExecuted()) {
transaction.performOperations();
}
transaction.connection = null;
transaction.dbType = null;
transaction.db = null;

View File

@ -34,20 +34,34 @@ import static com.djrapitops.plan.storage.database.sql.building.Sql.*;
public abstract class Patch extends OperationCriticalTransaction {
private static final String ALTER_TABLE = "ALTER TABLE ";
private boolean appliedPreviously = false;
private boolean appliedNow = false;
public abstract boolean hasBeenApplied();
protected abstract void applyPatch();
public boolean isApplied() {
if (!success) throw new IllegalStateException("Asked a Patch if it is applied before it was executed!");
return appliedPreviously || appliedNow;
}
public boolean wasApplied() {
return appliedNow;
}
@Override
protected boolean shouldBeExecuted() {
return !hasBeenApplied();
boolean hasBeenApplied = hasBeenApplied();
if (hasBeenApplied) appliedPreviously = true;
return !hasBeenApplied;
}
@Override
protected void performOperations() {
if (dbType == DBType.MYSQL) disableForeignKeyChecks();
applyPatch();
appliedNow = true;
if (dbType == DBType.MYSQL) enableForeignKeyChecks();
}

View File

@ -33,14 +33,14 @@ import java.sql.Statement;
* @author Fuzzlemann
*/
public class DBKeepAliveTask extends PluginRunnable {
private final IReconnect iReconnect;
private final Reconnector reconnector;
private final PluginLogger logger;
private final ErrorLogger errorLogger;
private Connection connection;
public DBKeepAliveTask(Connection connection, IReconnect iReconnect, PluginLogger logger, ErrorLogger errorLogger) {
public DBKeepAliveTask(Connection connection, Reconnector reconnector, PluginLogger logger, ErrorLogger errorLogger) {
this.connection = connection;
this.iReconnect = iReconnect;
this.reconnector = reconnector;
this.logger = logger;
this.errorLogger = errorLogger;
}
@ -56,7 +56,7 @@ public class DBKeepAliveTask extends PluginRunnable {
}
} catch (SQLException pingException) {
try {
connection = iReconnect.reconnect();
connection = reconnector.reconnect();
} catch (SQLException reconnectionError) {
errorLogger.error(reconnectionError, ErrorContext.builder()
.whatToDo("Reload Plan and Report this if the issue persists").build());
@ -68,7 +68,7 @@ public class DBKeepAliveTask extends PluginRunnable {
}
}
public interface IReconnect {
public interface Reconnector {
Connection reconnect() throws SQLException;
}
}

View File

@ -0,0 +1,39 @@
package com.djrapitops.plan.utilities;
import java.util.concurrent.atomic.AtomicInteger;
public class SemaphoreAccessCounter {
private final AtomicInteger accessCounter;
private final Object lockObject;
public SemaphoreAccessCounter() {
accessCounter = new AtomicInteger(0);
lockObject = new Object();
}
public void enter() {
accessCounter.incrementAndGet();
}
public void exit() {
synchronized (lockObject) {
int value = accessCounter.decrementAndGet();
if (value == 0) {
lockObject.notifyAll();
}
}
}
public void waitUntilNothingAccessing() {
while (accessCounter.get() > 0) {
synchronized (lockObject) {
try {
lockObject.wait();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}
}
}

View File

@ -33,6 +33,7 @@ import utilities.mocks.PluginMockComponent;
import java.nio.file.Path;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
@ -152,6 +153,7 @@ class DBPatchMySQLRegressionTest extends DBPatchRegressionTest {
KillsOptimizationPatch patch = new KillsOptimizationPatch();
underTest.executeTransaction(patch);
assertTrue(patch.hasBeenApplied());
assertTrue(patch.isApplied());
assertFalse(patch.wasApplied());
}
}

View File

@ -86,8 +86,11 @@ abstract class DBPatchRegressionTest {
void assertPatchesHaveBeenApplied(Patch[] patches) {
List<String> failed = new ArrayList<>();
for (Patch patch : patches) {
if (!patch.hasBeenApplied()) {
if (!patch.isApplied()) {
System.out.println("! NOT APPLIED: " + patch.getClass().getSimpleName());
failed.add(patch.getClass().getSimpleName());
} else {
System.out.println(" WAS APPLIED: " + patch.getClass().getSimpleName());
}
}
assertTrue(failed.isEmpty(), "Patches " + failed + " were not applied properly.");

View File

@ -292,7 +292,7 @@ public interface DatabaseTest extends DatabaseTestPreparer {
// Test expected result
Optional<BaseUser> updatedBaseUser = db().query(BaseUserQueries.fetchBaseUserOfPlayer(playerUUID));
assertEquals(0L, updatedBaseUser.isPresent() ? updatedBaseUser.get().getRegistered() : null);
assertTrue(testedPatch.hasBeenApplied());
assertTrue(testedPatch.isApplied());
}
@Test