From 7368eccbbdbad01ddf457d1771cd81f74d75d77d Mon Sep 17 00:00:00 2001 From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com> Date: Sat, 2 Mar 2024 08:53:27 +0200 Subject: [PATCH] Optimize unsatisfied extension conditional value cleanup Extensions support @Conditional value where a boolean provider determines if other values should exist. Unsatisfied values were being removed during database cleanup task. The cleanup transaction was very slow and could hang the server if it was performed near shutdown. The cleanup is now performed on boolean value change (individual value for one player) instead of with large cleanup transaction (all values and all players). Affects issues: - #3436 --- ...edConditionalPlayerResultsTransaction.java | 2 + ...edConditionalServerResultsTransaction.java | 4 +- .../StorePlayerBooleanResultTransaction.java | 99 +++++++++++++++++++ .../StoreServerBooleanResultTransaction.java | 89 +++++++++++++++++ .../plan/storage/upkeep/DBCleanTask.java | 6 +- .../queries/ExtensionsDatabaseTest.java | 10 +- 6 files changed, 199 insertions(+), 11 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalPlayerResultsTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalPlayerResultsTransaction.java index b75d6d72d..d688fe5a6 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalPlayerResultsTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalPlayerResultsTransaction.java @@ -38,7 +38,9 @@ import static com.djrapitops.plan.storage.database.sql.building.Sql.*; * - Delete all player values with IDs that are returned by the left join query after filtering * * @author AuroraLS3 + * @deprecated Cleanup is now done as part of {@link StorePlayerBooleanResultTransaction}. */ +@Deprecated(since = "2024-03-02") public class RemoveUnsatisfiedConditionalPlayerResultsTransaction extends ThrowawayTransaction { private final String providerTable; diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalServerResultsTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalServerResultsTransaction.java index d33d10a39..666b6c9e9 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalServerResultsTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/RemoveUnsatisfiedConditionalServerResultsTransaction.java @@ -41,7 +41,9 @@ import static com.djrapitops.plan.storage.database.sql.building.Sql.*; * - Delete all server values with IDs that are returned by the left join query after filtering * * @author AuroraLS3 + * @deprecated Cleanup is now done in {@link StoreServerBooleanResultTransaction}. */ +@Deprecated(since = "2024-03-02") public class RemoveUnsatisfiedConditionalServerResultsTransaction extends ThrowawayTransaction { private final String providerTable; @@ -55,7 +57,7 @@ public class RemoveUnsatisfiedConditionalServerResultsTransaction extends Throwa tableTable = ExtensionTableProviderTable.TABLE_NAME; serverTableValueTable = ExtensionServerTableValueTable.TABLE_NAME; } - + @Override protected void performOperations() { String selectSatisfiedConditions = getSatisfiedConditionsSQL(); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java index 6bb4826f6..ee0857dad 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java @@ -19,13 +19,20 @@ package com.djrapitops.plan.extension.implementation.storage.transactions.result import com.djrapitops.plan.extension.implementation.ProviderInformation; import com.djrapitops.plan.extension.implementation.providers.Parameters; import com.djrapitops.plan.identification.ServerUUID; +import com.djrapitops.plan.storage.database.queries.QueryStatement; +import com.djrapitops.plan.storage.database.sql.building.Sql; import com.djrapitops.plan.storage.database.sql.tables.extension.ExtensionProviderTable; import com.djrapitops.plan.storage.database.transactions.ExecStatement; import com.djrapitops.plan.storage.database.transactions.Executable; import com.djrapitops.plan.storage.database.transactions.ThrowawayTransaction; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.NotNull; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; import static com.djrapitops.plan.storage.database.sql.building.Sql.AND; @@ -61,6 +68,11 @@ public class StorePlayerBooleanResultTransaction extends ThrowawayTransaction { @Override protected void performOperations() { execute(storeValue()); + commitMidTransaction(); + List providerIds = selectUnfulfilledProviderIds(); + execute(deleteUnsatisfiedConditionalResults(providerIds)); + execute(deleteUnsatisfiedConditionalGroups(providerIds)); + execute(deleteUnsatisfiedConditionalTables()); } private Executable storeValue() { @@ -104,4 +116,91 @@ public class StorePlayerBooleanResultTransaction extends ThrowawayTransaction { } }; } + + private Executable deleteUnsatisfiedConditionalResults(List providerIds) { + @Language("SQL") String deleteUnsatisfiedValues = "DELETE FROM plan_extension_user_values " + + "WHERE uuid=? " + + "AND provider_id IN (" + Sql.nParameters(providerIds.size()) + ")"; + + return deleteIds(providerIds, deleteUnsatisfiedValues); + } + + @NotNull + private ExecStatement deleteIds(List providerIds, @Language("SQL") String deleteUnsatisfiedValues) { + return new ExecStatement(deleteUnsatisfiedValues) { + @Override + public void prepare(PreparedStatement statement) throws SQLException { + statement.setString(1, playerUUID.toString()); + for (int i = 0; i < providerIds.size(); i++) { + statement.setInt(i + 2, providerIds.get(i)); + } + } + }; + } + + private Executable deleteUnsatisfiedConditionalGroups(List providerIds) { + @Language("SQL") String deleteUnsatisfiedValues = "DELETE FROM plan_extension_groups " + + "WHERE uuid=? " + + "AND provider_id IN (" + Sql.nParameters(providerIds.size()) + ")"; + + return deleteIds(providerIds, deleteUnsatisfiedValues); + } + + private List selectUnfulfilledProviderIds() { + // Need to select: + // Provider IDs where condition of this provider is met + @Language("SQL") String selectUnsatisfiedProviderIds = "SELECT unfulfilled.id " + + "FROM plan_extension_providers stored " + + "JOIN plan_extension_providers unfulfilled ON unfulfilled.condition_name=" + + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. + (value ? "CONCAT('not_', " : "") + "stored.provided_condition" + (value ? ")" : "") + + " AND stored.plugin_id=unfulfilled.plugin_id" + + " WHERE stored.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + + " AND stored.provided_condition IS NOT NULL"; + + return extractIds(selectUnsatisfiedProviderIds); + } + + private Executable deleteUnsatisfiedConditionalTables() { + List tableIds = selectUnfulfilledTableIds(); + + @Language("SQL") String deleteUnsatisfiedValues = "DELETE FROM plan_extension_user_table_values " + + "WHERE uuid=? " + + "AND table_id IN (" + Sql.nParameters(tableIds.size()) + ")"; + + return deleteIds(tableIds, deleteUnsatisfiedValues); + } + + private List selectUnfulfilledTableIds() { + // Need to select: + // Provider IDs where condition of this provider is met + @Language("SQL") String selectUnsatisfiedProviderIds = "SELECT unfulfilled.id " + + "FROM plan_extension_providers stored " + + "JOIN plan_extension_tables unfulfilled ON unfulfilled.condition_name=" + + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. + (value ? "CONCAT('not_', " : "") + "stored.provided_condition" + (value ? ")" : "") + + " AND stored.plugin_id=unfulfilled.plugin_id" + + " WHERE stored.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + + " AND stored.provided_condition IS NOT NULL"; + + return extractIds(selectUnsatisfiedProviderIds); + } + + private List extractIds(@Language("SQL") String selectUnsatisfiedProviderIds) { + return query(new QueryStatement<>(selectUnsatisfiedProviderIds) { + @Override + public void prepare(PreparedStatement statement) throws SQLException { + ExtensionProviderTable.set3PluginValuesToStatement(statement, 1, providerName, pluginName, serverUUID); + } + + @Override + public List processResults(ResultSet set) throws SQLException { + List ids = new ArrayList<>(); + while (set.next()) { + ids.add(set.getInt(1)); + } + return ids; + } + }); + } } \ No newline at end of file diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java index 4e0b25a88..c241ebaa5 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java @@ -19,13 +19,19 @@ package com.djrapitops.plan.extension.implementation.storage.transactions.result import com.djrapitops.plan.extension.implementation.ProviderInformation; import com.djrapitops.plan.extension.implementation.providers.Parameters; import com.djrapitops.plan.identification.ServerUUID; +import com.djrapitops.plan.storage.database.queries.QueryStatement; +import com.djrapitops.plan.storage.database.sql.building.Sql; import com.djrapitops.plan.storage.database.sql.tables.extension.ExtensionProviderTable; import com.djrapitops.plan.storage.database.transactions.ExecStatement; import com.djrapitops.plan.storage.database.transactions.Executable; import com.djrapitops.plan.storage.database.transactions.ThrowawayTransaction; +import org.intellij.lang.annotations.Language; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; import static com.djrapitops.plan.storage.database.sql.building.Sql.WHERE; import static com.djrapitops.plan.storage.database.sql.tables.extension.ExtensionServerValueTable.*; @@ -61,6 +67,9 @@ public class StoreServerBooleanResultTransaction extends ThrowawayTransaction { @Override protected void performOperations() { execute(storeValue()); + commitMidTransaction(); + execute(deleteUnsatisfiedConditionalResults()); + execute(deleteUnsatisfiedConditionalTables()); } private Executable storeValue() { @@ -100,4 +109,84 @@ public class StoreServerBooleanResultTransaction extends ThrowawayTransaction { } }; } + + private Executable deleteUnsatisfiedConditionalResults() { + List providerIds = selectUnfulfilledProviderIds(); + + @Language("SQL") String deleteUnsatisfiedValues = "DELETE FROM plan_extension_server_values " + + "WHERE provider_id IN (" + Sql.nParameters(providerIds.size()) + ")"; + + return new ExecStatement(deleteUnsatisfiedValues) { + @Override + public void prepare(PreparedStatement statement) throws SQLException { + for (int i = 0; i < providerIds.size(); i++) { + statement.setInt(i + 1, providerIds.get(i)); + } + } + }; + } + + private List selectUnfulfilledProviderIds() { + // Need to select: + // Provider IDs where condition of this provider is met + @Language("SQL") String selectUnsatisfiedProviderIds = "SELECT unfulfilled.id " + + "FROM plan_extension_providers stored " + + "JOIN plan_extension_providers unfulfilled ON unfulfilled.condition_name=" + + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. + (value ? "CONCAT('not_', " : "") + "stored.provided_condition" + (value ? ")" : "") + + " AND stored.plugin_id=unfulfilled.plugin_id" + + " WHERE stored.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + + " AND stored.provided_condition IS NOT NULL"; + + return extractIds(selectUnsatisfiedProviderIds); + } + + private Executable deleteUnsatisfiedConditionalTables() { + List tableIds = selectUnfulfilledTableIds(); + + @Language("SQL") String deleteUnsatisfiedValues = "DELETE FROM plan_extension_server_table_values " + + "WHERE table_id IN (" + Sql.nParameters(tableIds.size()) + ")"; + + return new ExecStatement(deleteUnsatisfiedValues) { + @Override + public void prepare(PreparedStatement statement) throws SQLException { + for (int i = 0; i < tableIds.size(); i++) { + statement.setInt(i + 1, tableIds.get(i)); + } + } + }; + } + + private List selectUnfulfilledTableIds() { + // Need to select: + // Provider IDs where condition of this provider is met + @Language("SQL") String selectUnsatisfiedProviderIds = "SELECT unfulfilled.id " + + "FROM plan_extension_providers stored " + + "JOIN plan_extension_tables unfulfilled ON unfulfilled.condition_name=" + + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. + (value ? "CONCAT('not_', " : "") + "stored.provided_condition" + (value ? ")" : "") + + " AND stored.plugin_id=unfulfilled.plugin_id" + + " WHERE stored.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + + " AND stored.provided_condition IS NOT NULL"; + + return extractIds(selectUnsatisfiedProviderIds); + } + + private List extractIds(@Language("SQL") String selectUnsatisfiedProviderIds) { + return query(new QueryStatement<>(selectUnsatisfiedProviderIds) { + @Override + public void prepare(PreparedStatement statement) throws SQLException { + ExtensionProviderTable.set3PluginValuesToStatement(statement, 1, providerName, pluginName, serverUUID); + } + + @Override + public List processResults(ResultSet set) throws SQLException { + List ids = new ArrayList<>(); + while (set.next()) { + ids.add(set.getInt(1)); + } + return ids; + } + }); + } } \ No newline at end of file diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/upkeep/DBCleanTask.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/upkeep/DBCleanTask.java index 715fbd591..ed9568e98 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/upkeep/DBCleanTask.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/upkeep/DBCleanTask.java @@ -18,8 +18,6 @@ package com.djrapitops.plan.storage.upkeep; import com.djrapitops.plan.TaskSystem; import com.djrapitops.plan.exceptions.database.DBOpException; -import com.djrapitops.plan.extension.implementation.storage.transactions.results.RemoveUnsatisfiedConditionalPlayerResultsTransaction; -import com.djrapitops.plan.extension.implementation.storage.transactions.results.RemoveUnsatisfiedConditionalServerResultsTransaction; import com.djrapitops.plan.identification.ServerInfo; import com.djrapitops.plan.query.QuerySvc; import com.djrapitops.plan.settings.config.PlanConfig; @@ -112,8 +110,8 @@ public class DBCleanTask extends TaskSystem.Task { config.get(TimeSettings.DELETE_PING_DATA_AFTER) )); database.executeTransaction(new RemoveDuplicateUserInfoTransaction()); - database.executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); - database.executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); +// database.executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); +// database.executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); int removed = cleanOldPlayers(database); if (removed > 0) { logger.info(locale.getString(PluginLang.DB_NOTIFY_CLEAN, removed)); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java index ddc01d2ff..77dc4e9c2 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java @@ -30,8 +30,6 @@ import com.djrapitops.plan.extension.implementation.results.*; import com.djrapitops.plan.extension.implementation.storage.queries.ExtensionPlayerDataQuery; import com.djrapitops.plan.extension.implementation.storage.queries.ExtensionServerDataQuery; import com.djrapitops.plan.extension.implementation.storage.queries.ExtensionServerTableDataQuery; -import com.djrapitops.plan.extension.implementation.storage.transactions.results.RemoveUnsatisfiedConditionalPlayerResultsTransaction; -import com.djrapitops.plan.extension.implementation.storage.transactions.results.RemoveUnsatisfiedConditionalServerResultsTransaction; import com.djrapitops.plan.extension.table.Table; import com.djrapitops.plan.gathering.domain.ActiveSession; import com.djrapitops.plan.gathering.domain.WorldTimes; @@ -225,7 +223,7 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { ConditionalExtension.condition = false; extensionService.updatePlayerValues(playerUUID, TestConstants.PLAYER_ONE_NAME, CallEvents.MANUAL); - db().executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); +// db().executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); // Check that the wanted data exists checkThatPlayerDataExists(ConditionalExtension.condition); @@ -234,7 +232,7 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { ConditionalExtension.condition = false; extensionService.updatePlayerValues(playerUUID, TestConstants.PLAYER_ONE_NAME, CallEvents.MANUAL); - db().executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); +// db().executeTransaction(new RemoveUnsatisfiedConditionalPlayerResultsTransaction()); // Check that the wanted data exists checkThatPlayerDataExists(ConditionalExtension.condition); @@ -278,7 +276,7 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { ConditionalExtension.condition = false; extensionService.updateServerValues(CallEvents.MANUAL); - db().executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); +// db().executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); // Check that the wanted data exists checkThatServerDataExists(ConditionalExtension.condition); @@ -287,7 +285,7 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { ConditionalExtension.condition = false; extensionService.updatePlayerValues(playerUUID, TestConstants.PLAYER_ONE_NAME, CallEvents.MANUAL); - db().executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); +// db().executeTransaction(new RemoveUnsatisfiedConditionalServerResultsTransaction()); // Check that the wanted data exists checkThatServerDataExists(ConditionalExtension.condition);