3664 search result indexing

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3664-reduce-database.yaml

@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 3664
+title: "Reduce database traffic during query caching by using composite PK instead of sequence."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java

@@ -1,6 +1,5 @@
 package ca.uhn.fhir.jpa.dao.data;
 
-import ca.uhn.fhir.jpa.entity.Search;
 import ca.uhn.fhir.jpa.entity.SearchResult;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
@@ -39,13 +38,13 @@ public interface ISearchResultDao extends JpaRepository<SearchResult, Long>, IHa
  @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearchPid = :search")
  List<Long> findWithSearchPidOrderIndependent(@Param("search") Long theSearchPid);
 
- @Query(value="SELECT r.myId FROM SearchResult r WHERE r.mySearchPid = :search")
- Slice<Long> findForSearch(Pageable thePage, @Param("search") Long theSearchPid);
-
- @Modifying
- @Query("DELETE FROM SearchResult s WHERE s.myId IN :ids")
- void deleteByIds(@Param("ids") List<Long> theContent);
-
  @Query("SELECT count(r) FROM SearchResult r WHERE r.mySearchPid = :search")
  int countForSearch(@Param("search") Long theSearchPid);
+
+ @Query(value="SELECT min(r.myOrder) FROM SearchResult r WHERE r.mySearchPid = :search")
+ Integer findFirstOrder(@Param("search") Long theSearchPid);
+
+ @Modifying
+ @Query("DELETE FROM SearchResult r WHERE r.mySearchPid = :search and r.myOrder >= :rangeStart and r.myOrder < :rangeStart + :maxCount")
+ int deleteForParentWithLimit(@Param("search") Long theSearchPid, @Param("rangeStart") int theStart, @Param("maxCount") int theCount);
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/SearchResult.java

@@ -26,25 +26,35 @@
 import javax.persistence.*;
 import java.io.Serializable;
 
+/**
+ * Table for individual results in a cached search.
+ *
+ * We use a composite PK to avoid the traffic of fetching the sequence since we insert 250 at a time.
+ */
 @Entity
 @Table(name = "HFJ_SEARCH_RESULT", uniqueConstraints = {
- @UniqueConstraint(name = "IDX_SEARCHRES_ORDER", columnNames = {"SEARCH_PID", "SEARCH_ORDER"})
+ @UniqueConstraint(name = "HFJ_SEARCH_RESULT_PKEY", columnNames = {"SEARCH_PID", "SEARCH_ORDER"})
 })
