Skip to content

Cache Implementation Updates #1371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrisknoll merged 3 commits into from
Dec 4, 2019
Merged

Cache Implementation Updates #1371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d240d9b
Cache Implementation Updates.
chrisknoll Nov 23, 2019
c454d3b
Remove resultIdentifer and use designHash
anthonysena Dec 4, 2019
6ce38a7
Fix migrations.
chrisknoll Dec 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ sandbox/
**/resources/config/application*-dev-*.properties
**/resources/config/application*-dev-*.yaml
**/resources/config/application*-dev-*.yml
/.vscode
16 changes: 11 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@
</properties>
<build>
<finalName>WebAPI</finalName>
<sourceDirectory>${basedir}/src/main/java</sourceDirectory>
<testSourceDirectory>${basedir}/src/test/java</testSourceDirectory>
<outputDirectory>${basedir}/target/classes</outputDirectory>
<testOutputDirectory>${basedir}/target/test-classes</testOutputDirectory>
<resources>
<resource>
<directory>src/main/resources</directory>
Expand All @@ -212,14 +216,16 @@
<exclude>log4j.xml</exclude>
</excludes>
</resource>
<resource>
</resources>
<testResources>
<testResource>
<directory>src/test/resources</directory>
<filtering>true</filtering>
<includes>
<include>**/*.properties</include>
</includes>
</resource>
<resource>
</testResource>
<testResource>
<directory>src/test/resources</directory>
<filtering>false</filtering>
<includes>
Expand All @@ -228,8 +234,8 @@
<excludes>
<exclude>**/*.properties</exclude>
</excludes>
</resource>
</resources>
</testResource>
</testResources>
<plugins>
<plugin>
<groupId>org.springframework.boot</groupId>
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/ohdsi/webapi/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ interface Params {
String PACKAGE_FILE_NAME = "packageFilename";
String EXECUTABLE_FILE_NAME = "executableFilename";
String GENERATION_ID = "generation_id";
String DESIGN_HASH = "design_hash";
}

interface Variables {
Expand All @@ -81,6 +82,10 @@ interface Templates {
}

interface Tables {
String COHORT_GENERATIONS_TABLE = "cohort_generations";
String COHORT_CACHE = "cohort_cache";
String COHORT_INCLUSION_RESULT_CACHE = "cohort_inclusion_result_cache";
String COHORT_INCLUSION_STATS_CACHE = "cohort_inclusion_stats_cache";
String COHORT_SUMMARY_STATS_CACHE = "cohort_summary_stats_cache";
String COHORT_CENSOR_STATS_CACHE = "cohort_censor_stats";
}
}
11 changes: 6 additions & 5 deletions src/main/java/org/ohdsi/webapi/cohortcharacterization/GenerateLocalCohortTasklet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.ohdsi.webapi.cohortdefinition.CohortDefinition;
import org.ohdsi.webapi.cohortdefinition.CohortGenerationRequestBuilder;
import org.ohdsi.webapi.cohortdefinition.CohortGenerationUtils;
import org.ohdsi.webapi.generationcache.GenerationCacheHelper;
import org.ohdsi.webapi.service.CohortGenerationService;
import org.ohdsi.webapi.source.SourceService;
Expand All @@ -27,7 +28,6 @@
import java.util.stream.Collectors;

import static org.ohdsi.webapi.Constants.Params.*;
import static org.ohdsi.webapi.Constants.Tables.COHORT_GENERATIONS_TABLE;

