Skip to content

Commit

Permalink
Export checksum query and query id even if the query fails
Browse files Browse the repository at this point in the history
  • Loading branch information
caithagoras0 authored and mbasmanova committed Dec 4, 2019
1 parent 2d3a241 commit 7cedd37
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 94 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public AbstractVerification(
this.runTearDownOnResultMismatch = verifierConfig.isRunTearDownOnResultMismatch();
}

protected abstract VerificationResult verify(QueryBundle control, QueryBundle test);
protected abstract MatchResult verify(QueryBundle control, QueryBundle test);

protected abstract DeterminismAnalysis analyzeDeterminism(QueryBundle control, ChecksumResult firstChecksum);

Expand All @@ -110,7 +110,7 @@ public Optional<VerifierQueryEvent> run()
boolean resultMismatched = false;
QueryBundle control = null;
QueryBundle test = null;
VerificationResult verificationResult = null;
MatchResult matchResult = null;
Optional<DeterminismAnalysis> determinismAnalysis = Optional.empty();

QueryStats controlQueryStats = null;
Expand All @@ -121,23 +121,23 @@ public Optional<VerifierQueryEvent> run()
test = queryRewriter.rewriteQuery(sourceQuery.getTestQuery(), TEST);
controlQueryStats = setupAndRun(control, false);
testQueryStats = setupAndRun(test, false);
verificationResult = verify(control, test);
matchResult = verify(control, test);

if (verificationResult.getMatchResult().isMismatchPossiblyCausedByNonDeterminism()) {
determinismAnalysis = Optional.of(analyzeDeterminism(control, verificationResult.getMatchResult().getControlChecksum()));
if (matchResult.isMismatchPossiblyCausedByNonDeterminism()) {
determinismAnalysis = Optional.of(analyzeDeterminism(control, matchResult.getControlChecksum()));
}
boolean maybeDeterministic = !determinismAnalysis.isPresent() ||
determinismAnalysis.get().isDeterministic() ||
determinismAnalysis.get().isUnknown();
resultMismatched = maybeDeterministic && !verificationResult.getMatchResult().isMatched();
resultMismatched = maybeDeterministic && !matchResult.isMatched();

return Optional.of(buildEvent(
Optional.of(control),
Optional.of(test),
Optional.ofNullable(controlQueryStats),
Optional.ofNullable(testQueryStats),
Optional.empty(),
Optional.of(verificationResult),
Optional.of(matchResult),
determinismAnalysis));
}
catch (QueryException e) {
Expand All @@ -150,7 +150,7 @@ public Optional<VerifierQueryEvent> run()
Optional.ofNullable(controlQueryStats),
Optional.ofNullable(testQueryStats),
Optional.of(e),
Optional.ofNullable(verificationResult),
Optional.ofNullable(matchResult),
determinismAnalysis));
}
catch (Throwable t) {
Expand All @@ -175,6 +175,11 @@ protected QueryRewriter getQueryRewriter()
return queryRewriter;
}

protected VerificationContext getVerificationContext()
{
return verificationContext;
}

protected QueryStats setupAndRun(QueryBundle bundle, boolean determinismAnalysis)
{
checkState(!determinismAnalysis || bundle.getCluster() == CONTROL, "Determinism analysis can only be run on control cluster");
Expand Down Expand Up @@ -209,10 +214,10 @@ private VerifierQueryEvent buildEvent(
Optional<QueryStats> controlStats,
Optional<QueryStats> testStats,
Optional<QueryException> queryException,
Optional<VerificationResult> verificationResult,
Optional<MatchResult> matchResult,
Optional<DeterminismAnalysis> determinismAnalysis)
{
boolean succeeded = verificationResult.isPresent() && verificationResult.get().getMatchResult().isMatched();
boolean succeeded = matchResult.isPresent() && matchResult.get().isMatched();

QueryState controlState = getQueryState(controlStats, queryException, CONTROL);
QueryState testState = getQueryState(testStats, queryException, TEST);
Expand All @@ -227,8 +232,8 @@ private VerifierQueryEvent buildEvent(
queryException.get().getQueryStage().getTargetCluster(),
getStackTraceAsString(queryException.get().getCause()));
}
if (verificationResult.isPresent()) {
errorMessage += verificationResult.get().getMatchResult().getResultsComparison();
if (matchResult.isPresent()) {
errorMessage += matchResult.get().getResultsComparison();
}
}

Expand Down Expand Up @@ -259,7 +264,7 @@ else if (skippedReason.isPresent()) {
Optional<String> errorCode = Optional.empty();
if (!succeeded) {
errorCode = Optional.ofNullable(queryException.map(QueryException::getErrorCode).orElse(
verificationResult.map(VerificationResult::getMatchResult).map(MatchResult::getMatchType).map(MatchType::name).orElse(null)));
matchResult.map(MatchResult::getMatchType).map(MatchType::name).orElse(null)));
}

