Backport !6100 to 7.0 (#6131)

backport 6100 - ONLINE upgrade for Azure Sql server
This commit is contained in:
Michael Buckley 2024-07-19 15:36:48 -04:00 committed by GitHub
parent b2cba5d57a
commit 05d524ac9f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 107 additions and 41 deletions

View File

@ -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."

View File

@ -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).

View File

@ -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,

View File

@ -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<String> theIncludeColumns) {
Validate.notNull(theIncludeColumns);
Objects.requireNonNull(theIncludeColumns);
myIncludeColumns = theIncludeColumns;
}

View File

@ -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);

View File

@ -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;
}

View File

@ -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);

View File

@ -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")));

View File

@ -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