honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

remove distinct from search (#1712) 

* it works now

* oops

* add successful migration message

* add successful migration message

* Add fix for #1300

* Remove FIXME

* Test fix

* Test fix

Co-authored-by: James Agnew <jamesagnew@gmail.com>
This commit is contained in:
Ken Stevens 2020-02-15 12:45:59 -05:00 committed by GitHub
parent c5c1e3196b
commit da11d3d89e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 74 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IResultIterator.java
View File

@ -29,4 +29,6 @@ public interface IResultIterator extends Iterator<ResourcePersistentId>, Closeab
int getSkippedCount();
int getNonSkippedCount();
}

53
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java
View File

@ -254,7 +254,8 @@ public class SearchBuilder implements ISearchBuilder {
outerQuery.multiselect(myBuilder.countDistinct(myQueryRoot.getRoot()));
} else {
outerQuery.multiselect(myQueryRoot.get("myId").as(Long.class));
outerQuery.distinct(true);
// KHS This distinct call is causing performance issues in large installations
// outerQuery.distinct(true);
}
}
@ -965,6 +966,7 @@ public class SearchBuilder implements ISearchBuilder {
private SortSpec mySort;
private boolean myStillNeedToFetchIncludes;
private int mySkipCount = 0;
private int myNonSkipCount = 0;
private QueryIterator(SearchRuntimeDetails theSearchRuntimeDetails, RequestDetails theRequest) {
mySearchRuntimeDetails = theSearchRuntimeDetails;
@ -994,14 +996,7 @@ public class SearchBuilder implements ISearchBuilder {
myMaxResultsToFetch = myDaoConfig.getFetchSizeDefaultMaximum();
}
final TypedQuery<Long> query = createQuery(mySort, myMaxResultsToFetch, false, myRequest);
mySearchRuntimeDetails.setQueryStopwatch(new StopWatch());
Query<Long> hibernateQuery = (Query<Long>) query;
hibernateQuery.setFetchSize(myFetchSize);
ScrollableResults scroll = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY);
myResultsIterator = new ScrollableResultsIterator<>(scroll);
initializeIteratorQuery(myMaxResultsToFetch);
// If the query resulted in extra results being requested
if (myAlsoIncludePids != null) {
@ -1036,11 +1031,32 @@ public class SearchBuilder implements ISearchBuilder {
ResourcePersistentId next = new ResourcePersistentId(nextLong);
if (myPidSet.add(next)) {
myNext = next;
myNonSkipCount++;
break;
} else {
mySkipCount++;
}
}
if (!myResultsIterator.hasNext()) {
if (myMaxResultsToFetch != null && (mySkipCount + myNonSkipCount == myMaxResultsToFetch)) {
if (mySkipCount > 0 && myNonSkipCount == 0) {
myMaxResultsToFetch += 1000;
StorageProcessingMessage message = new StorageProcessingMessage();
String msg = "Pass completed with no matching results. This indicates an inefficient query! Retrying with new max count of " + myMaxResultsToFetch;
ourLog.warn(msg);
message.setMessage(msg);
HookParams params = new HookParams()
.add(RequestDetails.class, myRequest)
.addIfMatchesType(ServletRequestDetails.class, myRequest)
.add(StorageProcessingMessage.class, message);
JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, myRequest, Pointcut.JPA_PERFTRACE_WARNING, params);
initializeIteratorQuery(myMaxResultsToFetch);
}
}
}
}
}
@ -1100,6 +1116,20 @@ public class SearchBuilder implements ISearchBuilder {
}
private void initializeIteratorQuery(Integer theMaxResultsToFetch) {
final TypedQuery<Long> query = createQuery(mySort, theMaxResultsToFetch, false, myRequest);
mySearchRuntimeDetails.setQueryStopwatch(new StopWatch());
Query<Long> hibernateQuery = (Query<Long>) query;
hibernateQuery.setFetchSize(myFetchSize);
ScrollableResults scroll = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY);
myResultsIterator = new ScrollableResultsIterator<>(scroll);
mySkipCount = 0;
myNonSkipCount = 0;
}
@Override
public boolean hasNext() {
if (myNext == null) {
@ -1122,6 +1152,11 @@ public class SearchBuilder implements ISearchBuilder {
return mySkipCount;
}
@Override
public int getNonSkippedCount() {
return myNonSkipCount;
}
@Override
public void close() {
if (myResultsIterator != null) {

3
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java
View File

@ -774,10 +774,11 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
if (theResultIter.hasNext() == false) {
int skippedCount = theResultIter.getSkippedCount();
int nonSkippedCount = theResultIter.getNonSkippedCount();
int totalFetched = skippedCount + myCountSavedThisPass + myCountBlockedThisPass;
ourLog.trace("MaxToFetch[{}] SkippedCount[{}] CountSavedThisPass[{}] CountSavedThisTotal[{}] AdditionalPrefetchRemaining[{}]", myMaxResultsToFetch, skippedCount, myCountSavedThisPass, myCountSavedTotal, myAdditionalPrefetchThresholdsRemaining);
if (myMaxResultsToFetch != null && totalFetched < myMaxResultsToFetch) {
if (nonSkippedCount == 0 || (myMaxResultsToFetch != null && totalFetched < myMaxResultsToFetch)) {
ourLog.trace("Setting search status to FINISHED");
mySearch.setStatus(SearchStatusEnum.FINISHED);
mySearch.setTotalCount(myCountSavedTotal);

39
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java
View File

@ -182,33 +182,11 @@ public class SearchCoordinatorSvcImplTest {
when(mySearchBuilder.createQuery(any(), any(), any())).thenReturn(iter);
doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any());
when(mySearchResultCacheSvc.fetchResultPids(any(), anyInt(), anyInt())).thenAnswer(t -> {
List<ResourcePersistentId> returnedValues = iter.getReturnedValues();
int offset = t.getArgument(1, Integer.class);
int end = t.getArgument(2, Integer.class);
end = Math.min(end, returnedValues.size());
offset = Math.min(offset, returnedValues.size());
ourLog.info("findWithSearchUuid {} - {} out of {} values", offset, end, returnedValues.size());
return returnedValues.subList(offset, end);
});
when(mySearchResultCacheSvc.fetchAllResultPids(any())).thenReturn(allResults);
when(mySearchCacheSvc.tryToMarkSearchAsInProgress(any())).thenAnswer(t->{
Search search = t.getArgument(0, Search.class);
assertEquals(SearchStatusEnum.PASSCMPLET, search.getStatus());
search.setStatus(SearchStatusEnum.LOADING);
return Optional.of(search);
});
when(mySearchCacheSvc.save(any())).thenAnswer(t -> {
Search search = t.getArgument(0, Search.class);
myCurrentSearch = search;
return search;
});
when(mySearchCacheSvc.fetchByUuid(any())).thenAnswer(t -> Optional.ofNullable(myCurrentSearch));
IFhirResourceDao dao = myCallingDao;
when(myDaoRegistry.getResourceDao(any(String.class))).thenReturn(dao);
IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null);
assertNotNull(result.getUuid());
@ -602,6 +580,11 @@ public class SearchCoordinatorSvcImplTest {
return myWrap.getSkippedCount();
}
@Override
public int getNonSkippedCount() {
return myCount;
}
@Override
public void close() {
// nothing
@ -611,6 +594,7 @@ public class SearchCoordinatorSvcImplTest {
public static class ResultIterator extends BaseIterator<ResourcePersistentId> implements IResultIterator {
private final Iterator<ResourcePersistentId> myWrap;
private int myCount;
ResultIterator(Iterator<ResourcePersistentId> theWrap) {
myWrap = theWrap;
@ -623,6 +607,7 @@ public class SearchCoordinatorSvcImplTest {
@Override
public ResourcePersistentId next() {
myCount++;
return myWrap.next();
}
@ -631,6 +616,11 @@ public class SearchCoordinatorSvcImplTest {
return 0;
}
@Override
public int getNonSkippedCount() {
return myCount;
}
@Override
public void close() {
// nothing
@ -697,6 +687,11 @@ public class SearchCoordinatorSvcImplTest {
}
}
@Override
public int getNonSkippedCount() {
return 0;
}
@Override
public void close() {
// nothing

2
hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrator.java
View File

@ -63,6 +63,8 @@ public class FlywayMigrator extends BaseMigrator {
if (isDryRun()) {
StringBuilder statementBuilder = buildExecutedStatementsString();
ourLog.info("SQL that would be executed:\n\n***********************************\n{}***********************************", statementBuilder);
} else {
ourLog.info("Schema migrated successfully.");
}
} catch (Exception e) {
throw e;

8
hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/TaskOnlyMigrator.java
View File

@ -51,7 +51,11 @@ public class TaskOnlyMigrator extends BaseMigrator {
next.setConnectionProperties(connectionProperties);
try {
ourLog.info("Executing task of type: {}", next.getClass().getSimpleName());
if (isDryRun()) {
ourLog.info("Dry run {} {}", next.getFlywayVersion(), next.getDescription());
} else {
ourLog.info("Executing {} {}", next.getFlywayVersion(), next.getDescription());
}
next.execute();
addExecutedStatements(next.getExecutedStatements());
} catch (SQLException e) {
@ -61,6 +65,8 @@ public class TaskOnlyMigrator extends BaseMigrator {
if (isDryRun()) {
StringBuilder statementBuilder = buildExecutedStatementsString();
ourLog.info("SQL that would be executed:\n\n***********************************\n{}***********************************", statementBuilder);
} else {
ourLog.info("Schema migrated successfully.");
}
}