diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6099-support-online-migration-azure-sql-server.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6099-support-online-migration-azure-sql-server.yaml new file mode 100644 index 00000000000..eaab3c202ef --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6099-support-online-migration-azure-sql-server.yaml @@ -0,0 +1,4 @@ +--- +type: perf +issue: 6099 +title: "Database migrations that add or drop an index no longer lock tables when running on Azure Sql Server." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md index b7afafd1358..5766f559a29 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md @@ -30,6 +30,12 @@ Note that the Oracle JDBC drivers are not distributed in the Maven Central repos java -cp hapi-fhir-cli.jar ca.uhn.fhir.cli.App migrate-database -d ORACLE_12C -u "[url]" -n "[username]" -p "[password]" ``` +# Oracle and Sql Server Locking Note + +Some versions of Oracle and Sql Server (e.g. Oracle Standard or Sql Server Standard) do NOT support adding or removing an index without locking the underlying table. +If you run migrations while these systems are running, +they will have unavoidable long pauses in activity during these changes. + ## Migrating 3.4.0 to 3.5.0+ As of HAPI FHIR 3.5.0 a new mechanism for creating the JPA index tables (HFJ_SPIDX_xxx) has been implemented. This new mechanism uses hashes in place of large multi-column indexes. This improves both lookup times as well as required storage space. This change also paves the way for future ability to provide efficient multi-tenant searches (which is not yet implemented but is planned as an incremental improvement). diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/OracleEmbeddedDatabase.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/OracleEmbeddedDatabase.java index cb9ff893b0a..584ff3b9853 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/OracleEmbeddedDatabase.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/OracleEmbeddedDatabase.java @@ -40,6 +40,7 @@ public class OracleEmbeddedDatabase extends JpaEmbeddedDatabase { public OracleEmbeddedDatabase() { myContainer = new OracleContainer("gvenzl/oracle-xe:21-slim-faststart").withPrivilegedMode(true); + myContainer.start(); super.initialize( DriverTypeEnum.ORACLE_12C, diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java index e7e3b079e2b..6a3ce565b61 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java @@ -33,6 +33,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Set; public class AddIndexTask extends BaseTableTask { @@ -69,7 +70,7 @@ public class AddIndexTask extends BaseTableTask { super.validate(); Validate.notBlank(myIndexName, "Index name not specified"); Validate.isTrue( - myColumns.size() > 0, + !myColumns.isEmpty(), "Columns not specified for AddIndexTask " + myIndexName + " on table " + getTableName()); Validate.notNull(myUnique, "Uniqueness not specified"); setDescription("Add " + myIndexName + " index to table " + getTableName()); @@ -141,7 +142,7 @@ public class AddIndexTask extends BaseTableTask { } // Should we do this non-transactionally? Avoids a write-lock, but introduces weird failure modes. String postgresOnlineClause = ""; - String msSqlOracleOnlineClause = ""; + String oracleOnlineClause = ""; if (myOnline) { switch (getDriverType()) { case POSTGRES_9_4: @@ -150,25 +151,66 @@ public class AddIndexTask extends BaseTableTask { // This runs without a lock, and can't be done transactionally. setTransactional(false); break; - case ORACLE_12C: - if (myMetadataSource.isOnlineIndexSupported(getConnectionProperties())) { - msSqlOracleOnlineClause = " ONLINE DEFERRED INVALIDATION"; - } - break; case MSSQL_2012: + // handled below in buildOnlineCreateWithTryCatchFallback() + break; + case ORACLE_12C: + // todo: delete this once we figure out how run Oracle try-catch to match MSSQL. if (myMetadataSource.isOnlineIndexSupported(getConnectionProperties())) { - msSqlOracleOnlineClause = " WITH (ONLINE = ON)"; + oracleOnlineClause = " ONLINE DEFERRED INVALIDATION"; } break; default: } } - String sql = "create " + unique + "index " + postgresOnlineClause + myIndexName + " on " + getTableName() + "(" - + columns + ")" + includeClause + mssqlWhereClause + msSqlOracleOnlineClause; + String bareCreateSql = "create " + unique + "index " + postgresOnlineClause + myIndexName + " on " + + getTableName() + "(" + columns + ")" + includeClause + mssqlWhereClause + oracleOnlineClause; + + String sql; + if (myOnline && DriverTypeEnum.MSSQL_2012 == getDriverType()) { + sql = buildOnlineCreateWithTryCatchFallback(bareCreateSql); + } else { + sql = bareCreateSql; + } return sql; } + /** + * Wrap a Sql Server create index in a try/catch to try it first ONLINE + * (meaning no table locks), and on failure, without ONLINE (locking the table). + * + * This try-catch syntax was manually tested via sql + * {@code + * BEGIN TRY + * EXEC('create index FOO on TABLE_A (col1) WITH (ONLINE = ON)'); + * select 'Online-OK'; + * END TRY + * BEGIN CATCH + * create index FOO on TABLE_A (col1); + * select 'Offline'; + * END CATCH; + * -- Then inspect the result set - Online-OK means it ran the ONLINE version. + * -- Note: we use EXEC() in the online path to lower the severity of the error + * -- so the CATCH can catch it. + * } + * + * @param bareCreateSql + * @return + */ + static @Nonnull String buildOnlineCreateWithTryCatchFallback(String bareCreateSql) { + // Some "Editions" of Sql Server do not support ONLINE. + // @format:off + return "BEGIN TRY -- try first online, without locking the table \n" + + " EXEC('" + bareCreateSql + " WITH (ONLINE = ON)');\n" + + "END TRY \n" + + "BEGIN CATCH -- for Editions of Sql Server that don't support ONLINE, run with table locks \n" + + bareCreateSql + + "; \n" + + "END CATCH;"; + // @format:on + } + @Nonnull private String buildMSSqlNotNullWhereClause() { String mssqlWhereClause; @@ -192,7 +234,7 @@ public class AddIndexTask extends BaseTableTask { } private void setIncludeColumns(List theIncludeColumns) { - Validate.notNull(theIncludeColumns); + Objects.requireNonNull(theIncludeColumns); myIncludeColumns = theIncludeColumns; } diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTask.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTask.java index 80101f3ee3c..bb02eb0393e 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTask.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTask.java @@ -118,7 +118,12 @@ public class DropIndexTask extends BaseTableTask { sql.add("drop index " + myIndexName + (myOnline ? " ONLINE" : "")); break; case MSSQL_2012: - sql.add("drop index " + getTableName() + "." + myIndexName); + // use a try-catch to try online first, and fail over to lock path. + String sqlServerDrop = "drop index " + getTableName() + "." + myIndexName; + if (myOnline) { + sqlServerDrop = AddIndexTask.buildOnlineCreateWithTryCatchFallback(sqlServerDrop); + } + sql.add(sqlServerDrop); break; case COCKROACHDB_21_1: sql.add("drop index " + getTableName() + "@" + myIndexName); diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSource.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSource.java index 9a271968e7c..7a78f51edf0 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSource.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSource.java @@ -32,16 +32,23 @@ public class MetadataSource { */ public boolean isOnlineIndexSupported(DriverTypeEnum.ConnectionProperties theConnectionProperties) { + // todo: delete this once we figure out how run Oracle try-catch as well. switch (theConnectionProperties.getDriverType()) { case POSTGRES_9_4: case COCKROACHDB_21_1: return true; case MSSQL_2012: + // use a deny-list instead of allow list, so we have a better failure mode for new/unknown versions. + // Better to fail in dev than run with a table lock in production. String mssqlEdition = getEdition(theConnectionProperties); - return mssqlEdition.startsWith("Enterprise"); + return mssqlEdition == null // some weird version without an edition? + || + // these versions don't support ONLINE index creation + !mssqlEdition.startsWith("Standard Edition"); case ORACLE_12C: String oracleEdition = getEdition(theConnectionProperties); - return oracleEdition.contains("Enterprise"); + return oracleEdition == null // weird unknown version - try, and maybe fail. + || oracleEdition.contains("Enterprise"); default: return false; } diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTaskTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTaskTest.java index cb5ad881aea..a1b03c19897 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTaskTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTaskTest.java @@ -22,6 +22,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasItem; import static org.junit.jupiter.api.Assertions.assertEquals; +@SuppressWarnings("SqlDialectInspection") @MockitoSettings(strictness = Strictness.LENIENT) public class AddIndexTaskTest extends BaseTest { @@ -180,7 +181,7 @@ public class AddIndexTaskTest extends BaseTest { public void platformSyntaxWhenOn(DriverTypeEnum theDriver) { myTask.setDriverType(theDriver); myTask.setOnline(true); - DriverTypeEnum.ConnectionProperties props; + Mockito.when(mockMetadataSource.isOnlineIndexSupported(Mockito.any())).thenReturn(true); mySql = myTask.generateSql(); switch (theDriver) { @@ -192,7 +193,12 @@ public class AddIndexTaskTest extends BaseTest { assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) ONLINE DEFERRED INVALIDATION", mySql); break; case MSSQL_2012: - assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) WITH (ONLINE = ON)", mySql); + assertEquals("BEGIN TRY -- try first online, without locking the table \n" + + " EXEC('create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) WITH (ONLINE = ON)');\n" + + "END TRY \n" + + "BEGIN CATCH -- for Editions of Sql Server that don't support ONLINE, run with table locks \n" + + "create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL); \n" + + "END CATCH;", mySql); break; default: // unsupported is ok. But it means we lock the table for a bit. @@ -201,32 +207,19 @@ public class AddIndexTaskTest extends BaseTest { } } + /** + * We sniff the edition of Oracle to detect support for ONLINE migrations. + */ @ParameterizedTest(name = "{index}: {0}") @ValueSource(booleans = { true, false } ) - public void offForUnsupportedVersionsOfSqlServer(boolean theSupportedFlag) { - myTask.setDriverType(DriverTypeEnum.MSSQL_2012); - myTask.setOnline(true); - myTask.setMetadataSource(mockMetadataSource); - Mockito.when(mockMetadataSource.isOnlineIndexSupported(Mockito.any())).thenReturn(theSupportedFlag); - - mySql = myTask.generateSql(); - if (theSupportedFlag) { - assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) WITH (ONLINE = ON)", mySql); - } else { - assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL)", mySql); - } - } - - @ParameterizedTest(name = "{index}: {0}") - @ValueSource(booleans = { true, false } ) - public void offForUnsupportedVersionsOfOracleServer(boolean theSupportedFlag) { + public void offForUnsupportedVersionsOfOracleServer(boolean theOnlineIndexingSupportedFlag) { myTask.setDriverType(DriverTypeEnum.ORACLE_12C); myTask.setOnline(true); myTask.setMetadataSource(mockMetadataSource); - Mockito.when(mockMetadataSource.isOnlineIndexSupported(Mockito.any())).thenReturn(theSupportedFlag); + Mockito.when(mockMetadataSource.isOnlineIndexSupported(Mockito.any())).thenReturn(theOnlineIndexingSupportedFlag); mySql = myTask.generateSql(); - if (theSupportedFlag) { + if (theOnlineIndexingSupportedFlag) { assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) ONLINE DEFERRED INVALIDATION", mySql); } else { assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL)", mySql); diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTaskTest.java similarity index 95% rename from hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTest.java rename to hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTaskTest.java index 8f02d139ed9..77e0f9b2431 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTaskTest.java @@ -18,8 +18,9 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertEquals; -public class DropIndexTest extends BaseTest { +public class DropIndexTaskTest extends BaseTest { @ParameterizedTest(name = "{index}: {0}") @@ -251,7 +252,12 @@ public class DropIndexTest extends BaseTest { assertThat(mySql, equalTo(asList("drop index IDX_ANINDEX ONLINE"))); break; case MSSQL_2012: - assertThat(mySql, equalTo(asList("drop index SOMETABLE.IDX_ANINDEX"))); + assertEquals(asList("BEGIN TRY -- try first online, without locking the table \n" + + " EXEC('drop index SOMETABLE.IDX_ANINDEX WITH (ONLINE = ON)');\n" + + "END TRY \n" + + "BEGIN CATCH -- for Editions of Sql Server that don't support ONLINE, run with table locks \n" + + "drop index SOMETABLE.IDX_ANINDEX; \n" + + "END CATCH;"), mySql); break; case POSTGRES_9_4: assertThat(mySql, equalTo(asList("drop index CONCURRENTLY IDX_ANINDEX"))); diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSourceTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSourceTest.java index ed1d3253099..23959d34dbe 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSourceTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/MetadataSourceTest.java @@ -32,12 +32,14 @@ class MetadataSourceTest { "ORACLE_12C,Oracle Database 19c Enterprise Edition Release 19.0.0.0.0 - Production,true", "ORACLE_12C,Oracle Database 19c Express Edition Release 11.2.0.2.0 - 64bit Production,false", "COCKROACHDB_21_1,,true", - // sql server only supports it in Enterprise + // sql server only supports it in Enterprise and Developer // https://docs.microsoft.com/en-us/sql/sql-server/editions-and-components-of-sql-server-2019?view=sql-server-ver16#RDBMSHA - "MSSQL_2012,Developer Edition (64-bit),false", - "MSSQL_2012,Developer Edition (64-bit),false", + "MSSQL_2012,Developer Edition (64-bit),true", + "MSSQL_2012,Developer Edition (64-bit),true", "MSSQL_2012,Standard Edition (64-bit),false", - "MSSQL_2012,Enterprise Edition (64-bit),true" + "MSSQL_2012,Enterprise Edition (64-bit),true", + "MSSQL_2012,Azure SQL Edge Developer (64-bit),true", + "MSSQL_2012,Azure SQL Edge Premium (64-bit),true" }) void isOnlineIndexSupported(DriverTypeEnum theType, String theEdition, boolean theSupportedFlag) { // stub out our Sql Server edition lookup