Skip to content

Commit 88ee973

Browse files
spershinspershin
committed
Add 'use-test-checksum-in-determinism-analyzer' config property to Verifier.
1 parent 6f09c57 commit 88ee973

14 files changed

+187
-29
lines changed

‎presto-verifier/src/main/java/com/facebook/presto/verifier/event/DeterminismAnalysisRun.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,22 @@
2929
public class DeterminismAnalysisRun
3030
{
3131
private final String tableName;
32+
private final String clusterType;
3233
private final String queryId;
3334
private final List<String> setupQueryIds;
3435
private final List<String> teardownQueryIds;
3536
private final String checksumQueryId;
3637

3738
private DeterminismAnalysisRun(
3839
Optional<String> tableName,
40+
Optional<String> clusterType,
3941
Optional<String> queryId,
4042
List<String> setupQueryIds,
4143
List<String> teardownQueryIds,
4244
Optional<String> checksumQueryId)
4345
{
4446
this.tableName = tableName.orElse(null);
47+
this.clusterType = clusterType.orElse(null);
4548
this.queryId = queryId.orElse(null);
4649
this.setupQueryIds = ImmutableList.copyOf(setupQueryIds);
4750
this.teardownQueryIds = ImmutableList.copyOf(teardownQueryIds);
@@ -54,6 +57,12 @@ public String getTableName()
5457
return tableName;
5558
}
5659

60+
@EventField
61+
public String getClusterType()
62+
{
63+
return clusterType;
64+
}
65+
5766
@EventField
5867
public String getQueryId()
5968
{
@@ -86,6 +95,7 @@ public static Builder builder()
8695
public static class Builder
8796
{
8897
private Optional<String> tableName = Optional.empty();
98+
private Optional<String> clusterType = Optional.empty();
8999
private Optional<String> queryId = Optional.empty();
90100
private ImmutableList.Builder<String> setupQueryIds = ImmutableList.builder();
91101
private ImmutableList.Builder<String> teardownQueryIds = ImmutableList.builder();
@@ -100,6 +110,13 @@ public Builder setTableName(String tableName)
100110
return this;
101111
}
102112

113+
public Builder setClusterType(String clusterType)
114+
{
115+
checkState(!this.clusterType.isPresent(), "clusterType is already set");
116+
this.clusterType = Optional.of(clusterType);
117+
return this;
118+
}
119+
103120
public Builder setQueryId(String queryId)
104121
{
105122
checkState(!this.queryId.isPresent(), "queryId is already set");
@@ -128,7 +145,7 @@ public Builder setChecksumQueryId(String checksumQueryId)
128145

129146
public DeterminismAnalysisRun build()
130147
{
131-
return new DeterminismAnalysisRun(tableName, queryId, setupQueryIds.build(), teardownQueryIds.build(), checksumQueryId);
148+
return new DeterminismAnalysisRun(tableName, clusterType, queryId, setupQueryIds.build(), teardownQueryIds.build(), checksumQueryId);
132149
}
133150
}
134151
}

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java

+14-2
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public abstract class AbstractVerification<B extends QueryBundle, R extends Matc
9696
protected final String runningMode;
9797
protected final boolean saveSnapshot;
9898
protected final boolean isExplain;
99+
protected final boolean isRunDeterminismAnalysisOnTest;
99100
private final boolean concurrentControlAndTest;
100101
protected final SnapshotQueryConsumer snapshotQueryConsumer;
101102
protected final Map<String, SnapshotQuery> snapshotQueries;
@@ -131,6 +132,7 @@ public AbstractVerification(
131132
this.runningMode = verifierConfig.getRunningMode();
132133
this.saveSnapshot = verifierConfig.isSaveSnapshot();
133134
this.isExplain = verifierConfig.isExplain();
135+
this.isRunDeterminismAnalysisOnTest = verifierConfig.isRunDeterminismAnalysisOnTest();
134136
}
135137

136138
protected abstract B getQueryRewrite(ClusterType clusterType);
@@ -143,7 +145,7 @@ protected abstract R verify(
143145
ChecksumQueryContext controlChecksumQueryContext,
144146
ChecksumQueryContext testChecksumQueryContext);
145147

146-
protected abstract DeterminismAnalysisDetails analyzeDeterminism(B control, R matchResult);
148+
protected abstract DeterminismAnalysisDetails analyzeDeterminism(B controlObject, B testObject, R matchResult);
147149

148150
protected abstract Optional<String> resolveFailure(
149151
Optional<B> control,
@@ -166,6 +168,16 @@ protected PrestoAction getHelperAction()
166168
return queryActions.getHelperAction();
167169
}
168170

171+
protected QueryAction getTestAction()
172+
{
173+
return queryActions.getTestAction();
174+
}
175+
176+
protected QueryAction getControlAction()
177+
{
178+
return queryActions.getControlAction();
179+
}
180+
169181
protected boolean isControlEnabled()
170182
{
171183
return !skipControl || saveSnapshot;
@@ -291,7 +303,7 @@ else if ((isControlEnabled()) && !skipChecksum) {
291303
return new VerificationResult(this, true, Optional.empty());
292304
}
293305
else if (matchResult.get().isMismatchPossiblyCausedByNonDeterminism()) {
294-
determinismAnalysisDetails = Optional.of(analyzeDeterminism(control.get(), matchResult.get()));
306+
determinismAnalysisDetails = Optional.of(analyzeDeterminism(control.get(), test.get(), matchResult.get()));
295307
}
296308
}
297309
}

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataMatchResult.java