return new VerifierQueryEvent(
Expand All @@ -273,15 +278,15 @@ else if (skippedReason.isPresent()) {
Optional.of(buildQueryInfo(
sourceQuery.getControlConfiguration(),
sourceQuery.getControlQuery(),
verificationResult.map(VerificationResult::getControlChecksumQueryId),
verificationResult.map(VerificationResult::getControlChecksumQuery),
verificationContext.getControlChecksumQueryId(),
verificationContext.getControlChecksumQuery(),
control,
controlStats)),
Optional.of(buildQueryInfo(
sourceQuery.getTestConfiguration(),
sourceQuery.getTestQuery(),
verificationResult.map(VerificationResult::getTestChecksumQueryId),
verificationResult.map(VerificationResult::getTestChecksumQuery),
verificationContext.getTestChecksumQueryId(),
verificationContext.getTestChecksumQuery(),
test,
testStats)),
errorCode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import static com.facebook.presto.verifier.framework.MatchResult.MatchType.SCHEMA_MISMATCH;
import static com.facebook.presto.verifier.framework.QueryStage.CHECKSUM;
import static com.facebook.presto.verifier.framework.QueryStage.DESCRIBE;
import static com.facebook.presto.verifier.framework.VerifierUtil.callWithQueryStatsConsumer;
import static com.google.common.collect.Iterables.getOnlyElement;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -76,22 +77,25 @@ public DataVerification(
}

@Override
public VerificationResult verify(QueryBundle control, QueryBundle test)
public MatchResult verify(QueryBundle control, QueryBundle test)
{
List<Column> controlColumns = getColumns(control.getTableName());
List<Column> testColumns = getColumns(test.getTableName());
ChecksumQueryAndResult controlChecksum = computeChecksum(control, controlColumns);
ChecksumQueryAndResult testChecksum = computeChecksum(test, testColumns);
return new VerificationResult(
controlChecksum.getQueryId(),
testChecksum.getQueryId(),
formatSql(controlChecksum.getQuery()),
formatSql(testChecksum.getQuery()),
match(
controlColumns,
testColumns,
controlChecksum.getResult(),
testChecksum.getResult()));

Query controlChecksumQuery = checksumValidator.generateChecksumQuery(control.getTableName(), controlColumns);
Query testChecksumQuery = checksumValidator.generateChecksumQuery(test.getTableName(), testColumns);

getVerificationContext().setControlChecksumQuery(formatSql(controlChecksumQuery));
getVerificationContext().setTestChecksumQuery(formatSql(testChecksumQuery));

QueryResult<ChecksumResult> controlChecksum = callWithQueryStatsConsumer(
() -> executeChecksumQuery(controlChecksumQuery),
stats -> getVerificationContext().setControlChecksumQueryId(stats.getQueryId()));
QueryResult<ChecksumResult> testChecksum = callWithQueryStatsConsumer(
() -> executeChecksumQuery(testChecksumQuery),
stats -> getVerificationContext().setTestChecksumQueryId(stats.getQueryId()));

return match(controlColumns, testColumns, getOnlyElement(controlChecksum.getResults()), getOnlyElement(testChecksum.getResults()));
}

@Override
Expand Down Expand Up @@ -200,6 +204,11 @@ private List<Column> getColumns(QualifiedName tableName)
.getResults();
}

private QueryResult<ChecksumResult> executeChecksumQuery(Query query)
{
return getPrestoAction().execute(query, CHECKSUM, ChecksumResult::fromResultSet);
}

private ChecksumQueryAndResult computeChecksum(QueryBundle bundle, List<Column> columns)
{
Query checksumQuery = checksumValidator.generateChecksumQuery(bundle.getTableName(), columns);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,65 @@
import com.google.common.collect.ImmutableSet;

import java.util.List;
import java.util.Optional;

import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;

public class VerificationContext
{
private String controlChecksumQueryId;
private String controlChecksumQuery;
private String testChecksumQueryId;
private String testChecksumQuery;

private ImmutableSet.Builder<QueryException> queryExceptions = ImmutableSet.builder();

public Optional<String> getControlChecksumQueryId()
{
return Optional.ofNullable(controlChecksumQueryId);
}

public void setControlChecksumQueryId(String controlChecksumQueryId)
{
checkState(this.controlChecksumQueryId == null, "controlChecksumQueryId is already set");
this.controlChecksumQueryId = requireNonNull(controlChecksumQueryId, "controlChecksumQueryId is null");
}

public Optional<String> getControlChecksumQuery()
{
return Optional.ofNullable(controlChecksumQuery);
}

public void setControlChecksumQuery(String controlChecksumQuery)
{
checkState(this.controlChecksumQuery == null, "controlChecksumQuery is already set");
this.controlChecksumQuery = requireNonNull(controlChecksumQuery, "controlChecksumQuery is null");
}

public Optional<String> getTestChecksumQueryId()
{
return Optional.ofNullable(testChecksumQueryId);
}

public void setTestChecksumQueryId(String testChecksumQueryId)
{
checkState(this.testChecksumQueryId == null, "testChecksumQueryId is already set");
this.testChecksumQueryId = requireNonNull(testChecksumQueryId, "testChecksumQueryId is null");
}

public Optional<String> getTestChecksumQuery()
{
return Optional.ofNullable(testChecksumQuery);
}

public void setTestChecksumQuery(String testChecksumQuery)
{
checkState(this.testChecksumQuery == null, "testChecksumQuery is already set");
this.testChecksumQuery = requireNonNull(testChecksumQuery, "testChecksumQuery is null");
}

public void addException(QueryException exception)
{
queryExceptions.add(exception);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
*/
package com.facebook.presto.verifier.framework;

import com.facebook.presto.jdbc.QueryStats;
import com.facebook.presto.sql.parser.ParsingOptions;
import com.facebook.presto.sql.tree.Identifier;

import java.util.function.Consumer;
import java.util.function.Function;

import static com.facebook.presto.sql.parser.ParsingOptions.DecimalLiteralTreatment.AS_DOUBLE;

public class VerifierUtil
Expand All @@ -30,4 +34,27 @@ public static Identifier delimitedIdentifier(String name)
{
return new Identifier(name, true);
}

public static <V> QueryResult<V> callWithQueryStatsConsumer(Callable<QueryResult<V>> callable, Consumer<QueryStats> queryStatsConsumer)
{
return callWithQueryStatsConsumer(callable, QueryResult::getQueryStats, queryStatsConsumer);
}

private static <V> V callWithQueryStatsConsumer(Callable<V> callable, Function<V, QueryStats> queryStatsTransformer, Consumer<QueryStats> queryStatsConsumer)
{
try {
V result = callable.call();
queryStatsConsumer.accept(queryStatsTransformer.apply(result));
return result;
}
catch (QueryException e) {
e.getQueryStats().ifPresent(queryStatsConsumer);
throw e;
}
}

public interface Callable<V>
{
V call();
}
}
Loading

0 comments on commit 7cedd37

Please sign in to comment.