diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java index 0e24a6af2..c29c3f733 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Objects; import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Consumer; @@ -27,6 +28,8 @@ public final class MysqlUserBackend implements SqlUserBackend { private static final int USER_PAGE_SIZE = 512; + private static final long PEER_MIGRATION_GRACE_NANOS = TimeUnit.SECONDS.toNanos(5); + private static final long PEER_MIGRATION_RECHECK_MILLIS = 50; private final SqlUserSchema schema; private final SqlBackendLogger logger; private final HeadlessUserTable table; @@ -351,15 +354,18 @@ private void renamePostgresColumn(String storedName, String requestedName) throw private void migrateRetainedColumnToString(String storedName, SqlUserSchema.ColumnDefinition definition) throws SQLException { int jdbcType = registeredColumnType(storedName); - if (jdbcType != java.sql.Types.TINYINT && jdbcType != java.sql.Types.SMALLINT - && jdbcType != java.sql.Types.INTEGER && jdbcType != java.sql.Types.BIGINT - && jdbcType != java.sql.Types.REAL && jdbcType != java.sql.Types.FLOAT - && jdbcType != java.sql.Types.DOUBLE && jdbcType != java.sql.Types.NUMERIC - && jdbcType != java.sql.Types.DECIMAL && jdbcType != java.sql.Types.BOOLEAN - && jdbcType != java.sql.Types.BIT) return; + if (!isNumericOrBoolean(jdbcType)) return; String column = quote(storedName); if (getDbType() != DbType.POSTGRESQL) { - MysqlColumnAttributes attributes = mysqlColumnAttributes(storedName); + MysqlColumnAttributes attributes; + try { attributes = mysqlColumnAttributes(storedName); } + catch (SQLException inspectionFailure) { + if (migrationCompletedByPeer(storedName, inspectionFailure)) return; + throw inspectionFailure; + } + // Database metadata and information_schema are read through separate + // connections. A peer can finish the ALTER between those reads. + if (!retainedColumnNeedsStringMigration(storedName)) return; if (attributes.extra() != null && !attributes.extra().isBlank()) { throw new SQLException("Cannot safely migrate SQL column with generated or automatic attributes: " + storedName); @@ -375,6 +381,8 @@ private void migrateRetainedColumnToString(String storedName, try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); + } catch (SQLException ddlFailure) { + if (!migrationCompletedByPeer(storedName, ddlFailure)) throw ddlFailure; } return; } @@ -392,9 +400,44 @@ private void migrateRetainedColumnToString(String storedName, try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql.toString())) { statement.executeUpdate(); + } catch (SQLException ddlFailure) { + if (!migrationCompletedByPeer(storedName, ddlFailure)) throw ddlFailure; } } + private boolean migrationCompletedByPeer(String storedName, SQLException failure) throws SQLException { + long deadline = System.nanoTime() + PEER_MIGRATION_GRACE_NANOS; + do { + try { + if (!retainedColumnNeedsStringMigration(storedName)) return true; + } catch (SQLException inspectionFailure) { + if (inspectionFailure != failure) failure.addSuppressed(inspectionFailure); + } + if (System.nanoTime() >= deadline) return false; + try { Thread.sleep(PEER_MIGRATION_RECHECK_MILLIS); } + catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + failure.addSuppressed(new SQLException( + "Interrupted while waiting for concurrent SQL column migration: " + storedName, + interrupted)); + return false; + } + } while (true); + } + + private boolean retainedColumnNeedsStringMigration(String storedName) throws SQLException { + return isNumericOrBoolean(registeredColumnType(storedName)); + } + + private static boolean isNumericOrBoolean(int jdbcType) { + return jdbcType == java.sql.Types.TINYINT || jdbcType == java.sql.Types.SMALLINT + || jdbcType == java.sql.Types.INTEGER || jdbcType == java.sql.Types.BIGINT + || jdbcType == java.sql.Types.REAL || jdbcType == java.sql.Types.FLOAT + || jdbcType == java.sql.Types.DOUBLE || jdbcType == java.sql.Types.NUMERIC + || jdbcType == java.sql.Types.DECIMAL || jdbcType == java.sql.Types.BOOLEAN + || jdbcType == java.sql.Types.BIT; + } + private MysqlColumnAttributes mysqlColumnAttributes(String name) throws SQLException { String sql = "SELECT IS_NULLABLE, COLUMN_DEFAULT, EXTRA, COLUMN_COMMENT " + "FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME=? AND COLUMN_NAME=?"; diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackendSchemaExpansionTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackendSchemaExpansionTest.java index 94bcedc28..6d8d225df 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackendSchemaExpansionTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackendSchemaExpansionTest.java @@ -12,6 +12,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.concurrent.CopyOnWriteArrayList; import org.junit.jupiter.api.Test; import org.mockito.MockedConstruction; @@ -98,6 +99,56 @@ private void checkTypes(DbType type) throws Exception { assertMysqlFamilyMigratesRetainedNumericColumn(DbType.MARIADB); } + @Test void mariaDbAcceptsPeerMigrationBeforeAttributeInspectionCompletes() throws Exception { + Fixture fixture = new Fixture(DbType.MARIADB); + fixture.existing.add("VoteRemindersLast"); + fixture.numericColumns.add("VoteRemindersLast"); + fixture.peerMigrationDuringAttributeInspection = true; + SqlUserSchema schema = SqlUserSchema.builder() + .column("VoteRemindersLast", "TEXT", DataType.STRING).build(); + + try (var managers = fixture.managers(); var backend = fixture.open(schema)) { + assertTrue(backend.isOpen()); + assertFalse(fixture.numericColumns.contains("VoteRemindersLast")); + assertFalse(fixture.sql.stream().anyMatch(sql -> sql.startsWith( + "ALTER TABLE `Users` MODIFY COLUMN `VoteRemindersLast`"))); + } + fixture.assertClosed(); + } + + @Test void mariaDbWaitsForPeerMigrationAfterAttributeInspectionFailure() throws Exception { + Fixture fixture = new Fixture(DbType.MARIADB); + fixture.existing.add("VoteRemindersLast"); + fixture.numericColumns.add("VoteRemindersLast"); + fixture.columnDefaults.put("VoteRemindersLast", "'0'"); + fixture.peerMigrationAfterAttributeFailure = true; + SqlUserSchema schema = SqlUserSchema.builder() + .column("VoteRemindersLast", "TEXT", DataType.STRING).build(); + + try (var managers = fixture.managers(); var backend = fixture.open(schema)) { + assertTrue(backend.isOpen()); + assertFalse(fixture.numericColumns.contains("VoteRemindersLast")); + assertFalse(fixture.sql.stream().anyMatch(sql -> sql.startsWith( + "ALTER TABLE `Users` MODIFY COLUMN `VoteRemindersLast`"))); + } + fixture.assertClosed(); + } + + @Test void mariaDbAcceptsPeerMigrationAfterConflictingAlter() throws Exception { + Fixture fixture = new Fixture(DbType.MARIADB); + fixture.existing.add("VoteRemindersLast"); + fixture.numericColumns.add("VoteRemindersLast"); + fixture.peerMigrationOnAlterFailure = true; + SqlUserSchema schema = SqlUserSchema.builder() + .column("VoteRemindersLast", "TEXT", DataType.STRING).build(); + + try (var managers = fixture.managers(); var backend = fixture.open(schema)) { + assertTrue(backend.isOpen()); + assertFalse(fixture.numericColumns.contains("VoteRemindersLast")); + } + fixture.assertClosed(); + } + private void assertMysqlFamilyMigratesRetainedNumericColumn(DbType dbType) throws Exception { Fixture fixture = new Fixture(dbType); fixture.existing.add("Note"); @@ -151,7 +202,7 @@ private static final class Fixture { final DbType type; final List sql = new ArrayList<>(); final List existing = new ArrayList<>(List.of("uuid")); - final List numericColumns = new ArrayList<>(); + final List numericColumns = new CopyOnWriteArrayList<>(); final List booleanColumns = new ArrayList<>(); final List nonNullableColumns = new ArrayList<>(); final java.util.Map columnDefaults = new java.util.HashMap<>(); @@ -160,6 +211,9 @@ private static final class Fixture { final List results = new ArrayList<>(); SQLException addFailure; SQLException inspectFailure; + boolean peerMigrationDuringAttributeInspection; + boolean peerMigrationAfterAttributeFailure; + boolean peerMigrationOnAlterFailure; Fixture(DbType type) { this.type = type; } MockedConstruction managers() { @@ -199,6 +253,12 @@ Connection connection() throws SQLException { return null; }).when(statement).setString(anyInt(), anyString()); when(statement.executeUpdate()).thenAnswer(ignored -> { + if (peerMigrationOnAlterFailure && query.startsWith("ALTER TABLE `Users` MODIFY COLUMN")) { + peerMigrationOnAlterFailure = false; + numericColumns.remove("VoteRemindersLast"); + columnDefaults.put("VoteRemindersLast", "'0'"); + throw new SQLException("column definition changed concurrently", "HY000", 1901); + } if (query.startsWith("ALTER TABLE") && addFailure != null) throw addFailure; return 0; }); @@ -214,6 +274,24 @@ Connection connection() throws SQLException { when(result.getString("COLUMN_DEFAULT")).thenReturn(null); } else if (query.startsWith("SELECT IS_NULLABLE, COLUMN_DEFAULT")) { String column = stringParameters[2]; + if (peerMigrationDuringAttributeInspection && "VoteRemindersLast".equals(column)) { + peerMigrationDuringAttributeInspection = false; + numericColumns.remove(column); + columnDefaults.put(column, "'0'"); + } + if (peerMigrationAfterAttributeFailure && "VoteRemindersLast".equals(column)) { + peerMigrationAfterAttributeFailure = false; + Thread peer = new Thread(() -> { + try { Thread.sleep(100); } + catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + return; + } + numericColumns.remove(column); + }, "schema-migration-peer"); + peer.setDaemon(true); + peer.start(); + } when(result.next()).thenReturn(true, false); when(result.getString(1)).thenReturn(nonNullableColumns.contains(column) ? "NO" : "YES"); when(result.getString(2)).thenReturn(columnDefaults.get(column));