+9
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public enum MatchType
5353
private final DataType dataType;
5454
private final MatchType matchType;
5555
private final Optional<ChecksumResult> controlChecksum;
56+
private final Optional<ChecksumResult> testChecksum;
5657
private final OptionalLong controlRowCount;
5758
private final OptionalLong testRowCount;
5859
private final List<ColumnMatchResult<?>> mismatchedColumns;
@@ -61,13 +62,15 @@ public DataMatchResult(
6162
DataType dataType,
6263
MatchType matchType,
6364
Optional<ChecksumResult> controlChecksum,
65+
Optional<ChecksumResult> testChecksum,
6466
OptionalLong controlRowCount,
6567
OptionalLong testRowCount,
6668
List<ColumnMatchResult<?>> mismatchedColumns)
6769
{
6870
this.dataType = requireNonNull(dataType, "data type is null");
6971
this.matchType = requireNonNull(matchType, "match type is null");
7072
this.controlChecksum = requireNonNull(controlChecksum, "controlChecksum is null");
73+
this.testChecksum = requireNonNull(testChecksum, "testChecksum is null");
7174
this.controlRowCount = requireNonNull(controlRowCount, "controlRowCount is null");
7275
this.testRowCount = requireNonNull(testRowCount, "testRowCount is null");
7376
this.mismatchedColumns = ImmutableList.copyOf(mismatchedColumns);
@@ -114,6 +117,12 @@ public ChecksumResult getControlChecksum()
114117
return controlChecksum.get();
115118
}
116119

120+
public ChecksumResult getTestChecksum()
121+
{
122+
checkState(testChecksum.isPresent(), "testChecksum is missing");
123+
return testChecksum.get();
124+
}
125+
117126
public List<ColumnMatchResult<?>> getMismatchedColumns()
118127
{
119128
return mismatchedColumns;

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerification.java

+28-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.facebook.presto.verifier.checksum.ChecksumResult;
2121
import com.facebook.presto.verifier.checksum.ChecksumValidator;
2222
import com.facebook.presto.verifier.event.DeterminismAnalysisDetails;
23+
import com.facebook.presto.verifier.event.DeterminismAnalysisRun;
2324
import com.facebook.presto.verifier.event.QueryInfo;
2425
import com.facebook.presto.verifier.prestoaction.QueryActions;
2526
import com.facebook.presto.verifier.prestoaction.SqlExceptionClassifier;
@@ -34,6 +35,8 @@
3435
import java.util.Map;
3536
import java.util.Optional;
3637
import java.util.OptionalLong;
38+
import java.util.stream.Collectors;
39+
import java.util.stream.Stream;
3740

3841
import static com.facebook.presto.verifier.framework.DataMatchResult.DataType.DATA;
3942
import static com.facebook.presto.verifier.framework.DataMatchResult.MatchType.MATCH;
@@ -132,6 +135,7 @@ public DataMatchResult verify(
132135
DATA,
133136
MATCH,
134137
Optional.empty(),
138+
Optional.empty(),
135139
OptionalLong.empty(),
136140
OptionalLong.empty(),
137141
ImmutableList.of());
@@ -146,7 +150,14 @@ else if (QUERY_BANK_MODE.equals(runningMode)) {
146150
controlChecksumResult = ChecksumResult.fromJson(snapshotJson);
147151
}
148152
else {
149-
return new DataMatchResult(DATA, SNAPSHOT_DOES_NOT_EXIST, Optional.empty(), OptionalLong.empty(), OptionalLong.empty(), Collections.emptyList());
153+
return new DataMatchResult(
154+
DATA,
155+
SNAPSHOT_DOES_NOT_EXIST,
156+
Optional.empty(),
157+
Optional.empty(),
158+
OptionalLong.empty(),
159+
OptionalLong.empty(),
160+
Collections.emptyList());
150161
}
151162
}
152163

