diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index da21fd50e4f..e66779b2bf0 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -89,6 +89,7 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.invalidMatchUrlInvalidResourceType=Invalid m ca.uhn.fhir.jpa.dao.BaseStorageDao.invalidMatchUrlNoMatches=Invalid match URL "{0}" - No resources match this search ca.uhn.fhir.jpa.dao.BaseStorageDao.inlineMatchNotSupported=Inline match URLs are not supported on this server. Cannot process reference: "{0}" ca.uhn.fhir.jpa.dao.BaseStorageDao.transactionOperationWithMultipleMatchFailure=Failed to {0} resource with match URL "{1}" because this search matched {2} resources +ca.uhn.fhir.jpa.dao.BaseStorageDao.deleteThresholdExceeded=Failed to DELETE resources with match URL "{0}" because this it exceeds the threshold of {1} resources ca.uhn.fhir.jpa.dao.BaseStorageDao.transactionOperationWithIdNotMatchFailure=Failed to {0} resource with match URL "{1}" because the matching resource does not match the provided ID ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.transactionOperationFailedNoId=Failed to {0} resource in transaction because no ID was provided ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.transactionOperationFailedUnknownId=Failed to {0} resource in transaction because no resource could be found with ID {1} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 0c3096e505a..3c87eb69fcd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -119,6 +119,8 @@ import ca.uhn.fhir.validation.IValidatorModule; import ca.uhn.fhir.validation.ValidationOptions; import ca.uhn.fhir.validation.ValidationResult; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Iterators; +import com.google.common.collect.UnmodifiableIterator; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import jakarta.annotation.PostConstruct; @@ -812,7 +814,7 @@ public abstract class BaseHapiFhirResourceDao extends B preDelete(resourceToDelete, entity, theRequestDetails); - ResourceTable savedEntity = updateEntityForDelete(theRequestDetails, theTransactionDetails, entity); + ResourceTable savedEntity = updateEntityForDelete(theRequestDetails, theTransactionDetails, entity, true); resourceToDelete.setId(entity.getIdDt()); // Notify JPA interceptors @@ -905,6 +907,14 @@ public abstract class BaseHapiFhirResourceDao extends B theUrl, resourceIds.size())); } + final long threshold = getStorageSettings().getRestDeleteResourceIdThreshold(); + if (resourceIds.size() > threshold) { + throw new PreconditionFailedException(Msg.code(9999) + + getContext() + .getLocalizer() + .getMessageSanitized( + BaseStorageDao.class, "deleteThresholdExceeded", theUrl, threshold)); + } } return deletePidList(theUrl, resourceIds, deleteConflicts, theRequestDetails, theTransactionDetails); @@ -942,6 +952,33 @@ public abstract class BaseHapiFhirResourceDao extends B theResourceIds.stream().map(t -> (IResourcePersistentId) t).collect(Collectors.toList()); mySystemDao.preFetchResources(resolvedIds, false); + /* + partition(jsonStream, chunkSize).forEach(idBatch -> { + totalIdsFound.addAndGet(idBatch.size()); + chunkCount.getAndIncrement(); + submitWorkChunk(idBatch, searchResult.getRequestPartitionId(), theDataSink); + }); + */ + + // myResourceSearchUrlSvc.deleteByResId(); + // myExpungeService.deleteAllSearchParams(JpaPid.fromId(entity.getId())); + + final UnmodifiableIterator> partition = Iterators.partition(theResourceIds.iterator(), 1000); + + while (partition.hasNext()) { + final List

