From 9775ca19631536daeab9e2dc9281f09e259f1c0c Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Wed, 27 Apr 2022 12:13:57 -0400 Subject: [PATCH 1/6] Store column types in QueryResult in verifier --- .../java/io/trino/verifier/QueryResult.java | 9 ++++- .../java/io/trino/verifier/Validator.java | 40 +++++++++++++------ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/QueryResult.java b/service/trino-verifier/src/main/java/io/trino/verifier/QueryResult.java index 6069bf9fffd8..290f18bdfa26 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/QueryResult.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/QueryResult.java @@ -34,15 +34,17 @@ public enum State private final Duration wallTime; private final Duration cpuTime; private final String queryId; + private final List columnTypes; private final List> results; - public QueryResult(State state, Exception exception, Duration wallTime, Duration cpuTime, String queryId, List> results) + public QueryResult(State state, Exception exception, Duration wallTime, Duration cpuTime, String queryId, List columnTypes, List> results) { this.state = requireNonNull(state, "state is null"); this.exception = exception; this.wallTime = wallTime; this.cpuTime = cpuTime; this.queryId = queryId; + this.columnTypes = ImmutableList.copyOf(columnTypes); this.results = (results != null) ? ImmutableList.copyOf(results) : null; } @@ -71,6 +73,11 @@ public String getQueryId() return queryId; } + public List getColumnTypes() + { + return columnTypes; + } + public List> getResults() { return results; diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java b/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java index d5c7d153f4f6..3b8b3d92ca21 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java @@ -40,6 +40,7 @@ import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; +import java.sql.ResultSetMetaData; import java.sql.SQLClientInfoException; import java.sql.SQLException; import java.sql.Statement; @@ -217,7 +218,7 @@ private boolean validate() boolean tearDownTest = false; try { if (skipControl) { - controlResult = new QueryResult(State.SKIPPED, null, null, null, null, ImmutableList.of()); + controlResult = new QueryResult(State.SKIPPED, null, null, null, null, ImmutableList.of(), ImmutableList.of()); } else { controlResult = executePreAndMainForControl(); @@ -225,12 +226,12 @@ private boolean validate() // query has too many rows. Consider banning it. if (controlResult.getState() == State.TOO_MANY_ROWS) { - testResult = new QueryResult(State.INVALID, null, null, null, null, ImmutableList.of()); + testResult = new QueryResult(State.INVALID, null, null, null, null, ImmutableList.of(), ImmutableList.of()); return false; } // query failed in the control if (!skipControl && controlResult.getState() != State.SUCCESS) { - testResult = new QueryResult(State.INVALID, null, null, null, null, ImmutableList.of()); + testResult = new QueryResult(State.INVALID, null, null, null, null, ImmutableList.of(), ImmutableList.of()); return true; } @@ -304,11 +305,11 @@ private QueryResult tearDown(Query query, List postQueryResults, Fu QueryResult queryResult = executor.apply(postqueryString); postQueryResults.add(queryResult); if (queryResult.getState() != State.SUCCESS) { - return new QueryResult(State.FAILED_TO_TEARDOWN, queryResult.getException(), queryResult.getWallTime(), queryResult.getCpuTime(), queryResult.getQueryId(), ImmutableList.of()); + return new QueryResult(State.FAILED_TO_TEARDOWN, queryResult.getException(), queryResult.getWallTime(), queryResult.getCpuTime(), queryResult.getQueryId(), ImmutableList.of(), ImmutableList.of()); } } - return new QueryResult(State.SUCCESS, null, null, null, null, ImmutableList.of()); + return new QueryResult(State.SUCCESS, null, null, null, null, ImmutableList.of(), ImmutableList.of()); } private static QueryResult setup(Query query, List preQueryResults, Function executor) @@ -321,11 +322,11 @@ private static QueryResult setup(Query query, List preQueryResults, return queryResult; } else if (queryResult.getState() != State.SUCCESS) { - return new QueryResult(State.FAILED_TO_SETUP, queryResult.getException(), queryResult.getWallTime(), queryResult.getCpuTime(), queryResult.getQueryId(), ImmutableList.of()); + return new QueryResult(State.FAILED_TO_SETUP, queryResult.getException(), queryResult.getWallTime(), queryResult.getCpuTime(), queryResult.getQueryId(), ImmutableList.of(), ImmutableList.of()); } } - return new QueryResult(State.SUCCESS, null, null, null, null, ImmutableList.of()); + return new QueryResult(State.SUCCESS, null, null, null, null, ImmutableList.of(), ImmutableList.of()); } private boolean checkForDeterministicAndRerunTestQueriesIfNeeded() @@ -500,6 +501,7 @@ private QueryResult executeQuery(String url, String username, String password, Q ProgressMonitor progressMonitor = new ProgressMonitor(); List> results; + List columnTypes; try (Statement statement = connection.createStatement()) { Stopwatch stopwatch = Stopwatch.createStarted(); Statement limitedStatement = limiter.newProxy(statement, Statement.class, timeout.toMillis(), MILLISECONDS); @@ -517,10 +519,13 @@ private QueryResult executeQuery(String url, String username, String password, Q ResultSetConverter.class, timeout.toMillis() - stopwatch.elapsed(MILLISECONDS), MILLISECONDS); - results = converter.convert(limitedStatement.getResultSet()); + ResultSet resultSet = limitedStatement.getResultSet(); + results = converter.convert(resultSet); + columnTypes = getColumnTypes(resultSet); } else { results = ImmutableList.of(ImmutableList.of(limitedStatement.getLargeUpdateCount())); + columnTypes = ImmutableList.of("BIGINT"); } trinoStatement.clearProgressMonitor(); @@ -536,7 +541,7 @@ private QueryResult executeQuery(String url, String username, String password, Q } } - return new QueryResult(State.SUCCESS, null, nanosSince(start), queryCpuTime, queryId, results); + return new QueryResult(State.SUCCESS, null, nanosSince(start), queryCpuTime, queryId, columnTypes, results); } catch (SQLException e) { Exception exception = e; @@ -545,13 +550,13 @@ private QueryResult executeQuery(String url, String username, String password, Q exception = (Exception) e.getCause(); } State state = isPrestoQueryInvalid(e) ? State.INVALID : State.FAILED; - return new QueryResult(state, exception, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); + return new QueryResult(state, exception, nanosSince(start), queryCpuTime, queryId, ImmutableList.of(), ImmutableList.of()); } catch (VerifierException e) { - return new QueryResult(State.TOO_MANY_ROWS, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); + return new QueryResult(State.TOO_MANY_ROWS, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of(), ImmutableList.of()); } catch (UncheckedTimeoutException e) { - return new QueryResult(State.TIMEOUT, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); + return new QueryResult(State.TIMEOUT, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of(), ImmutableList.of()); } finally { executor.shutdownNow(); @@ -819,6 +824,17 @@ static int precisionCompare(double a, double b, int precision) return isClose(a, b, Math.pow(10, -1 * (precision - 1))) ? 0 : -1; } + private static List getColumnTypes(ResultSet resultSet) + throws SQLException + { + ResultSetMetaData metadata = resultSet.getMetaData(); + ImmutableList.Builder builder = ImmutableList.builder(); + for (int column = 1; column <= metadata.getColumnCount(); column++) { + builder.add(metadata.getColumnTypeName(column)); + } + return builder.build(); + } + public static class ChangedRow implements Comparable { From 48f733a0977924582d6c9afd78b1b46e83d445e5 Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Wed, 27 Apr 2022 18:54:36 -0400 Subject: [PATCH 2/6] Fix decimal comparison in verifier --- .../java/io/trino/verifier/Validator.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java b/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java index 3b8b3d92ca21..1d87bf80e829 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/Validator.java @@ -605,14 +605,6 @@ private List> convertJdbcResultSet(ResultSet resultSet) List row = new ArrayList<>(); for (int i = 1; i <= columnCount; i++) { Object object = resultSet.getObject(i); - if (object instanceof BigDecimal) { - if (((BigDecimal) object).scale() <= 0) { - object = ((BigDecimal) object).longValueExact(); - } - else { - object = ((BigDecimal) object).doubleValue(); - } - } if (object instanceof Array) { object = ((Array) object).getArray(); } @@ -709,12 +701,16 @@ private static Comparator columnComparator(int precision) Number y = (Number) b; boolean bothReal = isReal(x) && isReal(y); boolean bothIntegral = isIntegral(x) && isIntegral(y); - if (!(bothReal || bothIntegral)) { + boolean bothDecimals = isDecimal(x) && isDecimal(y); + if (!(bothReal || bothIntegral || bothDecimals)) { throw new TypesDoNotMatchException(format("item types do not match: %s vs %s", a.getClass().getName(), b.getClass().getName())); } if (isIntegral(x)) { return Long.compare(x.longValue(), y.longValue()); } + if (isDecimal(x)) { + return ((BigDecimal) x).compareTo((BigDecimal) y); + } return precisionCompare(x.doubleValue(), y.doubleValue(), precision); } if (a.getClass() != b.getClass()) { @@ -795,6 +791,11 @@ private static boolean isIntegral(Number x) return x instanceof Byte || x instanceof Short || x instanceof Integer || x instanceof Long; } + private static boolean isDecimal(Number x) + { + return x instanceof BigDecimal; + } + //adapted from http://floating-point-gui.de/errors/comparison/ private static boolean isClose(double a, double b, double epsilon) { From e49824e4be56da2c580be4442ada04f82069ae9a Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Wed, 27 Apr 2022 18:54:55 -0400 Subject: [PATCH 3/6] Use mediumtext for columns storing query text in verifier --- service/trino-verifier/README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/service/trino-verifier/README.md b/service/trino-verifier/README.md index ae0e5ef994a0..f23a6d655cb9 100644 --- a/service/trino-verifier/README.md +++ b/service/trino-verifier/README.md @@ -15,17 +15,17 @@ CREATE TABLE verifier_queries( name VARCHAR(256), test_catalog VARCHAR(256) NOT NULL, test_schema VARCHAR(256) NOT NULL, - test_prequeries TEXT, - test_query TEXT NOT NULL, - test_postqueries TEXT, + test_prequeries MEDIUMTEXT, + test_query MEDIUMTEXT NOT NULL, + test_postqueries MEDIUMTEXT, test_username VARCHAR(256) NOT NULL default 'verifier-test', test_password VARCHAR(256), test_session_properties_json VARCHAR(2048), control_catalog VARCHAR(256) NOT NULL, control_schema VARCHAR(256) NOT NULL, - control_prequeries TEXT, - control_query TEXT NOT NULL, - control_postqueries TEXT, + control_prequeries MEDIUMTEXT, + control_query MEDIUMTEXT NOT NULL, + control_postqueries MEDIUMTEXT, control_username VARCHAR(256) NOT NULL default 'verifier-test', control_password VARCHAR(256), control_session_properties_json VARCHAR(2048), From 04c682f7c8cbfb8dc87bf3b64b756907eee3edb3 Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Wed, 27 Apr 2022 18:56:05 -0400 Subject: [PATCH 4/6] Use fully specified class names in HiveQueryRunner To make it easier to uncomment --- .../src/test/java/io/trino/plugin/hive/HiveQueryRunner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/HiveQueryRunner.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/HiveQueryRunner.java index ad6f919af633..e36058bc9e57 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/HiveQueryRunner.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/HiveQueryRunner.java @@ -399,8 +399,8 @@ public static void main(String[] args) .setSecurity(ALLOW_ALL) // Uncomment to enable standard column naming (column names to be prefixed with the first letter of the table name, e.g.: o_orderkey vs orderkey) // and standard column types (decimals vs double for some columns). This will allow running unmodified tpch queries on the cluster. - // .setTpchColumnNaming(STANDARD) - // .setTpchDecimalTypeMapping(DECIMAL) + //.setTpchColumnNaming(ColumnNaming.STANDARD) + //.setTpchDecimalTypeMapping(DecimalTypeMapping.DECIMAL) .build(); Thread.sleep(10); log.info("======== SERVER STARTED ========"); From cf5d8f02ea9f0f8ae1d0da77d452c30e478ef7bc Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Thu, 28 Apr 2022 12:21:17 -0400 Subject: [PATCH 5/6] Support simplified control query generation in verifier When the set of queries run through verifier is fixed it is wasteful to recompute control result every run. The functionality added allows to configure verifier to store expected results in a form of a simple `SELECT * FROM VALUES ... ` Trino query. --- .../main/java/io/trino/verifier/Verifier.java | 100 ++++++++++++++++++ .../io/trino/verifier/VerifierConfig.java | 27 +++++ .../io/trino/verifier/TestVerifierConfig.java | 10 +- 3 files changed, 135 insertions(+), 2 deletions(-) diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java b/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java index 0d257c1cb364..e46b4f5557b3 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java @@ -24,8 +24,12 @@ import java.io.Closeable; import java.io.IOException; import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletionService; import java.util.concurrent.ExecutionException; @@ -40,6 +44,9 @@ import static io.trino.spi.StandardErrorCode.TOO_MANY_REQUESTS_FAILED; import static io.trino.verifier.QueryResult.State.SUCCESS; import static java.lang.String.format; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.Files.createDirectories; +import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newFixedThreadPool; import static java.util.concurrent.TimeUnit.SECONDS; @@ -145,6 +152,25 @@ public int run(List queries) continue; } + QueryResult controlResult = validator.getControlResult(); + if (config.isSimplifiedControlQueriesGenerationEnabled() && controlResult.getState() == SUCCESS) { + QueryPair queryPair = validator.getQueryPair(); + Path path = Paths.get(format( + "%s/%s/%s/%s.sql", + config.getSimplifiedControlQueriesOutputDirectory(), + config.getRunId(), + queryPair.getSuite(), + queryPair.getName())); + try { + String content = generateCorrespondingSelect(controlResult.getColumnTypes(), controlResult.getResults()); + createDirectories(path.getParent()); + Files.write(path, content.getBytes(UTF_8)); + } + catch (IOException | RuntimeException e) { + log.error(e, "Failed generating corresponding select statement for expected results for query %s", queryPair.getName()); + } + } + if (validator.valid()) { valid++; } @@ -278,4 +304,78 @@ private static boolean shouldAddStackTrace(Exception e) } return true; } + + private static String generateCorrespondingSelect(List columnTypes, List> rows) + { + StringBuilder sb = new StringBuilder("SELECT *\nFROM\n(\n VALUES\n"); + for (int rowIndex = 0; rowIndex < rows.size(); rowIndex++) { + List row = rows.get(rowIndex); + sb.append(" ("); + for (int columnIndex = 0; columnIndex < columnTypes.size(); columnIndex++) { + String type = columnTypes.get(columnIndex); + Optional value = Optional.ofNullable(row.get(columnIndex)).map(Object::toString); + String literal = getLiteral(type, value); + sb.append(literal); + if (columnIndex < columnTypes.size() - 1) { + sb.append(", "); + } + } + sb.append(")"); + if (rowIndex < rows.size() - 1) { + sb.append(","); + } + sb.append("\n"); + } + if (rows.isEmpty()) { + sb.append(" ("); + for (int columnIndex = 0; columnIndex < columnTypes.size(); columnIndex++) { + sb.append("NULL"); + if (columnIndex < columnTypes.size() - 1) { + sb.append(", "); + } + } + sb.append(")\n"); + } + sb.append(")\n"); + if (rows.isEmpty()) { + sb.append("WHERE 1=0\n"); + } + return sb.toString(); + } + + private static String getLiteral(String type, Optional value) + { + String baseType = getBaseType(type); + switch (baseType) { + case "TINYINT": + case "SMALLINT": + case "INTEGER": + case "BIGINT": + case "DECIMAL": + case "DATE": + case "TIME": + case "REAL": + case "DOUBLE": + return value.map(v -> baseType + " '" + v + "'").orElse("NULL"); + case "CHAR": + case "VARCHAR": + return value.map(v -> baseType + " '" + v.replaceAll("'", "''") + "'").orElse("NULL"); + case "VARBINARY": + return value.map(v -> "X'" + v + "'").orElse("NULL"); + case "UNKNOWN": + return "NULL"; + default: + throw new IllegalArgumentException(format("Unexpected type: %s", type)); + } + } + + private static String getBaseType(String type) + { + String baseType = type.toUpperCase(ENGLISH); + int index = baseType.indexOf('('); + if (index != -1) { + baseType = baseType.substring(0, index); + } + return baseType; + } } diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierConfig.java b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierConfig.java index b11071482fbe..5d61850f500a 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierConfig.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierConfig.java @@ -89,6 +89,8 @@ public class VerifierConfig private String shadowControlTablePrefix = "tmp_verifier_"; private boolean runTearDownOnResultMismatch; private boolean skipControl; + private boolean simplifiedControlQueriesGenerationEnabled; + private String simplifiedControlQueriesOutputDirectory = "/tmp/verifier/generated-control-queries"; private Duration regressionMinCpuTime = new Duration(5, TimeUnit.MINUTES); @@ -793,4 +795,29 @@ public VerifierConfig setSkipControl(boolean skipControl) this.skipControl = skipControl; return this; } + + public boolean isSimplifiedControlQueriesGenerationEnabled() + { + return simplifiedControlQueriesGenerationEnabled; + } + + @Config("simplified-control-queries-generation-enabled") + public VerifierConfig setSimplifiedControlQueriesGenerationEnabled(boolean simplifiedControlQueriesGenerationEnabled) + { + this.simplifiedControlQueriesGenerationEnabled = simplifiedControlQueriesGenerationEnabled; + return this; + } + + @NotNull + public String getSimplifiedControlQueriesOutputDirectory() + { + return simplifiedControlQueriesOutputDirectory; + } + + @Config("simplified-control-queries-output-directory") + public VerifierConfig setSimplifiedControlQueriesOutputDirectory(String simplifiedControlQueriesOutputDirectory) + { + this.simplifiedControlQueriesOutputDirectory = simplifiedControlQueriesOutputDirectory; + return this; + } } diff --git a/service/trino-verifier/src/test/java/io/trino/verifier/TestVerifierConfig.java b/service/trino-verifier/src/test/java/io/trino/verifier/TestVerifierConfig.java index 5c78c43b7524..18b00679813d 100644 --- a/service/trino-verifier/src/test/java/io/trino/verifier/TestVerifierConfig.java +++ b/service/trino-verifier/src/test/java/io/trino/verifier/TestVerifierConfig.java @@ -82,7 +82,9 @@ public void testDefaults() .setControlTeardownRetries(1) .setTestTeardownRetries(1) .setRunTearDownOnResultMismatch(false) - .setSkipControl(false)); + .setSkipControl(false) + .setSimplifiedControlQueriesGenerationEnabled(false) + .setSimplifiedControlQueriesOutputDirectory("/tmp/verifier/generated-control-queries")); } @Test @@ -137,6 +139,8 @@ public void testExplicitPropertyMappings() .put("test.teardown-retries", "7") .put("run-teardown-on-result-mismatch", "true") .put("skip-control", "true") + .put("simplified-control-queries-generation-enabled", "true") + .put("simplified-control-queries-output-directory", "/tmp/some/directory") .buildOrThrow(); VerifierConfig expected = new VerifierConfig().setTestUsernameOverride("verifier-test") @@ -187,7 +191,9 @@ public void testExplicitPropertyMappings() .setControlTeardownRetries(5) .setTestTeardownRetries(7) .setRunTearDownOnResultMismatch(true) - .setSkipControl(true); + .setSkipControl(true) + .setSimplifiedControlQueriesGenerationEnabled(true) + .setSimplifiedControlQueriesOutputDirectory("/tmp/some/directory"); assertFullMapping(properties, expected); } From 20fe37feda219e0c2e07dcc97fb88705ab9ddd0d Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Thu, 28 Apr 2022 12:52:58 -0400 Subject: [PATCH 6/6] Store cpu and wall time for queries run in verifier For INSERT queries the real query is run at "pre" phase, while the main test query is merely a check sum computation. Storing the total time spent on running all test or control related queries will make it less confusing. --- .../trino/verifier/DatabaseEventClient.java | 14 ++++++++ .../main/java/io/trino/verifier/Verifier.java | 30 +++++++++++++---- .../trino/verifier/VerifierQueryEventDao.java | 12 +++++++ .../verifier/VerifierQueryEventEntity.java | 33 +++++++++++++++++++ .../verifier/TestDatabaseEventClient.java | 8 +++++ 5 files changed, 91 insertions(+), 6 deletions(-) diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/DatabaseEventClient.java b/service/trino-verifier/src/main/java/io/trino/verifier/DatabaseEventClient.java index 29e16d6598ea..a50ed768743e 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/DatabaseEventClient.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/DatabaseEventClient.java @@ -16,11 +16,13 @@ import io.airlift.event.client.AbstractEventClient; import io.airlift.json.JsonCodec; +import javax.annotation.Nullable; import javax.annotation.PostConstruct; import javax.inject.Inject; import java.util.List; import java.util.Optional; +import java.util.OptionalDouble; import static java.util.Objects.requireNonNull; @@ -58,12 +60,24 @@ protected void postEvent(T event) queryEvent.getTestSetupQueryIds().isEmpty() ? Optional.empty() : Optional.of(codec.toJson(queryEvent.getTestSetupQueryIds())), Optional.ofNullable(queryEvent.getTestQueryId()), queryEvent.getTestTeardownQueryIds().isEmpty() ? Optional.empty() : Optional.of(codec.toJson(queryEvent.getTestTeardownQueryIds())), + toOptionalDouble(queryEvent.getTestCpuTimeSecs()), + toOptionalDouble(queryEvent.getTestWallTimeSecs()), Optional.ofNullable(queryEvent.getControlCatalog()), Optional.ofNullable(queryEvent.getControlSchema()), queryEvent.getControlSetupQueryIds().isEmpty() ? Optional.empty() : Optional.of(codec.toJson(queryEvent.getControlSetupQueryIds())), Optional.ofNullable(queryEvent.getControlQueryId()), queryEvent.getControlTeardownQueryIds().isEmpty() ? Optional.empty() : Optional.of(codec.toJson(queryEvent.getControlTeardownQueryIds())), + toOptionalDouble(queryEvent.getControlCpuTimeSecs()), + toOptionalDouble(queryEvent.getControlWallTimeSecs()), Optional.ofNullable(queryEvent.getErrorMessage())); dao.store(entity); } + + private static OptionalDouble toOptionalDouble(@Nullable Double value) + { + if (value == null) { + return OptionalDouble.empty(); + } + return OptionalDouble.of(value); + } } diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java b/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java index e46b4f5557b3..e9d85ae6d837 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/Verifier.java @@ -21,6 +21,8 @@ import io.trino.spi.ErrorCode; import io.trino.spi.TrinoException; +import javax.annotation.Nullable; + import java.io.Closeable; import java.io.IOException; import java.io.PrintStream; @@ -30,15 +32,19 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.OptionalDouble; import java.util.Set; import java.util.concurrent.CompletionService; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; +import java.util.function.Function; import java.util.regex.Pattern; +import java.util.stream.Stream; import static com.google.common.base.Throwables.getStackTraceAsString; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Streams.concat; import static io.trino.spi.StandardErrorCode.PAGE_TRANSPORT_TIMEOUT; import static io.trino.spi.StandardErrorCode.REMOTE_TASK_MISMATCH; import static io.trino.spi.StandardErrorCode.TOO_MANY_REQUESTS_FAILED; @@ -237,6 +243,9 @@ private VerifierQueryEvent buildEvent(Validator validator) } } + Stream controlQueries = concat(Stream.of(control), validator.getControlPreQueryResults().stream(), validator.getControlPostQueryResults().stream()); + Stream testQueries = concat(Stream.of(test), validator.getTestPreQueryResults().stream(), validator.getTestPostQueryResults().stream()); + return new VerifierQueryEvent( queryPair.getSuite(), config.getRunId(), @@ -257,8 +266,8 @@ private VerifierQueryEvent buildEvent(Validator validator) .map(QueryResult::getQueryId) .filter(Objects::nonNull) .collect(toImmutableList()), - optionalDurationToSeconds(test.getCpuTime()), - optionalDurationToSeconds(test.getWallTime()), + getTotalDurationInSeconds(testQueries, QueryResult::getCpuTime), + getTotalDurationInSeconds(testQueries, QueryResult::getWallTime), queryPair.getControl().getCatalog(), queryPair.getControl().getSchema(), queryPair.getControl().getPreQueries(), @@ -273,14 +282,23 @@ private VerifierQueryEvent buildEvent(Validator validator) .map(QueryResult::getQueryId) .filter(Objects::nonNull) .collect(toImmutableList()), - optionalDurationToSeconds(control.getCpuTime()), - optionalDurationToSeconds(control.getWallTime()), + getTotalDurationInSeconds(controlQueries, QueryResult::getCpuTime), + getTotalDurationInSeconds(controlQueries, QueryResult::getWallTime), errorMessage); } - private static Double optionalDurationToSeconds(Duration duration) + @Nullable + private static Double getTotalDurationInSeconds(Stream queries, Function metric) { - return duration != null ? duration.convertTo(SECONDS).getValue() : null; + OptionalDouble result = queries + .map(metric) + .filter(Objects::nonNull) + .mapToDouble(duration -> duration.getValue(SECONDS)) + .reduce(Double::sum); + if (result.isEmpty()) { + return null; + } + return result.getAsDouble(); } private static T takeUnchecked(CompletionService completionService) diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventDao.java b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventDao.java index 844bb0a5f068..20473b02eba6 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventDao.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventDao.java @@ -30,11 +30,15 @@ public interface VerifierQueryEventDao " test_setup_query_ids_json VARCHAR(255) NULL,\n" + " test_query_id VARCHAR(255) NULL,\n" + " test_teardown_query_ids_json VARCHAR(255) NULL,\n" + + " test_cpu_time_seconds DOUBLE NULL,\n" + + " test_wall_time_seconds DOUBLE NULL,\n" + " control_catalog VARCHAR(255) NULL,\n" + " control_schema VARCHAR(255) NULL,\n" + " control_setup_query_ids_json VARCHAR(255) NULL,\n" + " control_query_id VARCHAR(255) NULL,\n" + " control_teardown_query_ids_json VARCHAR(255) NULL,\n" + + " control_cpu_time_seconds DOUBLE NULL,\n" + + " control_wall_time_seconds DOUBLE NULL,\n" + " error_message TEXT NULL,\n" + " PRIMARY KEY (id),\n" + " INDEX run_id_name_index(run_id, name)\n" + @@ -52,11 +56,15 @@ public interface VerifierQueryEventDao " test_setup_query_ids_json,\n" + " test_query_id,\n" + " test_teardown_query_ids_json,\n" + + " test_cpu_time_seconds,\n" + + " test_wall_time_seconds,\n" + " control_catalog,\n" + " control_schema,\n" + " control_setup_query_ids_json,\n" + " control_query_id,\n" + " control_teardown_query_ids_json,\n" + + " control_cpu_time_seconds,\n" + + " control_wall_time_seconds,\n" + " error_message\n" + ")\n" + "VALUES (\n" + @@ -70,11 +78,15 @@ public interface VerifierQueryEventDao " :testSetupQueryIdsJson,\n" + " :testQueryId,\n" + " :testTeardownQueryIdsJson,\n" + + " :testCpuTimeSeconds,\n" + + " :testWallTimeSeconds,\n" + " :controlCatalog,\n" + " :controlSchema,\n" + " :controlSetupQueryIdsJson,\n" + " :controlQueryId,\n" + " :controlTeardownQueryIdsJson,\n" + + " :controlCpuTimeSeconds,\n" + + " :controlWallTimeSeconds,\n" + " :errorMessage\n" + ")") void store(@BindBean VerifierQueryEventEntity entity); diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventEntity.java b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventEntity.java index 9df3add4ee95..4554436e1e55 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventEntity.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/VerifierQueryEventEntity.java @@ -14,6 +14,7 @@ package io.trino.verifier; import java.util.Optional; +import java.util.OptionalDouble; import static java.util.Objects.requireNonNull; @@ -30,12 +31,16 @@ public class VerifierQueryEventEntity private final Optional testSetupQueryIdsJson; private final Optional testQueryId; private final Optional testTeardownQueryIdsJson; + private final OptionalDouble testCpuTimeSeconds; + private final OptionalDouble testWallTimeSeconds; private final Optional controlCatalog; private final Optional controlSchema; private final Optional controlSetupQueryIdsJson; private final Optional controlQueryId; private final Optional controlTeardownQueryIdsJson; + private final OptionalDouble controlCpuTimeSeconds; + private final OptionalDouble controlWallTimeSeconds; private final Optional errorMessage; @@ -50,11 +55,15 @@ public VerifierQueryEventEntity( Optional testSetupQueryIdsJson, Optional testQueryId, Optional testTeardownQueryIdsJson, + OptionalDouble testCpuTimeSeconds, + OptionalDouble testWallTimeSeconds, Optional controlCatalog, Optional controlSchema, Optional controlSetupQueryIdsJson, Optional controlQueryId, Optional controlTeardownQueryIdsJson, + OptionalDouble controlCpuTimeSeconds, + OptionalDouble controlWallTimeSeconds, Optional errorMessage) { this.suite = requireNonNull(suite, "suite is null"); @@ -67,11 +76,15 @@ public VerifierQueryEventEntity( this.testSetupQueryIdsJson = requireNonNull(testSetupQueryIdsJson, "testSetupQueryIdsJson is null"); this.testQueryId = requireNonNull(testQueryId, "testQueryId is null"); this.testTeardownQueryIdsJson = requireNonNull(testTeardownQueryIdsJson, "testTeardownQueryIdsJson is null"); + this.testCpuTimeSeconds = requireNonNull(testCpuTimeSeconds, "testCpuTimeSeconds is null"); + this.testWallTimeSeconds = requireNonNull(testWallTimeSeconds, "testWallTimeSeconds is null"); this.controlCatalog = requireNonNull(controlCatalog, "controlCatalog is null"); this.controlSchema = requireNonNull(controlSchema, "controlSchema is null"); this.controlSetupQueryIdsJson = requireNonNull(controlSetupQueryIdsJson, "controlSetupQueryIdsJson is null"); this.controlQueryId = requireNonNull(controlQueryId, "controlQueryId is null"); this.controlTeardownQueryIdsJson = requireNonNull(controlTeardownQueryIdsJson, "controlTeardownQueryIdsJson is null"); + this.controlCpuTimeSeconds = requireNonNull(controlCpuTimeSeconds, "controlCpuTimeSeconds is null"); + this.controlWallTimeSeconds = requireNonNull(controlWallTimeSeconds, "controlWallTimeSeconds is null"); this.errorMessage = requireNonNull(errorMessage, "errorMessage is null"); } @@ -125,6 +138,16 @@ public Optional getTestTeardownQueryIdsJson() return testTeardownQueryIdsJson; } + public OptionalDouble getTestCpuTimeSeconds() + { + return testCpuTimeSeconds; + } + + public OptionalDouble getTestWallTimeSeconds() + { + return testWallTimeSeconds; + } + public Optional getControlCatalog() { return controlCatalog; @@ -150,6 +173,16 @@ public Optional getControlTeardownQueryIdsJson() return controlTeardownQueryIdsJson; } + public OptionalDouble getControlCpuTimeSeconds() + { + return controlCpuTimeSeconds; + } + + public OptionalDouble getControlWallTimeSeconds() + { + return controlWallTimeSeconds; + } + public Optional getErrorMessage() { return errorMessage; diff --git a/service/trino-verifier/src/test/java/io/trino/verifier/TestDatabaseEventClient.java b/service/trino-verifier/src/test/java/io/trino/verifier/TestDatabaseEventClient.java index c7406531681b..c0625e28d415 100644 --- a/service/trino-verifier/src/test/java/io/trino/verifier/TestDatabaseEventClient.java +++ b/service/trino-verifier/src/test/java/io/trino/verifier/TestDatabaseEventClient.java @@ -153,11 +153,15 @@ public void testFull() assertEquals(resultSet.getString("test_setup_query_ids_json"), codec.toJson(FULL_EVENT.getTestSetupQueryIds())); assertEquals(resultSet.getString("test_query_id"), "TEST_QUERY_ID"); assertEquals(resultSet.getString("test_teardown_query_ids_json"), codec.toJson(FULL_EVENT.getTestTeardownQueryIds())); + assertEquals(resultSet.getDouble("test_cpu_time_seconds"), 1.1); + assertEquals(resultSet.getDouble("test_wall_time_seconds"), 2.2); assertEquals(resultSet.getString("control_catalog"), "controlcatalog"); assertEquals(resultSet.getString("control_schema"), "controlschema"); assertEquals(resultSet.getString("control_setup_query_ids_json"), codec.toJson(FULL_EVENT.getControlSetupQueryIds())); assertEquals(resultSet.getString("control_query_id"), "CONTROL_QUERY_ID"); assertEquals(resultSet.getString("control_teardown_query_ids_json"), codec.toJson(FULL_EVENT.getControlTeardownQueryIds())); + assertEquals(resultSet.getDouble("control_cpu_time_seconds"), 3.3); + assertEquals(resultSet.getDouble("control_wall_time_seconds"), 4.4); assertEquals(resultSet.getString("error_message"), "error message"); assertFalse(resultSet.next()); } @@ -186,11 +190,15 @@ public void testMinimal() assertNull(resultSet.getString("test_setup_query_ids_json")); assertNull(resultSet.getString("test_query_id")); assertNull(resultSet.getString("test_teardown_query_ids_json")); + assertNull(resultSet.getObject("test_cpu_time_seconds")); + assertNull(resultSet.getObject("test_wall_time_seconds")); assertNull(resultSet.getString("control_catalog")); assertNull(resultSet.getString("control_schema")); assertNull(resultSet.getString("control_setup_query_ids_json")); assertNull(resultSet.getString("control_query_id")); assertNull(resultSet.getString("control_teardown_query_ids_json")); + assertNull(resultSet.getObject("control_cpu_time_seconds")); + assertNull(resultSet.getObject("control_wall_time_seconds")); assertNull(resultSet.getString("error_message")); assertFalse(resultSet.next()); }