@@ -159,9 +170,23 @@ else if (QUERY_BANK_MODE.equals(runningMode)) {
159170
}
160171

161172
@Override
162-
protected DeterminismAnalysisDetails analyzeDeterminism(QueryObjectBundle control, DataMatchResult matchResult)
173+
protected DeterminismAnalysisDetails analyzeDeterminism(QueryObjectBundle controlObject, QueryObjectBundle testObject, DataMatchResult matchResult)
163174
{
164-
return determinismAnalyzer.analyze(control, matchResult.getControlChecksum());
175+
if (isRunDeterminismAnalysisOnTest) {
176+
DeterminismAnalysisDetails analysis = determinismAnalyzer.analyze(getTestAction(), testObject, matchResult.getTestChecksum());
177+
if (!analysis.getDeterminismAnalysis().isNonDeterministic()) {
178+
return analysis;
179+
}
180+
// In case we rerun determinism analysis on control, we keep the test runs for stats.
181+
List<DeterminismAnalysisRun> runs = analysis.getRuns();
182+
analysis = determinismAnalyzer.analyze(getControlAction(), controlObject, matchResult.getControlChecksum());
183+
return new DeterminismAnalysisDetails(
184+
analysis.getDeterminismAnalysis(),
185+
Stream.concat(runs.stream(), analysis.getRuns().stream()).collect(Collectors.toList()),
186+
LimitQueryDeterminismAnalysis.valueOf(analysis.getLimitQueryAnalysis()),
187+
Optional.ofNullable(analysis.getLimitQueryAnalysisQueryId()));
188+
}
189+
return determinismAnalyzer.analyze(getControlAction(), controlObject, matchResult.getControlChecksum());
165190
}
166191

167192
@Override

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DataVerificationUtil.java

+2
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public static DataMatchResult match(
8585
dataType,
8686
SCHEMA_MISMATCH,
8787
Optional.empty(),
88+
Optional.empty(),
8889
OptionalLong.empty(),
8990
OptionalLong.empty(),
9091
ImmutableList.of());
@@ -107,6 +108,7 @@ public static DataMatchResult match(
107108
dataType,
108109
matchType,
109110
Optional.of(controlChecksum),
111+
Optional.of(testChecksum),
110112
controlRowCount,
111113
testRowCount,
112114
mismatchedColumns);

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DdlVerification.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ else if (QUERY_BANK_MODE.equals(runningMode)) {
152152
}
153153

154154
@Override
155-
protected DeterminismAnalysisDetails analyzeDeterminism(QueryObjectBundle control, DdlMatchResult matchResult)
155+
protected DeterminismAnalysisDetails analyzeDeterminism(QueryObjectBundle controlObject, QueryObjectBundle testObject, DdlMatchResult matchResult)
156156
{
157157
throw new UnsupportedOperationException("analyzeDeterminism is not supported for DdlVerification");
158158
}

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/DeterminismAnalyzer.java

+21-18
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.facebook.presto.verifier.event.DeterminismAnalysisDetails;
2626
import com.facebook.presto.verifier.event.DeterminismAnalysisRun;
2727
import com.facebook.presto.verifier.prestoaction.PrestoAction;
28+
import com.facebook.presto.verifier.prestoaction.QueryAction;
2829
import com.facebook.presto.verifier.rewrite.QueryRewriter;
2930
import com.google.common.annotations.VisibleForTesting;
3031
import com.google.common.collect.ImmutableSet;
@@ -36,7 +37,6 @@
3637
import java.util.Set;
3738
import java.util.concurrent.atomic.AtomicBoolean;
3839

