Latest commit with experiments, including attempting to delete resource search URLs and SP indexes and turning off performing indexing, as well as a dedicated DeleteByUrlTest.
This commit is contained in:
Luke deGruchy
parent
1b5b281ee3
commit
813c10424e
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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<T extends IBaseResource> 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<T extends IBaseResource> 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<T extends IBaseResource> 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<List<P>> partition = Iterators.partition(theResourceIds.iterator(), 1000);
|
||||
|
||||
while (partition.hasNext()) {
|
||||
final List<P> next = partition.next();
|
||||
final List<Long> 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<T extends IBaseResource> 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<T extends IBaseResource> 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<T extends IBaseResource> 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() {
|
||||
|
|
|
|||
|
|
@ -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<ResourceSearchUrlEntity, Long>, IHapiFhirJpaRepository {
|
||||
|
||||
|
|
@ -36,4 +38,11 @@ public interface IResourceSearchUrlDao extends JpaRepository<ResourceSearchUrlEn
|
|||
@Modifying
|
||||
@Query("DELETE FROM ResourceSearchUrlEntity s WHERE (s.myResourcePid = :resID)")
|
||||
int deleteByResId(@Param("resID") long resId);
|
||||
|
||||
@Modifying
|
||||
@Query("DELETE FROM ResourceSearchUrlEntity s WHERE (s.myResourcePid IN (:resIDs))")
|
||||
int deleteByResIds(@Param("resIDs") Collection<Long> theResIds);
|
||||
|
||||
@Query("SELECT s FROM ResourceSearchUrlEntity s WHERE (s.myResourcePid = :resID)")
|
||||
List<ResourceSearchUrlEntity> finaAllByResourceId(@Param("resID") long resId);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Long> theResIds) {
|
||||
myResourceSearchUrlDao.deleteByResIds(theResIds);
|
||||
}
|
||||
|
||||
public List<ResourceSearchUrlEntity> 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.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -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<Patient> patientDao = unsafeCast(myDaoRegistry.getResourceDao("Patient"));
|
||||
final IFhirResourceDao<Observation> 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);
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
Loading…
Reference in New Issue