Drop tag definition constraint

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6582-drop-tag-fk-constraint.yaml

@@ -0,0 +1,4 @@
+---
+type: perf
+issue: 6582
+title: "Under heavy load, a foreign key constraint in the Tag Definition table (used for Tags, Security Labels, and Profile Definitions) can cause serious slowdowns when writing large numbers of resources (particularly if many resources contain the same tags/labels, or if the resources are being written individually or in smaller batches). This has been corrected. Also, a foreign key constraint on the Resource Link table has been dropped. This will significantly improve performance when writing resource collections with many links to resources not also in the same Bundle."

hapi-fhir-jpa-hibernate-services/src/main/java/ca/uhn/hapi/fhir/sql/hibernatesvc/DatabasePartitionModeIdFilteringMappingContributor.java

@@ -192,6 +192,15 @@ private void removePartitionedIdColumnsFromMetadata(
 						theClassLoaderService, theMetadata, foreignKey, table, nextEntry.getKey());
 			}
 
+			for (Property property : entityPersistentClass.getProperties()) {
+				Value value = property.getValue();
+				if (value instanceof ToOne) {
+					ToOne toOne = (ToOne) value;
+					filterPropertiesFromToOneRelationship(
+							theClassLoaderService, theMetadata, table, entityPersistentClass.getClassName(), toOne);
+				}
+			}
+
 			for (UniqueKey uniqueKey : table.getUniqueKeys().values()) {
 				// Adjust UniqueKey constraints, which are uniqueness
 				// constraints automatically generated to support FKs on
@@ -346,22 +355,9 @@ private void filterPartitionedIdsFromLocalFks(
 		Value value = theForeignKey.getColumn(0).getValue();
 		if (value instanceof ToOne) {
 			ToOne manyToOne = (ToOne) value;
-
-			String targetTableName = theMetadata
-					.getEntityBindingMap()
-					.get(manyToOne.getReferencedEntityName())
-					.getTable()
-					.getName();
-			Class<?> entityType = getType(theClassLoaderService, theEntityTypeName);
-			String propertyName = manyToOne.getPropertyName();
-			Set<String> columnNamesToRemoveFromFks =
-					determineFilteredColumnNamesInForeignKey(entityType, propertyName, targetTableName);
-
-			removeColumns(manyToOne.getColumns(), t1 -> columnNamesToRemoveFromFks.contains(t1.getName()));
+			Set<String> columnNamesToRemoveFromFks = filterPropertiesFromToOneRelationship(
+					theClassLoaderService, theMetadata, theTable, theEntityTypeName, manyToOne);
 			removeColumns(theForeignKey.getColumns(), t1 -> columnNamesToRemoveFromFks.contains(t1.getName()));
-
-			columnNamesToRemoveFromFks.forEach(t -> myQualifiedIdRemovedColumnNames.add(theTable.getName() + "#" + t));
-
 		} else {
 
 			theForeignKey
@@ -371,6 +367,29 @@ private void filterPartitionedIdsFromLocalFks(
 		}
 	}
 
+	@Nonnull
+	private Set<String> filterPropertiesFromToOneRelationship(
+			ClassLoaderService theClassLoaderService,
+			InFlightMetadataCollector theMetadata,
+			Table theTable,
+			String theEntityTypeName,
+			ToOne manyToOne) {
+		String targetTableName = theMetadata
+				.getEntityBindingMap()
+				.get(manyToOne.getReferencedEntityName())
+				.getTable()
+				.getName();
+		Class<?> entityType = getType(theClassLoaderService, theEntityTypeName);
+		String propertyName = manyToOne.getPropertyName();
+		Set<String> columnNamesToRemoveFromFks =
+				determineFilteredColumnNamesInForeignKey(entityType, propertyName, targetTableName);
+
+		removeColumns(manyToOne.getColumns(), t1 -> columnNamesToRemoveFromFks.contains(t1.getName()));
+
+		columnNamesToRemoveFromFks.forEach(t -> myQualifiedIdRemovedColumnNames.add(theTable.getName() + "#" + t));
+		return columnNamesToRemoveFromFks;
+	}
+
 	private void filterPartitionedIdsFromUniqueConstraints(UniqueKey uniqueKey, Table table) {
 		uniqueKey
 				.getColumns()

hapi-fhir-jpaserver-base/pom.xml

@@ -563,10 +563,14 @@
 						<dialect>
 							<className>ca.uhn.fhir.jpa.model.dialect.HapiFhirOracleDialect</className>
 							<targetFileName>oracle.sql</targetFileName>
+							<!-- We may be able to do this in a cleaner way once this is resolved: https://hibernate.atlassian.net/browse/HHH-19046 -->
+							<dropStatementsContainingRegex>add constraint FK_RESLINK_TARGET</dropStatementsContainingRegex>
 						</dialect>
 						<dialect>
 							<className>ca.uhn.fhir.jpa.model.dialect.HapiFhirSQLServerDialect</className>
 							<targetFileName>sqlserver.sql</targetFileName>
+							<!-- We may be able to do this in a cleaner way once this is resolved: https://hibernate.atlassian.net/browse/HHH-19046 -->
+							<dropStatementsContainingRegex>add constraint FK_RESLINK_TARGET</dropStatementsContainingRegex>
 						</dialect>
 						<dialect>
 							<className>ca.uhn.fhir.jpa.model.dialect.HapiFhirCockroachDialect</className>
@@ -580,6 +584,8 @@
 							<className>ca.uhn.fhir.jpa.model.dialect.HapiFhirPostgresDialect</className>
 							<targetFileName>postgres.sql</targetFileName>
 							<appendFile>classpath:ca/uhn/fhir/jpa/docs/database/hapifhirpostgres94-init01.sql</appendFile>
+							<!-- We may be able to do this in a cleaner way once this is resolved: https://hibernate.atlassian.net/browse/HHH-19046 -->
+							<dropStatementsContainingRegex>add constraint FK_RESLINK_TARGET</dropStatementsContainingRegex>
 						</dialect>
 					</dialects>
 				</configuration>

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -671,17 +671,17 @@ private boolean updateTags(
 
 		allResourceTagsNewAndOldFromTheEntity.forEach(tag -> {
 
-			// Don't keep duplicate tags
-			if (!allTagDefinitionsPresent.add(tag.getTag())) {
+			// Don't keep duplicate tags or tags with a missing definition
+			TagDefinition tagDefinition = tag.getTag();
+			if (tagDefinition == null || !allTagDefinitionsPresent.add(tagDefinition)) {
 				theEntity.getTags().remove(tag);
 			}
 
 			// Drop any tags that have been removed
-			if (!allResourceTagsFromTheResource.contains(tag)) {
+			if (tagDefinition != null && !allResourceTagsFromTheResource.contains(tag)) {
 				if (shouldDroppedTagBeRemovedOnUpdate(theRequest, tag)) {
 					theEntity.getTags().remove(tag);
-				} else if (HapiExtensions.EXT_SUBSCRIPTION_MATCHING_STRATEGY.equals(
-						tag.getTag().getSystem())) {
+				} else if (HapiExtensions.EXT_SUBSCRIPTION_MATCHING_STRATEGY.equals(tagDefinition.getSystem())) {
 					theEntity.getTags().remove(tag);
 				}
 			}
@@ -766,6 +766,9 @@ public BaseHasResource readEntity(IIdType theValueId, RequestDetails theRequest)
 	 * @return Returns <code>true</code> if the tag should be removed
 	 */
 	protected boolean shouldDroppedTagBeRemovedOnUpdate(RequestDetails theRequest, ResourceTag theTag) {
+		if (theTag.getTag() == null) {
+			return true;
+		}
 
 		Set<TagTypeEnum> metaSnapshotModeTokens = null;
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java

@@ -490,7 +490,19 @@ private <R extends IBaseResource> R populateResourceMetadataRi(
 			res.getMeta().getTag().clear();
 			res.getMeta().getProfile().clear();
 			res.getMeta().getSecurity().clear();
+
+			boolean haveWarnedForMissingTag = false;
 			for (BaseTag next : theTagList) {
+				if (next.getTag() == null) {
+					if (!haveWarnedForMissingTag) {
+						ourLog.warn(
+								"Tag definition HFJ_TAG_DEF#{} is missing, returned Resource.meta may not be complete",
+								next.getTagId());
+						haveWarnedForMissingTag = true;
+					}
+					continue;
+				}
+
 				switch (next.getTag().getTagType()) {
 					case PROFILE:
 						res.getMeta().addProfile(next.getTag().getCode());

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java

@@ -379,6 +379,29 @@ protected void init780() {
 				.nullable()
 				.withType(ColumnTypeEnum.TINYINT)
 				.heavyweightSkipByDefault();
+
+		/*
+		 * These two constraints were the last two that we had that used
+		 * hibernate-generated names. Yay!
+		 */
+		version.onTable("HFJ_RES_TAG").dropForeignKey("20250115.10", "FKbfcjbaftmiwr3rxkwsy23vneo", "HFJ_TAG_DEF");
+		version.onTable("HFJ_HISTORY_TAG").dropForeignKey("20250115.20", "FKtderym7awj6q8iq5c51xv4ndw", "HFJ_TAG_DEF");
+
+		/*
+		 * This migration drops a constraint from ResourceLink#myTargetResource. Not having this
+		 * constraint is a significant performance improvement in some cases, and the column is
+		 * nullable anyhow already because we also have the possibility of having a logical reference
+		 * instead of a hard one. We still keep the constraint present on the ResourceLink
+		 * entity for two reasons:
+		 * 1. We want to leave it in place on H2 to ensure that it helps to catch any bugs.
+		 * 2. We can't drop it as of 2025-01-16 because of this Hibernate bug:
+		 *    https://hibernate.atlassian.net/browse/HHH-19046
+		 */
+		version.onTable("HFJ_RES_LINK")
+				.dropForeignKey("20250115.30", "FK_RESLINK_TARGET", "HFJ_RESOURCE")
+				.runEvenDuringSchemaInitialization()
+				.onlyAppliesToPlatforms(
+						DriverTypeEnum.POSTGRES_9_4, DriverTypeEnum.MSSQL_2012, DriverTypeEnum.ORACLE_12C);
 	}
 
 	protected void init780_afterPartitionChanges() {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/entity/GeneratedSchemaTest.java

@@ -1,13 +1,11 @@
 package ca.uhn.fhir.jpa.entity;
 
-import ca.uhn.fhir.batch2.model.WorkChunkStatusEnum;
-import ca.uhn.fhir.jpa.model.dialect.HapiFhirH2Dialect;
 import ca.uhn.fhir.util.ClasspathUtil;
 import jakarta.annotation.Nonnull;
 import org.apache.commons.lang3.StringUtils;
-import org.hibernate.search.mapper.pojo.common.annotation.Param;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.Arrays;
@@ -37,21 +35,13 @@ public void testVerifyLongVarcharColumnDefinition() {
 		validateLongVarcharDatatype("sqlserver.sql", "varchar(max)");
 	}
 
-	private static void validateLongVarcharDatatype(String schemaName, String expectedDatatype) {
-		String schema = ClasspathUtil.loadResource("ca/uhn/hapi/fhir/jpa/docs/database/nonpartitioned/" +
-			"" + schemaName);
-		String[] lines = StringUtils.split(schema, '\n');
-		String resTextVc = Arrays.stream(lines).filter(t -> t.contains("RES_TEXT_VC ")).findFirst().orElseThrow();
-		assertThat(resTextVc).as("Wrong type in " + schemaName).contains("RES_TEXT_VC " + expectedDatatype);
-	}
-
 	@Test
 	void testVerifyPksNonPartitioned() {
 		String file = "ca/uhn/hapi/fhir/jpa/docs/database/nonpartitioned/postgres.sql";
 		List<String> lines = loadSchemaLines(file);
 
 		List<String> fks = lines.stream().filter(t -> t.startsWith("alter table if exists HFJ_RES_VER add constraint FK_RESOURCE_HISTORY_RESOURCE ")).toList();
-		assertEquals(1, fks.size(), ()-> String.join("\n", lines));
+		assertEquals(1, fks.size(), () -> String.join("\n", lines));
 		assertThat(fks.get(0)).contains("foreign key (RES_ID)");
 	}
 
@@ -61,10 +51,34 @@ void testVerifyPksPartitioned() {
 		List<String> lines = loadSchemaLines(file);
 
 		List<String> fks = lines.stream().filter(t -> t.startsWith("alter table if exists HFJ_RES_VER add constraint FK_RESOURCE_HISTORY_RESOURCE ")).toList();
-		assertEquals(1, fks.size(), ()-> String.join("\n", lines));
+		assertEquals(1, fks.size(), () -> String.join("\n", lines));
 		assertThat(fks.get(0)).contains("foreign key (RES_ID, PARTITION_ID)");
 	}
 
+	@ParameterizedTest
+	@CsvSource({
+		"ca/uhn/hapi/fhir/jpa/docs/database/partitioned/postgres.sql  , false",
+		"ca/uhn/hapi/fhir/jpa/docs/database/partitioned/sqlserver.sql , false",
+		"ca/uhn/hapi/fhir/jpa/docs/database/partitioned/oracle.sql    , false",
+		"ca/uhn/hapi/fhir/jpa/docs/database/partitioned/h2.sql        , true"
+	})
+	void verifyNoResourceLinkTargetFkConstraint(String theFileName, boolean theShouldContainConstraint) {
+		String file = ClasspathUtil.loadResource(theFileName);
+		if (theShouldContainConstraint) {
+			assertThat(file).contains("FK_RESLINK_TARGET");
+		} else {
+			assertThat(file).doesNotContain("FK_RESLINK_TARGET");
+		}
+	}
+
+	private static void validateLongVarcharDatatype(String schemaName, String expectedDatatype) {
+		String schema = ClasspathUtil.loadResource("ca/uhn/hapi/fhir/jpa/docs/database/nonpartitioned/" +
+			"" + schemaName);
+		String[] lines = StringUtils.split(schema, '\n');
+		String resTextVc = Arrays.stream(lines).filter(t -> t.contains("RES_TEXT_VC ")).findFirst().orElseThrow();
+		assertThat(resTextVc).as("Wrong type in " + schemaName).contains("RES_TEXT_VC " + expectedDatatype);
+	}
+
 	@Nonnull
 	private static List<String> loadSchemaLines(String file) {
 		String schema = ClasspathUtil.loadResource(file);

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java

@@ -19,10 +19,15 @@
  */
 package ca.uhn.fhir.jpa.model.entity;
 
+import jakarta.annotation.Nullable;
 import jakarta.persistence.Column;
+import jakarta.persistence.ConstraintMode;
+import jakarta.persistence.ForeignKey;
 import jakarta.persistence.JoinColumn;
 import jakarta.persistence.ManyToOne;
 import jakarta.persistence.MappedSuperclass;
+import org.hibernate.annotations.NotFound;
+import org.hibernate.annotations.NotFoundAction;
 
 import java.io.Serializable;
 
@@ -31,9 +36,15 @@ public abstract class BaseTag extends BasePartitionable implements Serializable
 
 	private static final long serialVersionUID = 1L;
 
-	// many baseTags -> one tag definition
+	/**
+	 * Every tag has a reference to the tag definition. Note that this field
+	 * must not have a FK constraint! In this case, Postgres (and maybe others)
+	 * are horribly slow writing to the table if there's an FK constraint.
+	 * See https://pganalyze.com/blog/5mins-postgres-multiXact-ids-foreign-keys-performance
+	 */
 	@ManyToOne(cascade = {})
-	@JoinColumn(name = "TAG_ID", nullable = false)
+	@JoinColumn(name = "TAG_ID", nullable = false, foreignKey = @ForeignKey(ConstraintMode.NO_CONSTRAINT))
+	@NotFound(action = NotFoundAction.IGNORE)
 	private TagDefinition myTag;
 
 	@Column(name = "TAG_ID", insertable = false, updatable = false)
@@ -43,6 +54,12 @@ public Long getTagId() {
 		return myTagId;
 	}
 
+	/**
+	 * This can be null if the tag definition has been deleted. This
+	 * should never happen unless someone has manually messed with
+	 * the database, but it could happen.
+	 */
+	@Nullable
 	public TagDefinition getTag() {
 		return myTag;
 	}

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceHistoryTag.java

@@ -154,12 +154,12 @@ public JpaPid getResourcePid() {
 	public String toString() {
 		ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
 		b.append("id", getId());
-		if (getPartitionId() != null) {
+		if (getPartitionId().getPartitionId() != null) {
 			b.append("partId", getPartitionId().getPartitionId());
 		}
 		b.append("versionId", myResourceHistoryPid);
 		b.append("resId", getResourceId());
-		b.append("tag", getTag().getId());
+		b.append("tag", getTagId());
 		return b.build();
 	}
 }

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceLink.java

@@ -124,6 +124,12 @@ public class ResourceLink extends BaseResourceIndex {
 						insertable = false,
 						updatable = false),
 			},
+			/*
+			 * TODO: We need to drop this constraint because it affects performance in pretty
+			 *  terrible ways on a lot of platforms. But a Hibernate bug present in Hibernate 6.6.4
+			 *  makes it impossible.
+			 *  See: https://hibernate.atlassian.net/browse/HHH-19046
+			 */
 			foreignKey = @ForeignKey(name = FK_RESLINK_TARGET))
 	private ResourceTable myTargetResource;
 

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTag.java

@@ -175,7 +175,7 @@ public String toString() {
 			b.append("partition", getPartitionId().getPartitionId());
 		}
 		b.append("resId", getResourceId());
-		b.append("tag", getTag().getId());
+		b.append("tag", getTagId());
 		return b.build();
 	}
 }