39-
import static com.facebook.presto.verifier.framework.ClusterType.CONTROL;
4040
import static com.facebook.presto.verifier.framework.DataVerificationUtil.getColumns;
4141
import static com.facebook.presto.verifier.framework.DataVerificationUtil.match;
4242
import static com.facebook.presto.verifier.framework.DataVerificationUtil.teardownSafely;
@@ -60,7 +60,7 @@
6060
public class DeterminismAnalyzer
6161
{
6262
private final SourceQuery sourceQuery;
63-
private final PrestoAction prestoAction;
63+
private final PrestoAction helperAction;
6464
private final QueryRewriter queryRewriter;
6565
private final ChecksumValidator checksumValidator;
6666
private final TypeManager typeManager;
@@ -72,14 +72,14 @@ public class DeterminismAnalyzer
7272

7373
public DeterminismAnalyzer(
7474
SourceQuery sourceQuery,
75-
PrestoAction prestoAction,
75+
PrestoAction helperAction,
7676
QueryRewriter queryRewriter,
7777
ChecksumValidator checksumValidator,
7878
TypeManager typeManager,
7979
DeterminismAnalyzerConfig config)
8080
{
8181
this.sourceQuery = requireNonNull(sourceQuery, "sourceQuery is null");
82-
this.prestoAction = requireNonNull(prestoAction, "prestoAction is null");
82+
this.helperAction = requireNonNull(helperAction, "helperAction is null");
8383
this.queryRewriter = requireNonNull(queryRewriter, "queryRewriter is null");
8484
this.checksumValidator = requireNonNull(checksumValidator, "checksumValidator is null");
8585
this.typeManager = requireNonNull(typeManager, "typeManager is null");
@@ -90,26 +90,27 @@ public DeterminismAnalyzer(
9090
this.handleLimitQuery = config.isHandleLimitQuery();
9191
}
9292

93-
protected DeterminismAnalysisDetails analyze(QueryObjectBundle control, ChecksumResult controlChecksum)
93+
protected DeterminismAnalysisDetails analyze(QueryAction queryAction, QueryObjectBundle objectBundle, ChecksumResult referenceChecksum)
9494
{
95+
requireNonNull(queryAction, "queryAction is null");
9596
DeterminismAnalysisDetails.Builder determinismAnalysisDetails = DeterminismAnalysisDetails.builder();
96-
DeterminismAnalysis analysis = analyze(control, controlChecksum, determinismAnalysisDetails);
97+
DeterminismAnalysis analysis = analyze(queryAction, objectBundle, referenceChecksum, determinismAnalysisDetails);
9798
return determinismAnalysisDetails.build(analysis);
9899
}
99100

100-
private DeterminismAnalysis analyze(QueryObjectBundle control, ChecksumResult controlChecksum, DeterminismAnalysisDetails.Builder determinismAnalysisDetails)
101+
private DeterminismAnalysis analyze(QueryAction actionForQuery, QueryObjectBundle objectBundle, ChecksumResult referenceChecksum, DeterminismAnalysisDetails.Builder determinismAnalysisDetails)
101102
{
102103
// Handle mutable catalogs
103-
if (isNonDeterministicCatalogReferenced(control.getQuery())) {
104+
if (isNonDeterministicCatalogReferenced(objectBundle.getQuery())) {
104105
return NON_DETERMINISTIC_CATALOG;
105106
}
106107

107108
// Handle limit query
108109
LimitQueryDeterminismAnalysis limitQueryAnalysis = new LimitQueryDeterminismAnalyzer(
109-
prestoAction,
110+
helperAction,
110111
handleLimitQuery,
111-
control.getQuery(),
112-
controlChecksum.getRowCount(),
112+
objectBundle.getQuery(),
113+
referenceChecksum.getRowCount(),
113114
determinismAnalysisDetails).analyze();
114115

115116
switch (limitQueryAnalysis) {
@@ -127,29 +128,31 @@ private DeterminismAnalysis analyze(QueryObjectBundle control, ChecksumResult co
127128
}
128129

129130
// Rerun control query multiple times
130-
List<Column> columns = getColumns(prestoAction, typeManager, control.getObjectName());
131+
List<Column> columns = getColumns(helperAction, typeManager, objectBundle.getObjectName());
132+
ClusterType clusterType = objectBundle.getCluster();
131133
Map<QueryBundle, DeterminismAnalysisRun.Builder> queryRuns = new HashMap<>();
132134
try {
133135
for (int i = 0; i < maxAnalysisRuns; i++) {
134-
QueryObjectBundle queryBundle = queryRewriter.rewriteQuery(sourceQuery.getQuery(CONTROL), sourceQuery.getQueryConfiguration(CONTROL), CONTROL, false);
136+
QueryObjectBundle queryBundle = queryRewriter.rewriteQuery(sourceQuery.getQuery(clusterType), sourceQuery.getQueryConfiguration(clusterType), clusterType, false);
135137
DeterminismAnalysisRun.Builder run = determinismAnalysisDetails.addRun().setTableName(queryBundle.getObjectName().toString());
138+
run.setClusterType(clusterType.name());
136139
queryRuns.put(queryBundle, run);
137140

138141
// Rerun setup and main query
139142
queryBundle.getSetupQueries().forEach(query -> runAndConsume(
140-
() -> prestoAction.execute(query, DETERMINISM_ANALYSIS_SETUP),
143+
() -> helperAction.execute(query, DETERMINISM_ANALYSIS_SETUP),
141144
stats -> stats.getQueryStats().map(QueryStats::getQueryId).ifPresent(run::addSetupQueryId)));
142145
runAndConsume(
143-
() -> prestoAction.execute(queryBundle.getQuery(), DETERMINISM_ANALYSIS_MAIN),
146+
() -> actionForQuery.execute(queryBundle.getQuery(), DETERMINISM_ANALYSIS_MAIN),
144147
stats -> stats.getQueryStats().map(QueryStats::getQueryId).ifPresent(run::setQueryId));
145148

146149
// Run checksum query
147150
Query checksumQuery = checksumValidator.generateChecksumQuery(queryBundle.getObjectName(), columns, Optional.empty());
148151
ChecksumResult testChecksum = getOnlyElement(callAndConsume(
149-
() -> prestoAction.execute(checksumQuery, DETERMINISM_ANALYSIS_CHECKSUM, ChecksumResult::fromResultSet),
152+
() -> helperAction.execute(checksumQuery, DETERMINISM_ANALYSIS_CHECKSUM, ChecksumResult::fromResultSet),
150153
stats -> stats.getQueryStats().map(QueryStats::getQueryId).ifPresent(run::setChecksumQueryId)).getResults());
151154

152-
DeterminismAnalysis analysis = matchResultToDeterminism(match(DataMatchResult.DataType.DATA, checksumValidator, columns, columns, controlChecksum, testChecksum));
155+
DeterminismAnalysis analysis = matchResultToDeterminism(match(DataMatchResult.DataType.DATA, checksumValidator, columns, columns, referenceChecksum, testChecksum));
153156
if (analysis != DETERMINISTIC) {
154157
return analysis;
155158
}
@@ -163,7 +166,7 @@ private DeterminismAnalysis analyze(QueryObjectBundle control, ChecksumResult co
163166
finally {
164167
if (runTeardown) {
165168
queryRuns.forEach((queryBundle, run) -> teardownSafely(
166-
prestoAction,
169+
helperAction,
167170
Optional.of(queryBundle),
168171
queryStats -> queryStats.getQueryStats().map(QueryStats::getQueryId).ifPresent(run::addTeardownQueryId)));
169172
}

‎presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainVerification.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ protected ExplainMatchResult verify(
122122
}
123123

124124
@Override
125-
protected DeterminismAnalysisDetails analyzeDeterminism(QueryBundle control, ExplainMatchResult matchResult)
125+
protected DeterminismAnalysisDetails analyzeDeterminism(QueryBundle controlObject, QueryBundle testObject, ExplainMatchResult matchResult)
126126
{
127127
throw new UnsupportedOperationException("analyzeDeterminism is not supported for ExplainVerification");
128128
}

0 commit comments

Comments
 (0)