From 56a71f92225f0a16068a3505ba7ca023268f4021 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 29 Sep 2017 09:28:47 -0400 Subject: [PATCH] Performance enhancements to JPA --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 45 +++-- .../BaseJpaSystemProviderDstu2Plus.java | 11 +- .../ca/uhn/fhir/jpa/config/TestR4Config.java | 5 +- .../dao/r4/FhirResourceDaoR4UpdateTest.java | 165 ++++++++++-------- src/changes/changes.xml | 7 + 5 files changed, 143 insertions(+), 90 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index a8d3770f938..0a89964524a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1659,63 +1659,63 @@ public abstract class BaseHapiFhirDao implements IDao { */ if (thePerformIndexing) { - for (ResourceIndexedSearchParamString next : existingStringParams) { + for (ResourceIndexedSearchParamString next : removeCommon(existingStringParams, stringParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamString next : stringParams) { + for (ResourceIndexedSearchParamString next : removeCommon(stringParams, existingStringParams)) { myEntityManager.persist(next); } - for (ResourceIndexedSearchParamToken next : existingTokenParams) { + for (ResourceIndexedSearchParamToken next : removeCommon(existingTokenParams, tokenParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamToken next : tokenParams) { + for (ResourceIndexedSearchParamToken next : removeCommon(tokenParams, existingTokenParams)) { myEntityManager.persist(next); } - for (ResourceIndexedSearchParamNumber next : existingNumberParams) { + for (ResourceIndexedSearchParamNumber next : removeCommon(existingNumberParams, numberParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamNumber next : numberParams) { + for (ResourceIndexedSearchParamNumber next : removeCommon(numberParams, existingNumberParams)) { myEntityManager.persist(next); } - for (ResourceIndexedSearchParamQuantity next : existingQuantityParams) { + for (ResourceIndexedSearchParamQuantity next : removeCommon(existingQuantityParams, quantityParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamQuantity next : quantityParams) { + for (ResourceIndexedSearchParamQuantity next : removeCommon(quantityParams, existingQuantityParams)) { myEntityManager.persist(next); } // Store date SP's - for (ResourceIndexedSearchParamDate next : existingDateParams) { + for (ResourceIndexedSearchParamDate next : removeCommon(existingDateParams, dateParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamDate next : dateParams) { + for (ResourceIndexedSearchParamDate next : removeCommon(dateParams, existingDateParams)) { myEntityManager.persist(next); } // Store URI SP's - for (ResourceIndexedSearchParamUri next : existingUriParams) { + for (ResourceIndexedSearchParamUri next : removeCommon(existingUriParams, uriParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamUri next : uriParams) { + for (ResourceIndexedSearchParamUri next : removeCommon(uriParams, existingUriParams)) { myEntityManager.persist(next); } // Store Coords SP's - for (ResourceIndexedSearchParamCoords next : existingCoordsParams) { + for (ResourceIndexedSearchParamCoords next : removeCommon(existingCoordsParams, coordsParams)) { myEntityManager.remove(next); } - for (ResourceIndexedSearchParamCoords next : coordsParams) { + for (ResourceIndexedSearchParamCoords next : removeCommon(coordsParams, existingCoordsParams)) { myEntityManager.persist(next); } // Store resource links - for (ResourceLink next : existingResourceLinks) { + for (ResourceLink next : removeCommon(existingResourceLinks, links)) { myEntityManager.remove(next); } - for (ResourceLink next : links) { + for (ResourceLink next : removeCommon(links, existingResourceLinks)) { myEntityManager.persist(next); } // make sure links are indexed @@ -1753,6 +1753,19 @@ public abstract class BaseHapiFhirDao implements IDao { return theEntity; } + private Collection removeCommon(Collection theInput, Collection theToRemove) { + assert theInput != theToRemove; + + if (theInput.isEmpty()) { + return theInput; + } + + ArrayList retVal = new ArrayList<>(theInput); + retVal.removeAll(theToRemove); + return retVal; + } + + protected ResourceTable updateEntity(IBaseResource theResource, ResourceTable entity, Date theDeletedTimestampOrNull, Date theUpdateTime) { return updateEntity(theResource, entity, theDeletedTimestampOrNull, true, true, theUpdateTime, false, true); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProviderDstu2Plus.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProviderDstu2Plus.java index e8ad50fbc6f..c9a26340966 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProviderDstu2Plus.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProviderDstu2Plus.java @@ -34,7 +34,7 @@ public abstract class BaseJpaSystemProviderDstu2Plus extends BaseJpaSyste @OperationParam(name="status") }) public IBaseResource markAllResourcesForReindexing() { - int count = getDao().markAllResourcesForReindexing(); + Integer count = getDao().markAllResourcesForReindexing(); IBaseParameters retVal = ParametersUtil.newInstance(getContext()); @@ -48,11 +48,16 @@ public abstract class BaseJpaSystemProviderDstu2Plus extends BaseJpaSyste @OperationParam(name="status") }) public IBaseResource performReindexingPass() { - int count = getDao().performReindexingPass(1000); + Integer count = getDao().performReindexingPass(1000); IBaseParameters retVal = ParametersUtil.newInstance(getContext()); - IPrimitiveType string = ParametersUtil.createString(getContext(), "Indexed " + count + " resources"); + IPrimitiveType string; + if (count == null) { + string = ParametersUtil.createString(getContext(), "Index pass already proceeding"); + } else { + string = ParametersUtil.createString(getContext(), "Indexed " + count + " resources"); + } ParametersUtil.addParameterToParameters(getContext(), retVal, string, "status"); return retVal; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index aba24d2be56..8794a67ebd8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.config; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; +import net.ttddyy.dsproxy.listener.ThreadQueryCountHolder; import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel; import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; import org.apache.commons.dbcp2.BasicDataSource; @@ -100,7 +101,7 @@ public class TestR4Config extends BaseJavaConfigR4 { .create(retVal) .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) - .countQuery() + .countQuery(new ThreadQueryCountHolder()) .build(); return dataSource; @@ -119,7 +120,7 @@ public class TestR4Config extends BaseJavaConfigR4 { private Properties jpaProperties() { Properties extraProperties = new Properties(); - extraProperties.put("hibernate.jdbc.batch_size", "50"); + extraProperties.put("hibernate.jdbc.batch_size", "1"); extraProperties.put("hibernate.format_sql", "false"); extraProperties.put("hibernate.show_sql", "false"); extraProperties.put("hibernate.hbm2ddl.auto", "update"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java index 95714e5e594..e767d8adca0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.verify; import java.util.*; +import net.ttddyy.dsproxy.QueryCountHolder; import org.hl7.fhir.r4.model.*; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -28,19 +29,10 @@ import ca.uhn.fhir.util.TestUtil; public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4UpdateTest.class); - @Test - public void testReCreateMatchResource() { - - CodeSystem codeSystem = new CodeSystem(); - codeSystem.setUrl("http://foo"); - IIdType id = myCodeSystemDao.create(codeSystem).getId().toUnqualifiedVersionless(); - - myCodeSystemDao.delete(id); - - codeSystem = new CodeSystem(); - codeSystem.setUrl("http://foo"); - myCodeSystemDao.update(codeSystem, "Patient?name=FAM").getId().toUnqualifiedVersionless(); - + @After + public void afterResetDao() { + myDaoConfig.setResourceMetaCountHardLimit(new DaoConfig().getResourceMetaCountHardLimit()); + myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields()); } @Test @@ -112,59 +104,6 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } - @After - public void afterResetDao() { - myDaoConfig.setResourceMetaCountHardLimit(new DaoConfig().getResourceMetaCountHardLimit()); - } - - @Test - public void testHardMetaCapIsEnforcedOnCreate() { - myDaoConfig.setResourceMetaCountHardLimit(3); - - IIdType id; - { - Patient patient = new Patient(); - patient.getMeta().addTag().setSystem("http://foo").setCode("1"); - patient.getMeta().addTag().setSystem("http://foo").setCode("2"); - patient.getMeta().addTag().setSystem("http://foo").setCode("3"); - patient.getMeta().addTag().setSystem("http://foo").setCode("4"); - patient.setActive(true); - try { - id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - fail(); - } catch (UnprocessableEntityException e) { - assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); - } - } - } - - @Test - public void testHardMetaCapIsEnforcedOnMetaAdd() { - myDaoConfig.setResourceMetaCountHardLimit(3); - - IIdType id; - { - Patient patient = new Patient(); - patient.setActive(true); - id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - - { - Meta meta = new Meta(); - meta.addTag().setSystem("http://foo").setCode("1"); - meta.addTag().setSystem("http://foo").setCode("2"); - meta.addTag().setSystem("http://foo").setCode("3"); - meta.addTag().setSystem("http://foo").setCode("4"); - try { - myPatientDao.metaAddOperation(id, meta, null); - fail(); - } catch (UnprocessableEntityException e) { - assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); - } - - } - } - @Test public void testDuplicateTagsOnAddTagsIgnored() { IIdType id; @@ -179,7 +118,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val2"); meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val3"); myPatientDao.metaAddOperation(id, meta, null); - + // Do a read { Patient patient = myPatientDao.read(id, mySrd); @@ -190,7 +129,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } } - + @Test public void testDuplicateTagsOnUpdateIgnored() { IIdType id; @@ -209,7 +148,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { patient.getMeta().addTag().setSystem("http://foo").setCode("bar").setDisplay("Val3"); myPatientDao.update(patient, mySrd).getId().toUnqualifiedVersionless(); } - + // Do a read on second version { Patient patient = myPatientDao.read(id, mySrd); @@ -243,6 +182,54 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } + @Test + public void testHardMetaCapIsEnforcedOnCreate() { + myDaoConfig.setResourceMetaCountHardLimit(3); + + IIdType id; + { + Patient patient = new Patient(); + patient.getMeta().addTag().setSystem("http://foo").setCode("1"); + patient.getMeta().addTag().setSystem("http://foo").setCode("2"); + patient.getMeta().addTag().setSystem("http://foo").setCode("3"); + patient.getMeta().addTag().setSystem("http://foo").setCode("4"); + patient.setActive(true); + try { + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + fail(); + } catch (UnprocessableEntityException e) { + assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); + } + } + } + + @Test + public void testHardMetaCapIsEnforcedOnMetaAdd() { + myDaoConfig.setResourceMetaCountHardLimit(3); + + IIdType id; + { + Patient patient = new Patient(); + patient.setActive(true); + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + { + Meta meta = new Meta(); + meta.addTag().setSystem("http://foo").setCode("1"); + meta.addTag().setSystem("http://foo").setCode("2"); + meta.addTag().setSystem("http://foo").setCode("3"); + meta.addTag().setSystem("http://foo").setCode("4"); + try { + myPatientDao.metaAddOperation(id, meta, null); + fail(); + } catch (UnprocessableEntityException e) { + assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); + } + + } + } + @Test public void testMultipleUpdatesWithNoChangesDoesNotResultInAnUpdateForDiscreteUpdates() { @@ -291,6 +278,21 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } + @Test + public void testReCreateMatchResource() { + + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl("http://foo"); + IIdType id = myCodeSystemDao.create(codeSystem).getId().toUnqualifiedVersionless(); + + myCodeSystemDao.delete(id); + + codeSystem = new CodeSystem(); + codeSystem.setUrl("http://foo"); + myCodeSystemDao.update(codeSystem, "Patient?name=FAM").getId().toUnqualifiedVersionless(); + + } + @Test public void testUpdateAndGetHistoryResource() throws InterruptedException { Patient patient = new Patient(); @@ -662,6 +664,31 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } + @Test + public void testUpdateReusesIndexes() { + myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED); + + QueryCountHolder.clear(); + + Patient pt = new Patient(); + pt.setActive(true); + pt.addName().setFamily("FAMILY1").addGiven("GIVEN1A").addGiven("GIVEN1B"); + IIdType id = myPatientDao.create(pt).getId().toUnqualifiedVersionless(); + + ourLog.info("Now have {} deleted", QueryCountHolder.getGrandTotal().getDelete()); + ourLog.info("Now have {} inserts", QueryCountHolder.getGrandTotal().getInsert()); + QueryCountHolder.clear(); + + pt.setId(id); + pt.getNameFirstRep().addGiven("GIVEN1C"); + myPatientDao.update(pt); + + ourLog.info("Now have {} deleted", QueryCountHolder.getGrandTotal().getDelete()); + ourLog.info("Now have {} inserts", QueryCountHolder.getGrandTotal().getInsert()); + assertEquals(0, QueryCountHolder.getGrandTotal().getDelete()); + assertEquals(4, QueryCountHolder.getGrandTotal().getInsert()); + } + @Test public void testUpdateUnknownNumericIdFails() { Patient p = new Patient(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6db2ea51114..99b8f3473c3 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -6,6 +6,13 @@ HAPI FHIR Changelog + + + A performance to the JPA server has been made which reduces the number + of changes to index tables when updating a resource with contents that + only make minor changes + + Support for FHIR R4 (current working draft) has been added]]>