public class GenerateLocalCohortTasklet implements StoppableTasklet {

Expand Down Expand Up @@ -83,11 +83,12 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon
String sessionId = SessionUtils.sessionId();
CohortGenerationRequestBuilder generationRequestBuilder = new CohortGenerationRequestBuilder(
sessionId,
targetSchema,
COHORT_GENERATIONS_TABLE,
GENERATION_ID,
false
targetSchema
);

int designHash = this.generationCacheHelper.computeHash(cd.getDetails().getExpression());
CohortGenerationUtils.insertInclusionRules(cd.getId(), cd.getExpression(), designHash, targetSchema, cancelableJdbcTemplate);

GenerationCacheHelper.CacheResult res = generationCacheHelper.computeCacheIfAbsent(cd, source, generationRequestBuilder, (resId, sqls) -> {
try {
generationCacheHelper.runCancelableCohortGeneration(cancelableJdbcTemplate, stmtCancel, sqls);
Expand Down
25 changes: 2 additions & 23 deletions src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,15 @@ public class CohortGenerationRequest {
private Source source;
private String sessionId;
private String targetSchema;
private String targetTable;
private String targetIdFieldName;
private Integer targetId;
private boolean generateStats;

public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, String targetSchema, String targetTable, String targetIdFieldName, Integer targetId, boolean generateStats) {
public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, String targetSchema) {
Comment thread
anthonysena marked this conversation as resolved.

this.expression = expression;
this.source = source;
this.sessionId = sessionId;
this.targetSchema = targetSchema;
this.targetTable = targetTable;
this.targetIdFieldName = targetIdFieldName;
this.targetId = targetId;
this.generateStats = generateStats;
this.targetSchema = targetSchema;
}

public CohortExpression getExpression() {
Expand All @@ -46,23 +40,8 @@ public String getTargetSchema() {
return targetSchema;
}

public String getTargetTable() {

return targetTable;
}

public String getTargetIdFieldName() {

return targetIdFieldName;
}

public Integer getTargetId() {

return targetId;
}

public boolean isGenerateStats() {

return generateStats;
}
}
10 changes: 2 additions & 8 deletions src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,12 @@ public class CohortGenerationRequestBuilder {
private Source source;
private String sessionId;
private String targetSchema;
private String targetTable;
private String targetIdFieldName;
private Integer targetId;
private boolean generateStats;

public CohortGenerationRequestBuilder(String sessionId, String targetSchema, String targetTable, String targetIdFieldName, boolean generateStats) {
public CohortGenerationRequestBuilder(String sessionId, String targetSchema) {

this.sessionId = sessionId;
this.targetSchema = targetSchema;
this.targetTable = targetTable;
this.targetIdFieldName = targetIdFieldName;
this.generateStats = generateStats;
}

public CohortGenerationRequestBuilder withSource(Source source) {
Expand All @@ -47,6 +41,6 @@ public CohortGenerationRequest build() {
throw new RuntimeException("CohortGenerationRequest should contain non-null expression, source and targetId");
}

return new CohortGenerationRequest(expression, source, sessionId, targetSchema, targetTable, targetIdFieldName, targetId, generateStats);
return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema);
}
}
76 changes: 48 additions & 28 deletions src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,89 @@
package org.ohdsi.webapi.cohortdefinition;

import com.google.common.base.MoreObjects;
import org.apache.commons.lang3.StringUtils;

import org.ohdsi.circe.cohortdefinition.CohortExpression;
import org.ohdsi.circe.cohortdefinition.CohortExpressionQueryBuilder;
import org.ohdsi.circe.cohortdefinition.InclusionRule;
import org.ohdsi.sql.SqlRender;
import org.ohdsi.sql.SqlSplit;
import org.ohdsi.sql.SqlTranslate;
import org.ohdsi.webapi.source.Source;
import org.ohdsi.webapi.util.SourceUtils;
import org.springframework.jdbc.core.JdbcTemplate;


import java.util.List;

import static org.ohdsi.webapi.Constants.Params.COHORT_ID_FIELD_NAME;
import static org.ohdsi.webapi.Constants.Params.TARGET_COHORT_ID;
import static org.ohdsi.webapi.Constants.Params.TARGET_DATABASE_SCHEMA;
import static org.ohdsi.webapi.Constants.Params.DESIGN_HASH;

import static org.ohdsi.webapi.Constants.Tables.COHORT_CACHE;
import static org.ohdsi.webapi.Constants.Tables.COHORT_CENSOR_STATS_CACHE;
import static org.ohdsi.webapi.Constants.Tables.COHORT_INCLUSION_RESULT_CACHE;
import static org.ohdsi.webapi.Constants.Tables.COHORT_INCLUSION_STATS_CACHE;
import static org.ohdsi.webapi.Constants.Tables.COHORT_SUMMARY_STATS_CACHE;

public class CohortGenerationUtils {

public static void insertInclusionRules(int cohortId, CohortExpression expression, int designHash, String targetSchema, JdbcTemplate jdbcTemplate) {

String deleteSql = String.format("DELETE FROM %s.cohort_inclusion WHERE cohort_definition_id = %d", targetSchema, cohortId);
jdbcTemplate.update(deleteSql);

String insertSql = StringUtils.replace("INSERT INTO @target_schema.cohort_inclusion (cohort_definition_id, design_hash, rule_sequence, name, description) VALUES (?,?,?,?,?)", "@target_schema", targetSchema);
List<InclusionRule> inclusionRules = expression.inclusionRules;
for (int i = 0; i< inclusionRules.size(); i++)
{
InclusionRule r = inclusionRules.get(i);
jdbcTemplate.update(insertSql, new Object[] { cohortId, designHash, i, r.name, r.description});
}
}

public static String[] buildGenerationSql(CohortGenerationRequest request) {

Source source = request.getSource();

String cdmSchema = SourceUtils.getCdmQualifier(source);
String vocabSchema = SourceUtils.getVocabQualifierOrNull(source);
String resultsSchema = SourceUtils.getResultsQualifier(source);

CohortExpressionQueryBuilder expressionQueryBuilder = new CohortExpressionQueryBuilder();
StringBuilder sqlBuilder = new StringBuilder();

CohortExpressionQueryBuilder.BuildExpressionQueryOptions options = new CohortExpressionQueryBuilder.BuildExpressionQueryOptions();
options.cohortIdFieldName = request.getTargetIdFieldName();
options.cohortIdFieldName = DESIGN_HASH;
options.cohortId = request.getTargetId();
options.cdmSchema = cdmSchema;
options.resultSchema = resultsSchema;
options.targetTable = request.getTargetSchema() + "." + request.getTargetTable();
options.vocabularySchema = vocabSchema;
options.generateStats = request.isGenerateStats();
options.generateStats = true; // always generate with stats
Comment thread
anthonysena marked this conversation as resolved.

final String oracleTempSchema = SourceUtils.getTempQualifier(source);

if (request.isGenerateStats()) {

String deleteSql = "DELETE FROM @target_database_schema.cohort_inclusion WHERE @cohort_id_field_name = @target_cohort_id;";
sqlBuilder.append(deleteSql).append("\n");

String insertSql = "INSERT INTO @target_database_schema.cohort_inclusion (@cohort_id_field_name, rule_sequence, name, description) SELECT @target_cohort_id as @cohort_id_field_name, @iteration as rule_sequence, CAST('@ruleName' as VARCHAR(255)) as name, CAST('@ruleDescription' as VARCHAR(1000)) as description;";

String[] names = new String[]{"iteration", "ruleName", "ruleDescription"};
List<InclusionRule> inclusionRules = request.getExpression().inclusionRules;
for (int i = 0; i < inclusionRules.size(); i++) {
InclusionRule r = inclusionRules.get(i);
String[] values = new String[]{((Integer) i).toString(), r.name, MoreObjects.firstNonNull(r.description, "")};

String inclusionRuleSql = SqlRender.renderSql(insertSql, names, values);

@chrisknoll chrisknoll Nov 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm mistaken, this code that renders the Inclusion Rule names directly into a SQL string is a source of sql injection. The update to do it via insertInclusionRules will use prepared statements.

sqlBuilder.append(inclusionRuleSql).append("\n");
}
}

String expressionSql = expressionQueryBuilder.buildExpressionQuery(request.getExpression(), options);
expressionSql = SqlRender.renderSql(

@chrisknoll chrisknoll Nov 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The below block is the way to override the result table names so that they write into a cache. This was the least invasive way I could think of to implement this functionality.

expressionSql,
new String[] {"target_cohort_table",
"results_database_schema.cohort_inclusion_result",
"results_database_schema.cohort_inclusion_stats",
"results_database_schema.cohort_summary_stats",
"results_database_schema.cohort_censor_stats",
"results_database_schema.cohort_inclusion"
},
new String[] {
COHORT_CACHE,
"@target_database_schema." + COHORT_INCLUSION_RESULT_CACHE,
"@target_database_schema." + COHORT_INCLUSION_STATS_CACHE,
"@target_database_schema." + COHORT_SUMMARY_STATS_CACHE,
"@target_database_schema." + COHORT_CENSOR_STATS_CACHE,
"@target_database_schema.cohort_inclusion"
}
);
sqlBuilder.append(expressionSql);

String renderedSql = SqlRender.renderSql(
sqlBuilder.toString(),
new String[] {TARGET_DATABASE_SCHEMA, COHORT_ID_FIELD_NAME, TARGET_COHORT_ID},
new String[]{request.getTargetSchema(), request.getTargetIdFieldName(), request.getTargetId().toString()}
new String[] {TARGET_DATABASE_SCHEMA},
new String[]{request.getTargetSchema()}
);
String translatedSql = SqlTranslate.translateSql(renderedSql, source.getSourceDialect(), request.getSessionId(), oracleTempSchema);
return SqlSplit.splitSql(translatedSql);
Expand Down
12 changes: 5 additions & 7 deletions src/main/java/org/ohdsi/webapi/cohortdefinition/GenerateCohortTasklet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import java.util.Map;

import static org.ohdsi.webapi.Constants.Params.*;
import static org.ohdsi.webapi.Constants.Tables.COHORT_GENERATIONS_TABLE;

/**
*
Expand Down Expand Up @@ -68,19 +67,18 @@ protected String[] prepareQueries(ChunkContext chunkContext, CancelableJdbcTempl
Integer sourceId = Integer.parseInt(jobParams.get(SOURCE_ID).toString());
String targetSchema = jobParams.get(TARGET_DATABASE_SCHEMA).toString();
String sessionId = jobParams.getOrDefault(SESSION_ID, SessionUtils.sessionId()).toString();
Boolean generateStats = Boolean.valueOf(jobParams.get(GENERATE_STATS).toString());

CohortDefinition cohortDefinition = cohortDefinitionRepository.findOneWithDetail(cohortDefinitionId);
Source source = sourceService.findBySourceId(sourceId);

CohortGenerationRequestBuilder generationRequestBuilder = new CohortGenerationRequestBuilder(
sessionId,
targetSchema,
COHORT_GENERATIONS_TABLE,
GENERATION_ID,
generateStats
targetSchema
);

int designHash = this.generationCacheHelper.computeHash(cohortDefinition.getDetails().getExpression());
CohortGenerationUtils.insertInclusionRules(cohortDefinitionId, cohortDefinition.getExpression(), designHash, targetSchema, jdbcTemplate);

GenerationCacheHelper.CacheResult res = generationCacheHelper.computeCacheIfAbsent(
cohortDefinition,
source,
Expand All @@ -90,7 +88,7 @@ protected String[] prepareQueries(ChunkContext chunkContext, CancelableJdbcTempl

String sql = SqlRender.renderSql(
copyGenerationIntoCohortTableSql,
new String[]{ RESULTS_DATABASE_SCHEMA, COHORT_DEFINITION_ID, GENERATION_ID },
new String[]{ RESULTS_DATABASE_SCHEMA, COHORT_DEFINITION_ID, DESIGN_HASH },
new String[]{ targetSchema, cohortDefinition.getId().toString(), res.getIdentifier().toString() }
);
sql = SqlTranslate.translateSql(sql, source.getSourceDialect());
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/ohdsi/webapi/generationcache/CleanupScheduler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public void removeOldCache() {
DateUtils.addDays(new Date(), -1 * invalidateAfterDays),
EntityGraphUtils.fromAttributePaths("source")
);
caches.forEach(gc -> generationCacheService.removeCache(gc.getType(), gc.getSource(), gc.getResultIdentifier()));
caches.forEach(gc -> generationCacheService.removeCache(gc.getType(), gc.getSource(), gc.getDesignHash()));
}
}
Loading