+@IdClass(SearchResult.PrimaryKey.class)
 public class SearchResult implements Serializable {
 
  private static final long serialVersionUID = 1L;
 
- @GeneratedValue(strategy = GenerationType.AUTO, generator = "SEQ_SEARCH_RES")
- @SequenceGenerator(name = "SEQ_SEARCH_RES", sequenceName = "SEQ_SEARCH_RES")
+
+ // TODO delete in 6.2
+ @Deprecated(forRemoval = true)
+ @Column(name = "PID", updatable = false, nullable = true)
+ private Long myPidUnused;
+
+ @Id
+ @Column(name = "SEARCH_PID", updatable = false, nullable = false)
+ private Long mySearchPid;
  @Id
- @Column(name = "PID")
- private Long myId;
- @Column(name = "SEARCH_ORDER", nullable = false, insertable = true, updatable = false)
+ @Column(name = "SEARCH_ORDER", updatable = false, nullable = false)
  private int myOrder;
- @Column(name = "RESOURCE_PID", insertable = true, updatable = false, nullable = false)
+ @Column(name = "RESOURCE_PID", updatable = false, nullable = false)
  private Long myResourcePid;
- @Column(name = "SEARCH_PID", insertable = true, updatable = false, nullable = false)
- private Long mySearchPid;
+
 
  /**
  * Constructor
@@ -90,13 +100,35 @@ public Long getResourcePid() {
  return myResourcePid;
  }
 
- public SearchResult setResourcePid(Long theResourcePid) {
+ public void setResourcePid(Long theResourcePid) {
  myResourcePid = theResourcePid;
- return this;
  }
 
  @Override
  public int hashCode() {
  return myResourcePid.hashCode();
  }
+
+ public static class PrimaryKey implements Serializable {
+ private static final long serialVersionUID = 1L;
+ private Long mySearchPid;
+ private int myOrder;
+
+ public Long getSearchPid() {
+ return mySearchPid;
+ }
+
+ public void setSearchPid(Long theSearchPid) {
+ mySearchPid = theSearchPid;
+ }
+
+ public int getOrder() {
+ return myOrder;
+ }
+
+ public void setOrder(int theOrder) {
+ myOrder = theOrder;
+ }
+ }
+
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java

@@ -84,6 +84,33 @@ public HapiFhirJpaMigrationTasks(Set<String> theFlags) {
  init560(); // 20211027 -
  init570(); // 20211102 -
  init600(); // 20211102 -
+ init610();
+ init620();
+ }
+
+ private void init620() {
+ // Changes for the next release
+ // We stage these over two releases to avoid breaking clustered servers during upgrades.
+// Builder version = forVersion(VersionEnum.V6_2_0);
+
+ // followup for 20220609 changes
+// version.onTable("HFJ_SEARCH_RESULT")
+// .dropColumn("202208xx.1", "PID");
+
+ }
+
+ private void init610() {
+ Builder version = forVersion(VersionEnum.V6_1_0);
+
+ // use composite PK to avoid sequence traffic in hot path during query.
+ Builder.BuilderWithTableName searchResultTable = version.onTable("HFJ_SEARCH_RESULT");
+ version.executeRawSql("20220609.1", "ALTER TABLE HFJ_SEARCH_RESULT DROP PRIMARY KEY");
+ searchResultTable.modifyColumn("20220609.2", "PID")
+ .nullable().withType(ColumnTypeEnum.LONG);
+ version.executeRawSql("20220609.3", "ALTER TABLE HFJ_SEARCH_RESULT ADD PRIMARY KEY (SEARCH_PID, SEARCH_ORDER)");
+ searchResultTable
+ .dropIndex("20220609.4","IDX_SEARCHRES_ORDER");
+ version.dropIdGenerator("20220609.5", "SEQ_SEARCH_RES");
  }
 
  private void init600() {

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

@@ -27,10 +27,8 @@
 import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
 import ca.uhn.fhir.jpa.entity.Search;
-import ca.uhn.fhir.jpa.entity.SearchInclude;
 import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.Lists;
 import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.time.DateUtils;
 import org.hl7.fhir.dstu3.model.InstantType;
@@ -47,7 +45,6 @@
 import java.time.Instant;
 import java.util.Collection;
 import java.util.Date;
-import java.util.List;
 import java.util.Optional;
 
 public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
@@ -56,12 +53,10 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc {
  * DELETE FROM foo WHERE params IN (term,term,term...)
  * type query and this can fail if we have 1000s of params
  */
- 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;
@@ -93,9 +88,7 @@ public Search save(Search theSearch) {
  Search newSearch;
  if (theSearch.getId() == null) {
  newSearch = mySearchDao.save(theSearch);
- for (SearchInclude next : theSearch.getIncludes()) {
- mySearchIncludeDao.save(next);
- }
+ mySearchIncludeDao.saveAll(theSearch.getIncludes());
  } else {
  newSearch = mySearchDao.save(theSearch);
  }
@@ -208,11 +201,10 @@ public void pollForStaleSearchesAndDeleteThem() {
  }
 
  int count = toDelete.getContent().size();
- if (count > 0) {
- if (ourLog.isDebugEnabled() || "true".equalsIgnoreCase(System.getProperty("test"))) {
- Long total = tt.execute(t -> mySearchDao.count());
- ourLog.debug("Deleted {} searches, {} remaining", count, total);
- }
+ if (count > 0
+ && (ourLog.isDebugEnabled() || "true".equalsIgnoreCase(System.getProperty("test")))) {
+ Long total = tt.execute(t -> mySearchDao.count());
+ ourLog.debug("Deleted {} searches, {} remaining", count, total);
  }
  }
 
@@ -229,21 +221,21 @@ private void deleteSearch(final Long theSearchPid) {
  * eventually
  */
  int max = ourMaximumResultsToDeleteInOnePass;
- Slice<Long> resultPids = mySearchResultDao.findForSearch(PageRequest.of(0, max), searchToDelete.getId());
- if (resultPids.hasContent()) {
-  List<List<Long>> partitions = Lists.partition(resultPids.getContent(), ourMaximumResultsToDeleteInOneStatement);
- for (List<Long> nextPartition : partitions) {
-  mySearchResultDao.deleteByIds(nextPartition);
-  }
-
+ Integer minOrder = mySearchResultDao.findFirstOrder(searchToDelete.getId());
+ int deletedCount;
+ if (minOrder == null) {
+ // no rows, we're done
+ deletedCount = 0;
+ } else {
+ deletedCount = mySearchResultDao.deleteForParentWithLimit(searchToDelete.getId(), minOrder, max);
  }
 
  // Only delete if we don't have results left in this search
- if (resultPids.getNumberOfElements() < max) {
+ if (deletedCount  < max) {
  ourLog.debug("Deleting search {}/{} - Created[{}]", searchToDelete.getId(), searchToDelete.getUuid(), new InstantType(searchToDelete.getCreated()));
  mySearchDao.deleteByPid(searchToDelete.getId());
  } else {
- ourLog.debug("Purged {} search results for deleted search {}/{}", resultPids.getSize(), searchToDelete.getId(), searchToDelete.getUuid());
+ ourLog.debug("Purged {} search results for deleted search {}/{}", deletedCount, searchToDelete.getId(), searchToDelete.getUuid());
  }
  });
  }
@@ -258,11 +250,6 @@ public static void setMaximumResultsToDeleteInOnePassForUnitTest(int theMaximumR
  ourMaximumResultsToDeleteInOnePass = theMaximumResultsToDeleteInOnePass;
  }
 
- @VisibleForTesting
- public static void setMaximumResultsToDeleteForUnitTest(int theMaximumResultsToDelete) {
- ourMaximumResultsToDeleteInOneStatement = theMaximumResultsToDelete;
- }
-
  /**
  * This is for unit tests only, do not call otherwise
  */

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/SearchCoordinatorSvcImplTest.java

@@ -1,6 +1,5 @@
 package ca.uhn.fhir.jpa.dao.r4;
 
-import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc;
 import ca.uhn.fhir.jpa.dao.data.ISearchDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
@@ -20,9 +19,7 @@
 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.junit.jupiter.api.Assertions.assertEquals;
-import static org.hamcrest.MatcherAssert.assertThat;
 
 public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
 
@@ -35,9 +32,6 @@ public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
  @Autowired
  private ISearchIncludeDao mySearchIncludeDao;
 
- @Autowired
- private ISearchCoordinatorSvc mySearchCoordinator;
-
  @Autowired
  private ISearchCacheSvc myDatabaseCacheSvc;
 

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/StaleSearchDeletingSvcR4Test.java

@@ -51,7 +51,6 @@ public void after() throws Exception {
  super.after();
  DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc);
  staleSearchDeletingSvc.setCutoffSlackForUnitTest(DatabaseSearchCacheSvcImpl.SEARCH_CLEANUP_JOB_INTERVAL_MILLIS);
- DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT);
  DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS);
  }
 
@@ -111,7 +110,6 @@ public void testEverythingInstanceWithContentFilter() throws Exception {
 
  @Test
  public void testDeleteVeryLargeSearch() {
- DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(10);
  DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(10);
 
  runInTransaction(() -> {
@@ -153,7 +151,6 @@ public void testDeleteVeryLargeSearch() {
 
  @Test
  public void testDeleteVerySmallSearch() {
- DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(10);
 
  runInTransaction(() -> {
  Search search = new Search();
@@ -179,7 +176,6 @@ public void testDeleteVerySmallSearch() {
 
  @Test
  public void testDontDeleteSearchBeforeExpiry() {
- DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(10);
 
  runInTransaction(() -> {
  Search search = new Search();