Do not load results into the database in JPA if there is no paging

provider
This commit is contained in:
James Agnew 2017-06-29 22:15:23 -04:00
parent 674424a30b
commit 7834bb2625
9 changed files with 127 additions and 25 deletions

View File

@ -79,4 +79,9 @@ public interface IRestfulServerDefaults {
*/
boolean isUseBrowserFriendlyContentTypes();
/**
* Returns the paging provider for this server
*/
IPagingProvider getPagingProvider();
}

View File

@ -48,6 +48,7 @@ import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
import ca.uhn.fhir.jpa.entity.*;
import ca.uhn.fhir.jpa.interceptor.IJpaServerInterceptor;
import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
import ca.uhn.fhir.jpa.term.IHapiTerminologySvc;
import ca.uhn.fhir.jpa.util.DeleteConflict;
import ca.uhn.fhir.jpa.util.StopWatch;
@ -562,6 +563,16 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
theSavedEntity.setVersion(newVersionLong);
}
protected boolean isPagingProviderDatabaseBacked(RequestDetails theRequestDetails) {
if (theRequestDetails == null) {
return false;
}
if (theRequestDetails.getServer().getPagingProvider() instanceof DatabaseBackedPagingProvider) {
return true;
}
return false;
}
@Override
public <MT extends IBaseMetaType> MT metaAddOperation(IIdType theResourceId, MT theMetaAdd, RequestDetails theRequestDetails) {
// Notify interceptors
@ -594,6 +605,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
return retVal;
}
@Override
public <MT extends IBaseMetaType> MT metaDeleteOperation(IIdType theResourceId, MT theMetaDel, RequestDetails theRequestDetails) {
// Notify interceptors
@ -628,7 +640,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
return retVal;
}
@Override
public <MT extends IBaseMetaType> MT metaGetOperation(Class<MT> theType, IIdType theId, RequestDetails theRequestDetails) {
// Notify interceptors
@ -860,12 +871,12 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
ourLog.debug("Indexing resource {} - PID {}", theResource.getIdElement().getValue(), theEntity.getId());
updateEntity(theResource, theEntity, null, true, false, theEntity.getUpdatedDate(), true, false);
}
@Override
public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm) {
removeTag(theId, theTagType, theScheme, theTerm, null);
}
@Override
public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm, RequestDetails theRequestDetails) {
// Notify interceptors
@ -919,15 +930,15 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
int max = myDaoConfig.getMaximumSearchResultCountInTransaction();
theParams.setLoadSynchronousUpTo(myDaoConfig.getMaximumSearchResultCountInTransaction());
}
if (!isPagingProviderDatabaseBacked(theRequestDetails)) {
theParams.setLoadSynchronous(true);
}
}
return mySearchCoordinatorSvc.registerSearch(this, theParams, getResourceName());
}
@Override
public Set<Long> searchForIds(SearchParameterMap theParams) {

View File

@ -46,7 +46,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2<Patient>im
super();
}
private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative) {
private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) {
SearchParameterMap paramMap = new SearchParameterMap();
if (theCount != null) {
paramMap.setCount(theCount.getValue());
@ -65,6 +65,10 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2<Patient>im
paramMap.add("_id", new StringParam(theId.getIdPart()));
}
if (!isPagingProviderDatabaseBacked(theRequestDetails)) {
paramMap.setLoadSynchronous(true);
}
return mySearchCoordinatorSvc.registerSearch(this, paramMap, getResourceName());
}
@ -74,7 +78,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2<Patient>im
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), null);
notifyInterceptors(RestOperationTypeEnum.EXTENDED_OPERATION_INSTANCE, requestDetails);
return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative);
return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails);
}
@Override
@ -83,7 +87,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2<Patient>im
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), null);
notifyInterceptors(RestOperationTypeEnum.EXTENDED_OPERATION_TYPE, requestDetails);
return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative);
return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails);
}
}

View File