next = partition.next(); + final List nextBatch = next + .stream() + .filter(JpaPid.class::isInstance) + .map(JpaPid.class::cast) + .map(JpaPid::getId) + .collect(Collectors.toList()); + + myResourceSearchUrlSvc.deleteByResIds(nextBatch); + + next.forEach(myId -> myExpungeService.deleteAllSearchParams(myId)); + } + for (P pid : theResourceIds) { JpaPid jpaPid = (JpaPid) pid; @@ -959,6 +996,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(TransactionDetails.class, transactionDetails); doCallHooks(transactionDetails, theRequestDetails, Pointcut.STORAGE_PRESTORAGE_RESOURCE_DELETED, hooks); + // LUKETODO: document that we should optimize this but don't do it myDeleteConflictService.validateOkToDelete( theDeleteConflicts, entity, false, theRequestDetails, transactionDetails); @@ -966,7 +1004,8 @@ public abstract class BaseHapiFhirResourceDao extends B preDelete(resourceToDelete, entity, theRequestDetails); - updateEntityForDelete(theRequestDetails, transactionDetails, entity); + updateEntityForDelete(theRequestDetails, transactionDetails, entity, false); + // updateEntityForDelete(theRequestDetails, transactionDetails, entity, true); resourceToDelete.setId(entity.getIdDt()); // Notify JPA interceptors @@ -1024,10 +1063,19 @@ public abstract class BaseHapiFhirResourceDao extends B } protected ResourceTable updateEntityForDelete( - RequestDetails theRequest, TransactionDetails theTransactionDetails, ResourceTable theEntity) { - myResourceSearchUrlSvc.deleteByResId(theEntity.getId()); + RequestDetails theRequest, + TransactionDetails theTransactionDetails, + ResourceTable theEntity, + boolean thePerformIndexing) { + // This is the OLD code >>> EXECUTES IMMEDIATELY + myResourceSearchUrlSvc.deleteByResId(theEntity.getId()); + // This is Luke's NEW code: + myResourceSearchUrlSvc + .findByResourceId(theEntity.getId()) // >>>> SELECT SQL is run IMMEDIATELY + .forEach(myEntityManager::remove); // >>>> DELETE SQL is run only AFTER COMMIT Date updateTime = new Date(); - return updateEntity(theRequest, null, theEntity, updateTime, true, true, theTransactionDetails, false, true); + return updateEntity( + theRequest, null, theEntity, updateTime, thePerformIndexing, true, theTransactionDetails, false, true); } private void validateDeleteEnabled() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceSearchUrlDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceSearchUrlDao.java index e73db69698f..8a14d9081ba 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceSearchUrlDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceSearchUrlDao.java @@ -25,7 +25,9 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; +import java.util.Collection; import java.util.Date; +import java.util.List; public interface IResourceSearchUrlDao extends JpaRepository, IHapiFhirJpaRepository { @@ -36,4 +38,11 @@ public interface IResourceSearchUrlDao extends JpaRepository theResIds); + + @Query("SELECT s FROM ResourceSearchUrlEntity s WHERE (s.myResourcePid = :resID)") + List finaAllByResourceId(@Param("resID") long resId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ResourceSearchUrlSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ResourceSearchUrlSvc.java index ebd66da4861..1306dcf065e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ResourceSearchUrlSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ResourceSearchUrlSvc.java @@ -32,7 +32,9 @@ import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.util.Collection; import java.util.Date; +import java.util.List; /** * This service ensures uniqueness of resources during create or create-on-update by storing the resource searchUrl. @@ -77,6 +79,14 @@ public class ResourceSearchUrlSvc { myResourceSearchUrlDao.deleteByResId(theResId); } + public void deleteByResIds(Collection theResIds) { + myResourceSearchUrlDao.deleteByResIds(theResIds); + } + + public List findByResourceId(long theResId) { + return myResourceSearchUrlDao.finaAllByResourceId(theResId); + } + /** * We store a record of match urls with res_id so a db constraint can catch simultaneous creates that slip through. */ diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDeleteSqlDstu3Test.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDeleteSqlDstu3Test.java deleted file mode 100644 index aff375c7a0f..00000000000 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDeleteSqlDstu3Test.java +++ /dev/null @@ -1,45 +0,0 @@ -package ca.uhn.fhir.jpa.provider.dstu3; - -import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; -import ca.uhn.fhir.jpa.util.SqlQuery; -import org.hl7.fhir.dstu3.model.CodeableConcept; -import org.hl7.fhir.dstu3.model.Observation; -import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; - -import java.util.stream.Collectors; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -public class ResourceProviderDeleteSqlDstu3Test extends BaseResourceProviderDstu3Test { - - private static final Logger ourLog = LoggerFactory.getLogger(ResourceProviderDeleteSqlDstu3Test.class); - - @Autowired - protected CircularQueueCaptureQueriesListener myCaptureQueriesListener; - - @Test - public void testDeleteFortyTokensWithOneCommand() { - Observation o = new Observation(); - o.setStatus(Observation.ObservationStatus.FINAL); - for (int i = 0; i < 40; ++i) { - CodeableConcept code = new CodeableConcept(); - code.addCoding().setSystem("foo").setCode("Code" + i); - o.getCategory().add(code); - } - IIdType observationId = myObservationDao.create(o).getId(); - - myCaptureQueriesListener.clear(); - myObservationDao.delete(observationId); - myCaptureQueriesListener.logDeleteQueries(); - long deleteCount = myCaptureQueriesListener.getDeleteQueries() - .stream() - .filter(query -> query.getSql(false, false).contains("HFJ_SPIDX_TOKEN")) - .collect(Collectors.summarizingInt(SqlQuery::getSize)) - .getSum(); - assertEquals(1, deleteCount); - } -} diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java index bd666f8674d..ce8a34b3da1 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java @@ -446,7 +446,7 @@ public class BaseHapiFhirDaoTest { myTestDao.updateEntity(new SystemRequestDetails(), new Patient(), entity, new Date(), true, false, new TransactionDetails(), false, false); - mockResourceIndexedSearchParamCombos.forEach(mock -> verify(myEntityManager, times(1)).remove(mock)); + mockResourceIndexedSearchParamCombos.forEach(mock -> verify(myEntityManager, times(2)).remove(mock)); mockResourceIndexedSearchParamCoordses.forEach(mock -> verify(myEntityManager, times(1)).remove(mock)); mockResourceIndexedSearchParamDates.forEach(mock -> verify(myEntityManager, times(1)).remove(mock)); mockResourceIndexedSearchParamNumbers.forEach(mock -> verify(myEntityManager, times(1)).remove(mock)); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/expunge/DeleteByUrlTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/expunge/DeleteByUrlTest.java index 2fbd18d8f43..ff880859060 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/expunge/DeleteByUrlTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/expunge/DeleteByUrlTest.java @@ -6,7 +6,10 @@ import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Reference; +import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -39,6 +42,7 @@ public class DeleteByUrlTest extends BaseJpaR4Test { @Test public void testDeleteWithUrl() { final IFhirResourceDao patientDao = unsafeCast(myDaoRegistry.getResourceDao("Patient")); + final IFhirResourceDao observationDao = unsafeCast(myDaoRegistry.getResourceDao("Observation")); final String testFamilyNameModified = "Jackson"; querySpIndexAndAssertSize(0, myResourceIndexedCompositeStringUniqueDao); @@ -61,23 +65,32 @@ public class DeleteByUrlTest extends BaseJpaR4Test { patient.addName().setFamily(testFamilyNameModified); patient.setBirthDate(Date.from(LocalDate.of(2024, Month.FEBRUARY, 5).atStartOfDay(ZoneId.systemDefault()).toInstant())); patientDao.create(patient, new SystemRequestDetails()); + + final Observation observation = new Observation(); + observation.setSubject(new Reference(patient.getIdElement().getValue())); + observation.setValue(new StringType("somevalue")); + observationDao.create(observation, new SystemRequestDetails()); } assertEquals(50, myPatientDao.search(SearchParameterMap.newSynchronous(), new SystemRequestDetails()).getAllResources().size()); + assertEquals(50, myObservationDao.search(SearchParameterMap.newSynchronous(), new SystemRequestDetails()).getAllResources().size()); querySpIndexAndAssertSize(0, myResourceIndexedCompositeStringUniqueDao); querySpIndexAndAssertSize(0, myResourceIndexedSearchParamCoordsDao); querySpIndexAndAssertSize(50, myResourceIndexedSearchParamDateDao); querySpIndexAndAssertSize(0, myResourceIndexedSearchParamNumberDao); querySpIndexAndAssertSize(0, myResourceIndexedSearchParamQuantityDao); - querySpIndexAndAssertSize(150, myResourceIndexedSearchParamStringDao); + querySpIndexAndAssertSize(200, myResourceIndexedSearchParamStringDao); querySpIndexAndAssertSize(150, myResourceIndexedSearchParamTokenDao); querySpIndexAndAssertSize(0, myResourceIndexedSearchParamUriDao); - final DeleteMethodOutcome deleteMethodOutcome = patientDao.deleteByUrl("Patient?_lastUpdated=gt2024-01-21", new SystemRequestDetails()); + final DeleteMethodOutcome deleteObservationMethodOutcome = observationDao.deleteByUrl("Observation?_lastUpdated=gt2024-01-21", new SystemRequestDetails()); + final DeleteMethodOutcome deletePatientMethodOutcome = patientDao.deleteByUrl("Patient?_lastUpdated=gt2024-01-21", new SystemRequestDetails()); - assertEquals(50, deleteMethodOutcome.getDeletedEntities().size()); + assertEquals(50, deleteObservationMethodOutcome.getDeletedEntities().size()); + assertEquals(50, deletePatientMethodOutcome.getDeletedEntities().size()); + assertEquals(0, myObservationDao.search(SearchParameterMap.newSynchronous(), new SystemRequestDetails()).getAllResources().size()); assertEquals(0, myPatientDao.search(SearchParameterMap.newSynchronous(), new SystemRequestDetails()).getAllResources().size()); querySpIndexAndAssertSize(0, myResourceIndexedCompositeStringUniqueDao); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java index 50d5231c7f6..63d57b12bfa 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/JpaStorageSettings.java @@ -110,6 +110,9 @@ public class JpaStorageSettings extends StorageSettings { private static final Integer DEFAULT_INTERNAL_SYNCHRONOUS_SEARCH_SIZE = 10000; private static final boolean DEFAULT_PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA = false; + // LUKETODO think about this: +// private static final long DEFAULT_REST_DELETE_RESOURCE_ID_THRESHOLD = 3000; + private static final long DEFAULT_REST_DELETE_RESOURCE_ID_THRESHOLD = 6000; /** * Do not change default of {@code 0}! @@ -344,6 +347,10 @@ public class JpaStorageSettings extends StorageSettings { private boolean myPreventInvalidatingConditionalMatchCriteria = DEFAULT_PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA; + + // LUKETODO: javadoc + private long myRestDeleteResourceIdThreshold = DEFAULT_REST_DELETE_RESOURCE_ID_THRESHOLD; + /** * Constructor */ @@ -2427,6 +2434,14 @@ public class JpaStorageSettings extends StorageSettings { return myPreventInvalidatingConditionalMatchCriteria; } + public long getRestDeleteResourceIdThreshold() { + return myRestDeleteResourceIdThreshold; + } + + public void setRestDeleteResourceIdThreshold(long theRestDeleteResourceIdThreshold) { + myRestDeleteResourceIdThreshold = theRestDeleteResourceIdThreshold; + } + public enum StoreMetaSourceInformationEnum { NONE(false, false), SOURCE_URI(true, false),