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

Improve efficiency in search deleting (#1759) 

* Improve efficiency in search deleting

* Changelog

* Address review comments
This commit is contained in:
James Agnew 2020-03-17 12:33:38 -04:00 committed by GitHub
parent 5867d62d62
commit 288abe4504
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 159 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1759-efficiency-in-search-deleting.yaml Normal file
View File

@ -0,0 +1,6 @@
---
type: fix
issue: 1759
title: When deleting searches from the query cache where a large number of searches with a large
number of results were present, the system would repeatedly mark the same rows as deletion
candidates. This put unneccessary pressure on the database and has been corrected.

9
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java
View File

@ -37,12 +37,18 @@ public interface ISearchDao extends JpaRepository<Search, Long> {
@Query("SELECT s FROM Search s LEFT OUTER JOIN FETCH s.myIncludes WHERE s.myUuid = :uuid")
Optional<Search> findByUuidAndFetchIncludes(@Param("uuid") String theUuid);
@Query("SELECT s.myId FROM Search s WHERE (s.myCreated < :cutoff) AND (s.myExpiryOrNull IS NULL OR s.myExpiryOrNull < :now)")
@Query("SELECT s.myId FROM Search s WHERE (s.myCreated < :cutoff) AND (s.myExpiryOrNull IS NULL OR s.myExpiryOrNull < :now) AND (s.myDeleted IS NULL OR s.myDeleted = FALSE)")
Slice<Long> findWhereCreatedBefore(@Param("cutoff") Date theCutoff, @Param("now") Date theNow, Pageable thePage);
@Query("SELECT s.myId FROM Search s WHERE s.myDeleted = TRUE")
Slice<Long> findDeleted(Pageable thePage);
@Query("SELECT s FROM Search s WHERE s.myResourceType = :type AND mySearchQueryStringHash = :hash AND (s.myCreated > :cutoff) AND s.myDeleted = false AND s.myStatus <> 'FAILED'")
Collection<Search> findWithCutoffOrExpiry(@Param("type") String theResourceType, @Param("hash") int theHashCode, @Param("cutoff") Date theCreatedCutoff);
@Query("SELECT COUNT(s) FROM Search s WHERE s.myDeleted = TRUE")
int countDeleted();
@Modifying
@Query("UPDATE Search s SET s.myDeleted = :deleted WHERE s.myId = :pid")
void updateDeleted(@Param("pid") Long thePid, @Param("deleted") boolean theDeleted);
@ -50,4 +56,5 @@ public interface ISearchDao extends JpaRepository<Search, Long> {
@Modifying
@Query("DELETE FROM Search s WHERE s.myId = :pid")
void deleteByPid(@Param("pid") Long theId);
}

29
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchCacheSvcImpl.java vendored
View File

@ -56,9 +56,11 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT = 500;
public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS = 20000;
public static final long SEARCH_CLEANUP_JOB_INTERVAL_MILLIS = 10 * DateUtils.MILLIS_PER_SECOND;
public static final int DEFAULT_MAX_DELETE_CANDIDATES_TO_FIND = 2000;
private static final Logger ourLog = LoggerFactory.getLogger(DatabaseSearchCacheSvcImpl.class);
private static int ourMaximumResultsToDeleteInOneStatement = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT;
private static int ourMaximumResultsToDeleteInOnePass = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS;
private static int ourMaximumSearchesToCheckForDeletionCandidacy = DEFAULT_MAX_DELETE_CANDIDATES_TO_FIND;
private static Long ourNowForUnitTests;
/*
* We give a bit of extra leeway just to avoid race conditions where a query result
@ -66,7 +68,6 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
* the result is to be deleted
*/
private long myCutoffSlack = SEARCH_CLEANUP_JOB_INTERVAL_MILLIS;
@Autowired
private ISearchDao mySearchDao;
@Autowired
@ -105,7 +106,6 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
return mySearchDao.findByUuidAndFetchIncludes(theUuid);
}
void setSearchDaoForUnitTest(ISearchDao theSearchDao) {
mySearchDao = theSearchDao;
}
@ -166,18 +166,27 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
ourLog.debug("Searching for searches which are before {}", cutoff);
TransactionTemplate tt = new TransactionTemplate(myTxManager);
final Slice<Long> toDelete = tt.execute(theStatus ->
mySearchDao.findWhereCreatedBefore(cutoff, new Date(), PageRequest.of(0, 2000))
);
assert toDelete != null;
for (final Long nextSearchToDelete : toDelete) {
// Mark searches as deleted if they should be
final Slice<Long> toMarkDeleted = tt.execute(theStatus ->
mySearchDao.findWhereCreatedBefore(cutoff, new Date(), PageRequest.of(0, ourMaximumSearchesToCheckForDeletionCandidacy))
);
assert toMarkDeleted != null;
for (final Long nextSearchToDelete : toMarkDeleted) {
ourLog.debug("Deleting search with PID {}", nextSearchToDelete);
tt.execute(t -> {
mySearchDao.updateDeleted(nextSearchToDelete, true);
return null;
});
}
// Delete searches that are marked as deleted
final Slice<Long> toDelete = tt.execute(theStatus ->
mySearchDao.findDeleted(PageRequest.of(0, ourMaximumSearchesToCheckForDeletionCandidacy))
);
assert toDelete != null;
for (final Long nextSearchToDelete : toDelete) {
ourLog.debug("Deleting search with PID {}", nextSearchToDelete);
tt.execute(t -> {
deleteSearch(nextSearchToDelete);
return null;
@ -193,7 +202,6 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
}
}
private void deleteSearch(final Long theSearchPid) {
mySearchDao.findById(theSearchPid).ifPresent(searchToDelete -> {
mySearchIncludeDao.deleteForSearch(searchToDelete.getId());
@ -226,6 +234,11 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
});
}
@VisibleForTesting
public static void setMaximumSearchesToCheckForDeletionCandidacyForUnitTest(int theMaximumSearchesToCheckForDeletionCandidacy) {
ourMaximumSearchesToCheckForDeletionCandidacy = theMaximumSearchesToCheckForDeletionCandidacy;
}
@VisibleForTesting
public static void setMaximumResultsToDeleteInOnePassForUnitTest(int theMaximumResultsToDeleteInOnePass) {
ourMaximumResultsToDeleteInOnePass = theMaximumResultsToDeleteInOnePass;

124
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchCoordinatorSvcImplTest.java Normal file
View File

@ -0,0 +1,124 @@
package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc;
import ca.uhn.fhir.jpa.dao.data.ISearchDao;
import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.entity.SearchResult;
import ca.uhn.fhir.jpa.entity.SearchTypeEnum;
import ca.uhn.fhir.jpa.model.cross.ResourcePersistentId;
import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc;
import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl;
import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.param.StringAndListParam;
import ca.uhn.fhir.rest.param.StringOrListParam;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.util.TestUtil;
import org.apache.commons.lang3.time.DateUtils;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Patient;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
import java.util.Date;
import java.util.List;
import java.util.UUID;
import static ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.DEFAULT_MAX_DELETE_CANDIDATES_TO_FIND;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
@AfterClass
public static void afterClassClearContext() {
TestUtil.clearAllStaticFieldsForUnitTest();
}
@Autowired
private ISearchDao mySearchDao;
@Autowired
private ISearchResultDao mySearchResultDao;
@Autowired
private ISearchCoordinatorSvc mySearchCoordinator;
@Autowired
private ISearchCacheSvc myDataaseCacheSvc;
@After
public void after() {
DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS);
DatabaseSearchCacheSvcImpl.setMaximumSearchesToCheckForDeletionCandidacyForUnitTest(DEFAULT_MAX_DELETE_CANDIDATES_TO_FIND);
}
@Test
public void testDeleteDontMarkPreviouslyMarkedSearchesAsDeleted() {
DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(5);
DatabaseSearchCacheSvcImpl.setMaximumSearchesToCheckForDeletionCandidacyForUnitTest(10);
// Create lots of searches
runInTransaction(()->{
for (int i = 0; i < 20; i++) {
Search search = new Search();
search.setCreated(DateUtils.addDays(new Date(), -1));
search.setLastUpdated(DateUtils.addDays(new Date(), -1), DateUtils.addDays(new Date(), -1));
search.setUuid(UUID.randomUUID().toString());
search.setSearchType(SearchTypeEnum.SEARCH);
search.setStatus(SearchStatusEnum.FINISHED);
mySearchDao.save(search);
// Add a bunch of search results to a few (enough that it will take multiple passes)
if (i < 3) {
for (int j = 0; j < 10; j++) {
SearchResult sr = new SearchResult(search);
sr.setOrder(j);
sr.setResourcePid((long) j);
mySearchResultDao.save(sr);
}
}
}
});
runInTransaction(()->{
assertEquals(20, mySearchDao.count());
assertEquals(30, mySearchResultDao.count());
});
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
runInTransaction(()->{
// We should delete up to 10, but 3 don't get deleted since they have too many results to delete in one pass
assertEquals(13, mySearchDao.count());
assertEquals(3, mySearchDao.countDeleted());
// We delete a max of 5 results per search, so half are gone
assertEquals(15, mySearchResultDao.count());
});
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
runInTransaction(()->{
// Once again we attempt to delete 10, but the first 3 don't get deleted and still remain
// (total is 6 because 3 weren't deleted, and they blocked another 3 that might have been)
assertEquals(6, mySearchDao.count());
assertEquals(6, mySearchDao.countDeleted());
assertEquals(0, mySearchResultDao.count());
});
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
runInTransaction(()->{
assertEquals(0, mySearchDao.count());
assertEquals(0, mySearchDao.countDeleted());
assertEquals(0, mySearchResultDao.count());
});
}
}