Skip to content

Commit 602ac81

Browse files
author
Eric Secules
committed
fixed #7448 by prefetching all the results if an accurate count is requested and setting the count based on number of iterations of the result iterator. Still run the dedicated count query if it's just a count of results that was requested, not the results themselves.
1 parent 3217893 commit 602ac81

File tree

1 file changed

+69
-58
lines changed
  • hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/tasks

1 file changed

+69
-58
lines changed

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/tasks/SearchTask.java

Lines changed: 69 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ private void doSaveSearch() {
571571
@SuppressWarnings({"rawtypes", "unchecked"})
572572
private void doSearch() {
573573
/*
574-
* If the user has explicitly requested a _count, perform a
574+
* If the user has explicitly only requested a _count, perform a
575575
*
576576
* SELECT COUNT(*) ....
577577
*
@@ -582,66 +582,75 @@ private void doSearch() {
582582
? isWantCount(myParams)
583583
: SearchParameterMapCalculator.isWantCount(myStorageSettings.getDefaultTotalMode());
584584

585-
if (myParamWantOnlyCount || myParamOrDefaultWantCount) {
586-
doCountOnlyQuery(myParamWantOnlyCount);
587-
if (myParamWantOnlyCount) {
588-
return;
589-
}
585+
if (myParamWantOnlyCount) {
586+
doCountOnlyQuery();
587+
ourLog.trace("Done count");
588+
return;
590589
}
591590

592-
ourLog.trace("Done count");
593591
ISearchBuilder sb = newSearchBuilder();
594592

595593
/*
596-
* Figure out how many results we're actually going to fetch from the
597-
* database in this pass. This calculation takes into consideration the
598-
* "pre-fetch thresholds" specified in StorageSettings#getSearchPreFetchThresholds()
599-
* as well as the value of the _count parameter.
594+
* If _total=accurate is requested, we need to fetch all results to provide
595+
* a consistent count. This avoids race conditions where a separate COUNT(*)
596+
* query could return a different value than the actual result set if records
597+
* are being added during the search.
600598
*/
601-
int currentlyLoaded = defaultIfNull(mySearch.getNumFound(), 0);
602-
int minWanted = 0;
603-
604-
// if no count is provided,
605-
// we only use the values in SearchPreFetchThresholds
606-
// but if there is a count...
607-
if (myParams.getCount() != null) {
608-
minWanted = Math.min(myParams.getCount(), myPagingProvider.getMaximumPageSize());
609-
minWanted += currentlyLoaded;
610-
}
611-
612-
// iterate through the search thresholds
613-
for (Iterator<Integer> iter =
614-
myStorageSettings.getSearchPreFetchThresholds().iterator();
615-
iter.hasNext(); ) {
616-
int next = iter.next();
617-
if (next != -1 && next <= currentlyLoaded) {
618-
continue;
599+
if (myParamOrDefaultWantCount) {
600+
sb.setMaxResultsToFetch(null);
601+
sb.setDeduplicateInDatabase(true);
602+
} else {
603+
/*
604+
* Figure out how many results we're actually going to fetch from the
605+
* database in this pass. This calculation takes into consideration the
606+
* "pre-fetch thresholds" specified in StorageSettings#getSearchPreFetchThresholds()
607+
* as well as the value of the _count parameter.
608+
*/
609+
int currentlyLoaded = defaultIfNull(mySearch.getNumFound(), 0);
610+
int minWanted = 0;
611+
612+
// if no count is provided,
613+
// we only use the values in SearchPreFetchThresholds
614+
// but if there is a count...
615+
if (myParams.getCount() != null) {
616+
minWanted = Math.min(myParams.getCount(), myPagingProvider.getMaximumPageSize());
617+
minWanted += currentlyLoaded;
619618
}
620619

621-
if (next == -1) {
622-
sb.setMaxResultsToFetch(null);
623-
/*
624-
* If we're past the last prefetch threshold then
625-
* we're potentially fetching unlimited amounts of data.
626-
* We'll move responsibility for deduplication to the database in this case
627-
* so that we don't run the risk of blowing out the memory
628-
* in the app server
629-
*/
630-
sb.setDeduplicateInDatabase(true);
631-
} else {
632-
// we want at least 1 more than our requested amount
633-
// so we know that there are other results
634-
// (in case we get the exact amount back)
635-
myMaxResultsToFetch = Math.max(next, minWanted) + 1;
636-
sb.setMaxResultsToFetch(myMaxResultsToFetch);
637-
}
620+
// iterate through the search thresholds
621+
for (Iterator<Integer> iter =
622+
myStorageSettings.getSearchPreFetchThresholds().iterator();
623+
iter.hasNext(); ) {
624+
int next = iter.next();
625+
if (next != -1 && next <= currentlyLoaded) {
626+
continue;
627+
}
638628

639-
if (iter.hasNext()) {
640-
myAdditionalPrefetchThresholdsRemaining = true;
641-
}
629+
if (next == -1) {
630+
sb.setMaxResultsToFetch(null);
631+
/*
632+
* If we're past the last prefetch threshold then
633+
* we're potentially fetching unlimited amounts of data.
634+
* We'll move responsibility for deduplication to the database in this case
635+
* so that we don't run the risk of blowing out the memory
636+
* in the app server
637+
*/
638+
sb.setDeduplicateInDatabase(true);
639+
} else {
640+
// we want at least 1 more than our requested amount
641+
// so we know that there are other results
642+
// (in case we get the exact amount back)
643+
myMaxResultsToFetch = Math.max(next, minWanted) + 1;
644+
sb.setMaxResultsToFetch(myMaxResultsToFetch);
645+
}
642646

643-
// If we get here's we've found an appropriate threshold
644-
break;
647+
if (iter.hasNext()) {
648+
myAdditionalPrefetchThresholdsRemaining = true;
649+
}
650+
651+
// If we get here's we've found an appropriate threshold
652+
break;
653+
}
645654
}
646655

647656
/*
@@ -683,11 +692,12 @@ private void doSearch() {
683692
* matching the search off of the disk and into memory. After
684693
* every X results, we commit to the HFJ_SEARCH table.
685694
*/
686-
int syncSize = mySyncSize;
695+
int totalCount = 0;
687696
while (resultIterator.hasNext()) {
688697
myUnsyncedPids.add(resultIterator.next());
698+
totalCount++;
689699

690-
boolean shouldSync = myUnsyncedPids.size() >= syncSize;
700+
boolean shouldSync = myUnsyncedPids.size() >= mySyncSize;
691701

692702
if (myStorageSettings.getCountSearchResultsUpTo() != null
693703
&& myStorageSettings.getCountSearchResultsUpTo() > 0
@@ -716,6 +726,10 @@ private void doSearch() {
716726

717727
saveUnsynced(resultIterator);
718728

729+
if (myParamOrDefaultWantCount) {
730+
mySearch.setTotalCount(totalCount);
731+
}
732+
719733
} catch (IOException e) {
720734
ourLog.error("IO failure during database access", e);
721735
throw new InternalErrorException(Msg.code(1166) + e);
@@ -724,9 +738,8 @@ private void doSearch() {
724738

725739
/**
726740
* Does the query but only for the count.
727-
* @param theParamWantOnlyCount - if count query is wanted only
728741
*/
729-
private void doCountOnlyQuery(boolean theParamWantOnlyCount) {
742+
private void doCountOnlyQuery() {
730743
ourLog.trace("Performing count");
731744
@SuppressWarnings("rawtypes")
732745
ISearchBuilder sb = newSearchBuilder();
@@ -749,9 +762,7 @@ private void doCountOnlyQuery(boolean theParamWantOnlyCount) {
749762
.withRequestPartitionId(myRequestPartitionId)
750763
.execute(() -> {
751764
mySearch.setTotalCount(count.intValue());
752-
if (theParamWantOnlyCount) {
753-
mySearch.setStatus(SearchStatusEnum.FINISHED);
754-
}
765+
mySearch.setStatus(SearchStatusEnum.FINISHED);
755766
doSaveSearch();
756767
});
757768
}

0 commit comments

Comments
 (0)