Completed work on large ValueSet expansion support; ready for review.

This commit is contained in:
Diederik Muylwyk 2019-08-23 17:40:45 -04:00
parent 10958a8e4d
commit b41926307c
6 changed files with 56 additions and 33 deletions

View File

@ -134,3 +134,4 @@ ca.uhn.fhir.jpa.util.jsonpatch.JsonPatchUtils.failedToApplyPatch=Failed to apply
ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.notExpanded=The ValueSet is waiting to be picked up and pre-expanded by a scheduled task. ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.notExpanded=The ValueSet is waiting to be picked up and pre-expanded by a scheduled task.
ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.expansionInProgress=The ValueSet has been picked up by a scheduled task and pre-expansion is in progress. ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.expansionInProgress=The ValueSet has been picked up by a scheduled task and pre-expansion is in progress.
ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.expanded=The ValueSet has been picked up by a scheduled task and pre-expansion is complete. ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.expanded=The ValueSet has been picked up by a scheduled task and pre-expansion is complete.
ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum.failedToExpand=The ValueSet has been picked up by a scheduled task and pre-expansion has failed.

View File

@ -149,7 +149,7 @@ public class DaoConfig {
/** /**
* EXPERIMENTAL - Do not use in production! Do not change default of {@code false}! * EXPERIMENTAL - Do not use in production! Do not change default of {@code false}!
*/ */
private boolean myPreExpandValueSetsExperimental = true; // FIXME: DM 2019-08-19 - Return to false; private boolean myPreExpandValueSetsExperimental = false;
private boolean myFilterParameterEnabled = false; private boolean myFilterParameterEnabled = false;
private StoreMetaSourceInformation myStoreMetaSourceInformation = StoreMetaSourceInformation.SOURCE_URI_AND_REQUEST_ID; private StoreMetaSourceInformation myStoreMetaSourceInformation = StoreMetaSourceInformation.SOURCE_URI_AND_REQUEST_ID;
/** /**

View File

@ -32,7 +32,7 @@ public enum TermValueSetPreExpansionStatusEnum {
/** /**
* Sorting agnostic. * Sorting agnostic.
*/ */
// FIXME: add a unit test that verifies a message exists for each code
NOT_EXPANDED("notExpanded"), NOT_EXPANDED("notExpanded"),
EXPANSION_IN_PROGRESS("expansionInProgress"), EXPANSION_IN_PROGRESS("expansionInProgress"),
EXPANDED("expanded"), EXPANDED("expanded"),
@ -41,7 +41,7 @@ public enum TermValueSetPreExpansionStatusEnum {
private static Map<String, TermValueSetPreExpansionStatusEnum> ourValues; private static Map<String, TermValueSetPreExpansionStatusEnum> ourValues;
private String myCode; private String myCode;
private TermValueSetPreExpansionStatusEnum(String theCode) { TermValueSetPreExpansionStatusEnum(String theCode) {
myCode = theCode; myCode = theCode;
} }

View File

@ -351,7 +351,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
if (optionalExistingTermConceptMapById.isPresent()) { if (optionalExistingTermConceptMapById.isPresent()) {
TermConceptMap existingTermConceptMap = optionalExistingTermConceptMapById.get(); TermConceptMap existingTermConceptMap = optionalExistingTermConceptMapById.get();
ourLog.info("Deleting existing TermConceptMap {} and its children...", existingTermConceptMap.getId()); ourLog.info("Deleting existing TermConceptMap[{}] and its children...", existingTermConceptMap.getId());
for (TermConceptMapGroup group : existingTermConceptMap.getConceptMapGroups()) { for (TermConceptMapGroup group : existingTermConceptMap.getConceptMapGroups()) {
for (TermConceptMapGroupElement element : group.getConceptMapGroupElements()) { for (TermConceptMapGroupElement element : group.getConceptMapGroupElements()) {
@ -368,7 +368,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
} }
myConceptMapDao.deleteTermConceptMapById(existingTermConceptMap.getId()); myConceptMapDao.deleteTermConceptMapById(existingTermConceptMap.getId());
ourLog.info("Done deleting existing TermConceptMap {} and its children.", existingTermConceptMap.getId()); ourLog.info("Done deleting existing TermConceptMap[{}] and its children.", existingTermConceptMap.getId());
ourLog.info("Flushing..."); ourLog.info("Flushing...");
myConceptMapGroupElementTargetDao.flush(); myConceptMapGroupElementTargetDao.flush();
@ -392,11 +392,11 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
if (optionalExistingTermValueSetById.isPresent()) { if (optionalExistingTermValueSetById.isPresent()) {
TermValueSet existingTermValueSet = optionalExistingTermValueSetById.get(); TermValueSet existingTermValueSet = optionalExistingTermValueSetById.get();
ourLog.info("Deleting existing TermValueSet {} and its children...", existingTermValueSet.getId()); ourLog.info("Deleting existing TermValueSet[{}] and its children...", existingTermValueSet.getId());
myValueSetConceptDesignationDao.deleteByTermValueSetId(existingTermValueSet.getId()); myValueSetConceptDesignationDao.deleteByTermValueSetId(existingTermValueSet.getId());
myValueSetConceptDao.deleteByTermValueSetId(existingTermValueSet.getId()); myValueSetConceptDao.deleteByTermValueSetId(existingTermValueSet.getId());
myValueSetDao.deleteByTermValueSetId(existingTermValueSet.getId()); myValueSetDao.deleteByTermValueSetId(existingTermValueSet.getId());
ourLog.info("Done deleting existing TermValueSet {} and its children.", existingTermValueSet.getId()); ourLog.info("Done deleting existing TermValueSet[{}] and its children.", existingTermValueSet.getId());
ourLog.info("Flushing..."); ourLog.info("Flushing...");
myValueSetConceptDesignationDao.flush(); myValueSetConceptDesignationDao.flush();
@ -622,13 +622,16 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
private void expandValueSet(ValueSet theValueSetToExpand, IValueSetConceptAccumulator theValueSetCodeAccumulator, AtomicInteger theCodeCounter) { private void expandValueSet(ValueSet theValueSetToExpand, IValueSetConceptAccumulator theValueSetCodeAccumulator, AtomicInteger theCodeCounter) {
Set<String> addedCodes = new HashSet<>(); Set<String> addedCodes = new HashSet<>();
StopWatch sw = new StopWatch();
String valueSetInfo = getValueSetInfo(theValueSetToExpand);
ourLog.info("Working with {}", valueSetInfo);
// Handle includes // Handle includes
ourLog.debug("Handling includes"); ourLog.debug("Handling includes");
for (ValueSet.ConceptSetComponent include : theValueSetToExpand.getCompose().getInclude()) { for (ValueSet.ConceptSetComponent include : theValueSetToExpand.getCompose().getInclude()) {
ourLog.info("Working with " + identifyValueSetForLogging(theValueSetToExpand));
for (int i = 0; ; i++) { for (int i = 0; ; i++) {
int finalI = i; int finalI = i;
boolean shouldContinue = myTxTemplate.execute(t -> { Boolean shouldContinue = myTxTemplate.execute(t -> {
boolean add = true; boolean add = true;
return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, include, add, theCodeCounter, finalI); return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, include, add, theCodeCounter, finalI);
}); });
@ -641,10 +644,9 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
// Handle excludes // Handle excludes
ourLog.debug("Handling excludes"); ourLog.debug("Handling excludes");
for (ValueSet.ConceptSetComponent exclude : theValueSetToExpand.getCompose().getExclude()) { for (ValueSet.ConceptSetComponent exclude : theValueSetToExpand.getCompose().getExclude()) {
ourLog.info("Working with " + identifyValueSetForLogging(theValueSetToExpand));
for (int i = 0; ; i++) { for (int i = 0; ; i++) {
int finalI = i; int finalI = i;
boolean shouldContinue = myTxTemplate.execute(t -> { Boolean shouldContinue = myTxTemplate.execute(t -> {
boolean add = false; boolean add = false;
return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, exclude, add, theCodeCounter, finalI); return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, exclude, add, theCodeCounter, finalI);
}); });
@ -653,9 +655,11 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
} }
} }
} }
ourLog.info("Done working with {} in {}ms", valueSetInfo, sw.getMillis());
} }
private String identifyValueSetForLogging(ValueSet theValueSet) { private String getValueSetInfo(ValueSet theValueSet) {
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();
boolean isIdentified = false; boolean isIdentified = false;
sb sb
@ -703,16 +707,16 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
} }
/** /**
* @return Returns true if there are potentially more results to return * @return Returns true if there are potentially more results to process.
*/ */
private boolean expandValueSetHandleIncludeOrExclude(IValueSetConceptAccumulator theValueSetCodeAccumulator, Set<String> theAddedCodes, ValueSet.ConceptSetComponent theInclude, boolean theAdd, AtomicInteger theCodeCounter, int theQueryIndex) { private Boolean expandValueSetHandleIncludeOrExclude(IValueSetConceptAccumulator theValueSetCodeAccumulator, Set<String> theAddedCodes, ValueSet.ConceptSetComponent theInclude, boolean theAdd, AtomicInteger theCodeCounter, int theQueryIndex) {
String system = theInclude.getSystem(); String system = theInclude.getSystem();
boolean hasSystem = isNotBlank(system); boolean hasSystem = isNotBlank(system);
boolean hasValueSet = theInclude.getValueSet().size() > 0; boolean hasValueSet = theInclude.getValueSet().size() > 0;
if (hasSystem) { if (hasSystem) {
ourLog.info("Starting {} expansion around code system: {}", (theAdd ? "inclusion" : "exclusion"), system); ourLog.info("Starting {} expansion around CodeSystem: {}", (theAdd ? "inclusion" : "exclusion"), system);
TermCodeSystem cs = myCodeSystemDao.findByCodeSystemUri(system); TermCodeSystem cs = myCodeSystemDao.findByCodeSystemUri(system);
if (cs != null) { if (cs != null) {
@ -859,7 +863,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
AtomicInteger countForBatch = new AtomicInteger(0); AtomicInteger countForBatch = new AtomicInteger(0);
List resultList = jpaQuery.getResultList(); List resultList = jpaQuery.getResultList();
int resultsInBatch = jpaQuery.getResultSize(); int resultsInBatch = resultList.size();
int firstResult = jpaQuery.getFirstResult(); int firstResult = jpaQuery.getFirstResult();
for (Object next : resultList) { for (Object next : resultList) {
count.incrementAndGet(); count.incrementAndGet();
@ -910,7 +914,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
} else if (hasValueSet) { } else if (hasValueSet) {
for (CanonicalType nextValueSet : theInclude.getValueSet()) { for (CanonicalType nextValueSet : theInclude.getValueSet()) {
ourLog.info("Starting {} expansion around ValueSet URI: {}", (theAdd ? "inclusion" : "exclusion"), nextValueSet.getValueAsString()); ourLog.info("Starting {} expansion around ValueSet: {}", (theAdd ? "inclusion" : "exclusion"), nextValueSet.getValueAsString());
List<VersionIndependentConcept> expanded = expandValueSet(nextValueSet.getValueAsString()); List<VersionIndependentConcept> expanded = expandValueSet(nextValueSet.getValueAsString());
for (VersionIndependentConcept nextConcept : expanded) { for (VersionIndependentConcept nextConcept : expanded) {
@ -1750,8 +1754,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
ourLog.info("Done storing TermConceptMap."); ourLog.info("Done storing TermConceptMap.");
} }
// @Scheduled(fixedDelay = 600000) // 10 minutes. @Scheduled(fixedDelay = 600000) // 10 minutes.
@Scheduled(fixedDelay = 60000) // FIXME: DM 2019-08-19 - Remove this!
@Override @Override
public synchronized void preExpandValueSetToTerminologyTables() { public synchronized void preExpandValueSetToTerminologyTables() {
if (isNotSafeToPreExpandValueSets()) { if (isNotSafeToPreExpandValueSets()) {
@ -1761,10 +1764,9 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); TransactionTemplate txTemplate = new TransactionTemplate(myTxManager);
while (true) { while (true) {
TermValueSet valueSetToExpand = txTemplate.execute(t -> { TermValueSet valueSetToExpand = txTemplate.execute(t -> {
Optional<TermValueSet> optionalTermValueSet = getNextTermValueSetNotExpanded(); Optional<TermValueSet> optionalTermValueSet = getNextTermValueSetNotExpanded();
if (optionalTermValueSet.isPresent() == false) { if (!optionalTermValueSet.isPresent()) {
return null; return null;
} }
@ -1776,12 +1778,15 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
return; return;
} }
// Ok so we have a VS to expand // We have a ValueSet to pre-expand.
try { try {
ValueSet valueSet = txTemplate.execute(t -> getValueSetFromResourceTable(valueSetToExpand.getResource())); ValueSet valueSet = txTemplate.execute(t -> {
TermValueSet refreshedValueSetToExpand = myValueSetDao.findById(valueSetToExpand.getId()).get();
return getValueSetFromResourceTable(refreshedValueSetToExpand.getResource());
});
expandValueSet(valueSet, new ValueSetConceptAccumulator(valueSetToExpand, myValueSetConceptDao, myValueSetConceptDesignationDao)); expandValueSet(valueSet, new ValueSetConceptAccumulator(valueSetToExpand, myValueSetConceptDao, myValueSetConceptDesignationDao));
// We're done with this guy // We are done with this ValueSet.
txTemplate.execute(t -> { txTemplate.execute(t -> {
valueSetToExpand.setExpansionStatus(TermValueSetPreExpansionStatusEnum.EXPANDED); valueSetToExpand.setExpansionStatus(TermValueSetPreExpansionStatusEnum.EXPANDED);
myValueSetDao.saveAndFlush(valueSetToExpand); myValueSetDao.saveAndFlush(valueSetToExpand);
@ -1789,7 +1794,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
}); });
} catch (Exception e) { } catch (Exception e) {
ourLog.error("Failed to expand valueset: " + e.getMessage(), e); ourLog.error("Failed to pre-expand ValueSet: " + e.getMessage(), e);
txTemplate.execute(t -> { txTemplate.execute(t -> {
valueSetToExpand.setExpansionStatus(TermValueSetPreExpansionStatusEnum.FAILED_TO_EXPAND); valueSetToExpand.setExpansionStatus(TermValueSetPreExpansionStatusEnum.FAILED_TO_EXPAND);
myValueSetDao.saveAndFlush(valueSetToExpand); myValueSetDao.saveAndFlush(valueSetToExpand);
@ -1797,8 +1802,6 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
}); });
} }
} }
} }
private boolean isNotSafeToPreExpandValueSets() { private boolean isNotSafeToPreExpandValueSets() {

View File

@ -103,10 +103,9 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
} }
myValueSetConceptDao.save(concept); myValueSetConceptDao.save(concept);
// if (myConceptsSaved++ % 2 == 0) { // FIXME: DM 2019-08-23 - Reset to 250. if (myConceptsSaved++ % 250 == 0) { // TODO: DM 2019-08-23 - This message never appears in the log. Fix it!
ourLog.info("Have pre-expanded {} concepts in ValueSet[{}]", myConceptsSaved, myTermValueSet.getUrl()); ourLog.info("Have pre-expanded {} concepts in ValueSet[{}]", myConceptsSaved, myTermValueSet.getUrl());
// myValueSetConceptDao.flush(); }
// }
return concept; return concept;
} }
@ -128,9 +127,8 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
designation.setValue(theDesignation.getValue()); designation.setValue(theDesignation.getValue());
myValueSetConceptDesignationDao.save(designation); myValueSetConceptDesignationDao.save(designation);
if (myDesignationsSaved++ % 2 == 0) { // FIXME: DM 2019-08-23 - Reset to 250. if (myDesignationsSaved++ % 250 == 0) { // TODO: DM 2019-08-23 - This message never appears in the log. Fix it!
ourLog.info("Have pre-expanded {} designations in ValueSet[{}]", myDesignationsSaved, myTermValueSet.getUrl()); ourLog.info("Have pre-expanded {} designations for Concept[{}|{}] in ValueSet[{}]", myDesignationsSaved, theConcept.getSystem(), theConcept.getCode(), myTermValueSet.getUrl());
myValueSetConceptDesignationDao.flush();
} }
return designation; return designation;

View File

@ -0,0 +1,21 @@
package ca.uhn.fhir.jpa.entity;
import ca.uhn.fhir.i18n.HapiLocalizer;
import org.junit.Test;
import static org.junit.Assert.fail;
public class TermValueSetPreExpansionStatusEnumTest {
@Test
public void testHaveDescriptions() {
HapiLocalizer localizer = new HapiLocalizer();
for (TermValueSetPreExpansionStatusEnum next : TermValueSetPreExpansionStatusEnum.values()) {
String key = "ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum." + next.getCode();
String msg = localizer.getMessage(key);
if (msg.equals(HapiLocalizer.UNKNOWN_I18N_KEY_MESSAGE)) {
fail("No value for key: " + key);
}
}
}
}