@ -50,7 +50,7 @@ public class FhirResourceDaoPatientDstu3 extends FhirResourceDaoDstu3<Patient>im
@Autowired
private ISearchParamRegistry mySerarchParamRegistry;
private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative) {
private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) {
SearchParameterMap paramMap = new SearchParameterMap();
if (theCount != null) {
paramMap.setCount(theCount.getValue());
@ -69,17 +69,21 @@ public class FhirResourceDaoPatientDstu3 extends FhirResourceDaoDstu3<Patient>im
paramMap.add("_id", new StringParam(theId.getIdPart()));
}
if (!isPagingProviderDatabaseBacked(theRequestDetails)) {
paramMap.setLoadSynchronous(true);
}
return mySearchCoordinatorSvc.registerSearch(this, paramMap, getResourceName());
}
@Override
public IBundleProvider patientInstanceEverything(HttpServletRequest theServletRequest, IIdType theId, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) {
return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative);
return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails);
}
@Override
public IBundleProvider patientTypeEverything(HttpServletRequest theServletRequest, IPrimitiveType<Integer> theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) {
return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative);
return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails);
}
}

View File

@ -12,9 +12,7 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.*;
import org.springframework.web.context.ContextLoader;
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@ -25,17 +23,16 @@ import ca.uhn.fhir.jpa.config.WebsocketDstu2Config;
import ca.uhn.fhir.jpa.config.WebsocketDstu2DispatcherConfig;
import ca.uhn.fhir.jpa.dao.dstu2.BaseJpaDstu2Test;
import ca.uhn.fhir.jpa.interceptor.RestHookSubscriptionDstu2Interceptor;
import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider;
import ca.uhn.fhir.model.api.Bundle;
import ca.uhn.fhir.model.api.BundleEntry;
import ca.uhn.fhir.model.dstu2.resource.Patient;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator;
import ca.uhn.fhir.parser.LenientErrorHandler;
import ca.uhn.fhir.rest.client.IGenericClient;
import ca.uhn.fhir.rest.client.ServerValidationModeEnum;
import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.util.TestUtil;
@ -45,10 +42,11 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test {
protected static CloseableHttpClient ourHttpClient;
protected static int ourPort;
protected static RestfulServer ourRestServer;
private static Server ourServer;
protected static Server ourServer;
protected static String ourServerBase;
private static GenericWebApplicationContext ourWebApplicationContext;
protected static GenericWebApplicationContext ourWebApplicationContext;
protected static RestHookSubscriptionDstu2Interceptor ourRestHookSubscriptionInterceptor;
protected static DatabaseBackedPagingProvider ourPagingProvider;
public BaseResourceProviderDstu2Test() {
super();
@ -83,7 +81,8 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test {
confProvider.setImplementationDescription("THIS IS THE DESC");
ourRestServer.setServerConformanceProvider(confProvider);
ourRestServer.setPagingProvider(new FifoMemoryPagingProvider(10));
ourPagingProvider = myAppCtx.getBean(DatabaseBackedPagingProvider.class);
ourRestServer.setPagingProvider(ourPagingProvider);
Server server = new Server(ourPort);
@ -126,6 +125,8 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test {
ourServer = server;
}
ourRestServer.setPagingProvider(ourPagingProvider);
}
protected List<IdDt> toIdListUnqualifiedVersionless(Bundle found) {

View File

@ -184,6 +184,40 @@ public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test {
assertEquals(null, response.getLink("next"));
}
@Test
public void testPagingOverEverythingSetWithNoPagingProvider() throws InterruptedException {
ourRestServer.setPagingProvider(null);
Patient p = new Patient();
p.setActive(true);
String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue();
for (int i = 0; i < 20; i++) {
Observation o = new Observation();
o.getSubject().setReference(pid);
o.addIdentifier().setSystem("foo").setValue(Integer.toString(i));
myObservationDao.create(o);
}
mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50);
mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10);
mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true);
ca.uhn.fhir.model.dstu2.resource.Bundle response = ourClient
.operation()
.onInstance(new IdDt(pid))
.named("everything")
.withSearchParameter(Parameters.class, "_count", new NumberParam(10))
.returnResourceType(ca.uhn.fhir.model.dstu2.resource.Bundle.class)
.useHttpGet()
.execute();
assertEquals(21, response.getEntry().size());
assertEquals(21, response.getTotalElement().getValue().intValue());
assertEquals(null, response.getLink("next"));
}
private void checkParamMissing(String paramName) throws IOException, ClientProtocolException {

View File

@ -56,6 +56,7 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test {
protected static String ourServerBase;
private static GenericWebApplicationContext ourWebApplicationContext;
private TerminologyUploaderProviderDstu3 myTerminologyUploaderProvider;
protected static DatabaseBackedPagingProvider ourPagingProvider;
protected static RestHookSubscriptionDstu3Interceptor ourRestHookSubscriptionInterceptor;
protected static ISearchDao mySearchEntityDao;
protected static ISearchCoordinatorSvc mySearchCoordinatorSvc;
@ -95,7 +96,7 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test {
confProvider.setImplementationDescription("THIS IS THE DESC");
ourRestServer.setServerConformanceProvider(confProvider);
ourRestServer.setPagingProvider(myAppCtx.getBean(DatabaseBackedPagingProvider.class));
ourPagingProvider = myAppCtx.getBean(DatabaseBackedPagingProvider.class);
Server server = new Server(ourPort);
@ -162,6 +163,8 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test {
ourServer = server;
}
ourRestServer.setPagingProvider(ourPagingProvider);
}
protected boolean shouldLogClient() {

View File

@ -11,7 +11,6 @@ import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
@ -48,7 +47,6 @@ import org.hl7.fhir.dstu3.model.Observation.ObservationStatus;
import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType;
import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType;
import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus;
import org.hl7.fhir.instance.model.Encounter.EncounterState;
import org.hl7.fhir.instance.model.api.*;
import org.junit.*;
import org.springframework.test.util.AopTestUtils;
@ -58,7 +56,6 @@ import org.springframework.transaction.support.TransactionCallback;
import com.google.common.collect.Lists;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.jpa.dao.SearchBuilder;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl;
import ca.uhn.fhir.model.api.TemporalPrecisionEnum;
@ -2019,6 +2016,40 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
}
@Test
public void testPagingOverEverythingSetWithNoPagingProvider() throws InterruptedException {
ourRestServer.setPagingProvider(null);
Patient p = new Patient();
p.setActive(true);
String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue();
for (int i = 0; i < 20; i++) {
Observation o = new Observation();
o.getSubject().setReference(pid);
o.addIdentifier().setSystem("foo").setValue(Integer.toString(i));
myObservationDao.create(o);
}
mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50);
mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10);
mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true);
Bundle response = ourClient
.operation()
.onInstance(new IdType(pid))
.named("everything")
.withSearchParameter(Parameters.class, "_count", new NumberParam(10))
.returnResourceType(Bundle.class)
.useHttpGet()
.execute();
assertEquals(21, response.getEntry().size());
assertEquals(21, response.getTotalElement().getValue().intValue());
assertEquals(null, response.getLink("next"));
}
@Test
public void testPatchUsingJsonPatch() throws Exception {
String methodName = "testPatchUsingJsonPatch";

View File

@ -66,6 +66,15 @@
JPA search now uses hibernate ScrollableResults instead of plain JPA List. This
should improve performance over large search results.
</action>
<action type="add">
JPA servers with no paging provider configured, or with a paging provider other than
DatabaseBackedPagingProvider will load all results in a single pass and keep them
in memory. Using this setup is not a good idea unless you know for sure that you
will never have very large queries since it means that all results will be loaded into
memory, but there are valid reasons to need this and it will perform better than
paging to the database in that case. This fix also resolves a NullPointerException
when performing an $everything search. Thanks to Kamal Othman for reporting!
</action>
</release>
<release version="2.5" date="2017-06-08">
<action type="fix">