Change to a minimal first pre-fetch to avoid latency on the async path. (#4174)

* Change to a minimal first pre-fetch to avoid latency on the async path.

The "async" path is now synchronous, and was causing latency for first page by loading 500 results.
We avoid pre-fetch on the first page, and only pre-fetch after users fetch the second page.

* Fix a nasty bug continuing a pre-fetched search.

* Update hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3664-shorten-first-prefetch.yaml

* Patch up tests that assumed 500 pre-fetch first step
This commit is contained in:
michaelabuckley 2022-10-21 16:45:30 -04:00 committed by GitHub
parent 2830792c4b
commit 929767a09b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 33 additions and 38 deletions

View File

@ -0,0 +1,4 @@
---
type: fix
issue: 3664
title: "Initial page loading has been optimized to reduce the number of prefetched resources. This should improve the speed of initial search queries in many cases."

View File

@ -92,7 +92,6 @@ import java.util.function.Consumer;
import java.util.stream.Collectors;
import static ca.uhn.fhir.jpa.util.QueryParameterUtils.DEFAULT_SYNC_SIZE;
import static ca.uhn.fhir.jpa.util.SearchParameterMapCalculator.isWantCount;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;

View File

@ -338,7 +338,8 @@ public class SearchBuilder implements ISearchBuilder {
private void init(SearchParameterMap theParams, String theSearchUuid, RequestPartitionId theRequestPartitionId) {
myCriteriaBuilder = myEntityManager.getCriteriaBuilder();
myParams = theParams;
// we mutate the params. Make a private copy.
myParams = theParams.clone();
mySearchUuid = theSearchUuid;
myRequestPartitionId = theRequestPartitionId;
}

View File

@ -581,7 +581,7 @@ public class SearchTask implements Callable<Void> {
int currentlyLoaded = defaultIfNull(mySearch.getNumFound(), 0);
int minWanted = 0;
if (myParams.getCount() != null) {
minWanted = myParams.getCount();
minWanted = myParams.getCount() + 1; // Always fetch one past this page, so we know if there is a next page.
minWanted = Math.min(minWanted, myPagingProvider.getMaximumPageSize());
minWanted += currentlyLoaded;
}

View File

@ -166,7 +166,7 @@ public class ResourceProviderR4ElasticTest extends BaseResourceProviderR4Test {
Coding blood_count = new Coding("http://loinc.org", "789-8", "Erythrocytes in Blood by Automated count for code: " + (index + 1));
createObservationWithCode(blood_count);
});
HttpGet countQuery = new HttpGet(BaseResourceProviderR4Test.ourServerBase + "/Observation?code=789-8&_count=5");
HttpGet countQuery = new HttpGet(BaseResourceProviderR4Test.ourServerBase + "/Observation?code=789-8&_count=5&_total=accurate");
myCaptureQueriesListener.clear();
try (CloseableHttpResponse response = BaseResourceProviderR4Test.ourHttpClient.execute(countQuery)) {
myCaptureQueriesListener.logSelectQueriesForCurrentThread();

View File

@ -58,6 +58,7 @@ import ca.uhn.fhir.model.primitive.UriDt;
import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
import ca.uhn.fhir.rest.api.SummaryEnum;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.gclient.NumberClientParam;
@ -325,6 +326,7 @@ public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test {
.forResource(Organization.class)
.where(Organization.NAME.matches().value("rpdstu2_testCountParam_01"))
.count(10)
.totalMode(SearchTotalModeEnum.ACCURATE)
.returnBundle(Bundle.class)
.execute();
assertEquals(100, found.getTotalElement().getValue().intValue());
@ -334,10 +336,9 @@ public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test {
.search()
.forResource(Organization.class)
.where(Organization.NAME.matches().value("rpdstu2_testCountParam_01"))
.count(999)
.count(50)
.returnBundle(Bundle.class)
.execute();
assertEquals(100, found.getTotalElement().getValue().intValue());
assertEquals(50, found.getEntry().size());
}

View File

@ -15,6 +15,7 @@ import ca.uhn.fhir.parser.StrictErrorHandler;
import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
import ca.uhn.fhir.rest.api.SummaryEnum;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.client.api.IClientInterceptor;
@ -649,12 +650,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
}
ourClient.transaction().withResources(resources).prettyPrint().encodedXml().execute();
Bundle found = ourClient.search().forResource(Organization.class).where(Organization.NAME.matches().value("rpdstu2_testCountParam_01")).count(10).returnBundle(Bundle.class).execute();
Bundle found = ourClient.search().forResource(Organization.class)
.where(Organization.NAME.matches().value("rpdstu2_testCountParam_01"))
.totalMode(SearchTotalModeEnum.ACCURATE)
.count(10).returnBundle(Bundle.class).execute();
assertEquals(100, found.getTotal());
assertEquals(10, found.getEntry().size());
found = ourClient.search().forResource(Organization.class).where(Organization.NAME.matches().value("rpdstu2_testCountParam_01")).count(999).returnBundle(Bundle.class).execute();
assertEquals(100, found.getTotal());
found = ourClient.search().forResource(Organization.class).where(Organization.NAME.matches().value("rpdstu2_testCountParam_01")).count(50).returnBundle(Bundle.class).execute();
assertEquals(50, found.getEntry().size());
}

View File

@ -1162,7 +1162,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test {
myCaptureQueriesListener.clear();
ourLog.info("** About to perform search");
IBundleProvider search = myPatientDao.search(new SearchParameterMap().setLoadSynchronous(false));
IBundleProvider search = myPatientDao.search(new SearchParameterMap().setCount(50).setLoadSynchronous(false));
ourLog.info("** About to retrieve resources");
search.getResources(0, 20);
ourLog.info("** Done retrieving resources");
@ -1171,13 +1171,13 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test {
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
assertEquals(4, myCaptureQueriesListener.countSelectQueries());
// Batches of 30 are written for each query - so 9 inserts total
assertEquals(221, myCaptureQueriesListener.logInsertQueries());
// first prefetch is 50+1
assertEquals(51, myCaptureQueriesListener.logInsertQueries());
assertEquals(1, myCaptureQueriesListener.countUpdateQueries());
assertEquals(0, myCaptureQueriesListener.countDeleteQueries());
assertEquals(4, myCaptureQueriesListener.countSelectQueriesForCurrentThread());
assertEquals(9, myCaptureQueriesListener.countInsertQueriesForCurrentThread());
assertEquals(3, myCaptureQueriesListener.countInsertQueriesForCurrentThread());
assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread());
assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread());

View File

@ -24,6 +24,7 @@ import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.client.apache.ResourceEntity;
@ -1001,7 +1002,10 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
}
private Bundle getAllResourcesOfType(String theResourceName) {
return myClient.search().forResource(theResourceName).cacheControl(new CacheControlDirective().setNoCache(true)).returnBundle(Bundle.class).execute();
return myClient.search().forResource(theResourceName)
.totalMode(SearchTotalModeEnum.ACCURATE)
.cacheControl(new CacheControlDirective().setNoCache(true))
.returnBundle(Bundle.class).execute();
}
@AfterAll

View File

@ -535,32 +535,11 @@ public abstract class BaseJpaTest extends BaseTest {
protected List<IIdType> toUnqualifiedVersionlessIds(IBundleProvider theProvider) {
List<IIdType> retVal = new ArrayList<>();
Integer size = theProvider.size();
StopWatch sw = new StopWatch();
while (size == null) {
int timeout = 20000;
if (sw.getMillis() > timeout) {
String message = "Waited over " + timeout + "ms for search " + theProvider.getUuid();
ourLog.info(message);
fail(message);
}
try {
Thread.sleep(100);
} catch (InterruptedException theE) {
//ignore
}
if (theProvider instanceof PersistedJpaBundleProvider) {
PersistedJpaBundleProvider provider = (PersistedJpaBundleProvider) theProvider;
provider.clearCachedDataForUnitTest();
}
size = theProvider.size();
}
Integer size = theProvider.size();
ourLog.info("Found {} results", size);
List<IBaseResource> resources = theProvider instanceof PersistedJpaBundleProvider ?
theProvider.getResources(0, size) :
theProvider.getResources(0, Integer.MAX_VALUE);
List<IBaseResource> resources = theProvider.getResources(0, Integer.MAX_VALUE/4);
for (IBaseResource next : resources) {
retVal.add(next.getIdElement().toUnqualifiedVersionless());
}

View File

@ -183,7 +183,11 @@ public class DaoConfig {
private int myExpungeThreadCount;
private Set<String> myBundleTypesAllowedForStorage;
private boolean myValidateSearchParameterExpressionsOnSave = true;
private List<Integer> mySearchPreFetchThresholds = Arrays.asList(500, 2000, -1);
// start with a tiny number so our first page always loads quickly.
// If they fetch the second page, fetch more.
// Use prime sizes to avoid empty next links.
private List<Integer> mySearchPreFetchThresholds = Arrays.asList(13, 503, 2003, -1);
private List<WarmCacheEntry> myWarmCacheEntries = new ArrayList<>();
private boolean myDisableHashBasedSearches;
private boolean myEnableInMemorySubscriptionMatching = true;