From 686ed219b6e5cad15ba00ca81dda7bc4ef5e0f55 Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Tue, 12 Apr 2022 19:59:25 -0400 Subject: [PATCH 1/2] Support skipping control runs in verifier --- .../java/io/trino/verifier/QueryResult.java | 2 +- .../main/java/io/trino/verifier/Validator.java | 18 +++++++++++++----- .../main/java/io/trino/verifier/Verifier.java | 1 + .../java/io/trino/verifier/VerifierConfig.java | 13 +++++++++++++ .../io/trino/verifier/TestVerifierConfig.java | 7 +++++-- 5 files changed, 33 insertions(+), 8 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 b0a32215171d..6069bf9fffd8 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 @@ -26,7 +26,7 @@ public class QueryResult { public enum State { - INVALID, FAILED, SUCCESS, TOO_MANY_ROWS, TIMEOUT, FAILED_TO_SETUP, FAILED_TO_TEARDOWN + INVALID, FAILED, SUCCESS, TOO_MANY_ROWS, TIMEOUT, FAILED_TO_SETUP, FAILED_TO_TEARDOWN, SKIPPED } private final State state; 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 2ca19318a004..af498ebc9c41 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 @@ -89,6 +89,7 @@ public class Validator private final int controlTeardownRetries; private final int testTeardownRetries; private final boolean runTearDownOnResultMismatch; + private final boolean skipControl; private Boolean valid; @@ -116,6 +117,7 @@ public Validator( int controlTeardownRetries, int testTeardownRetries, boolean runTearDownOnResultMismatch, + boolean skipControl, QueryPair queryPair) { this.testUsername = requireNonNull(queryPair.getTest().getUsername(), "test username is null"); @@ -135,6 +137,7 @@ public Validator( this.controlTeardownRetries = controlTeardownRetries; this.testTeardownRetries = testTeardownRetries; this.runTearDownOnResultMismatch = runTearDownOnResultMismatch; + this.skipControl = skipControl; this.queryPair = requireNonNull(queryPair, "queryPair is null"); this.controlSessionProperties = queryPair.getControl().getSessionProperties(); @@ -147,7 +150,7 @@ public boolean isSkipped() return true; } - if (getControlResult().getState() != State.SUCCESS) { + if (!skipControl && getControlResult().getState() != State.SUCCESS) { return true; } @@ -213,7 +216,12 @@ private boolean validate() boolean tearDownControl = true; boolean tearDownTest = false; try { - controlResult = executePreAndMainForControl(); + if (skipControl) { + controlResult = new QueryResult(State.SKIPPED, null, null, null, null, ImmutableList.of()); + } + else { + controlResult = executePreAndMainForControl(); + } // query has too many rows. Consider banning it. if (controlResult.getState() == State.TOO_MANY_ROWS) { @@ -221,7 +229,7 @@ private boolean validate() return false; } // query failed in the control - if (controlResult.getState() != State.SUCCESS) { + if (!skipControl && controlResult.getState() != State.SUCCESS) { testResult = new QueryResult(State.INVALID, null, null, null, null, ImmutableList.of()); return true; } @@ -229,11 +237,11 @@ private boolean validate() testResult = executePreAndMainForTest(); tearDownTest = true; - if (controlResult.getState() != State.SUCCESS || testResult.getState() != State.SUCCESS) { + if ((!skipControl && controlResult.getState() != State.SUCCESS) || testResult.getState() != State.SUCCESS) { return false; } - if (!checkCorrectness) { + if (skipControl || !checkCorrectness) { return true; } 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 3c344673af81..0d257c1cb364 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 @@ -111,6 +111,7 @@ public int run(List queries) config.getControlTeardownRetries(), config.getTestTeardownRetries(), config.getRunTearDownOnResultMismatch(), + config.isSkipControl(), query); completionService.submit(validator::valid, validator); queriesSubmitted++; 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 242ba1b63aa0..b11071482fbe 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 @@ -88,6 +88,7 @@ public class VerifierConfig private String shadowTestTablePrefix = "tmp_verifier_"; private String shadowControlTablePrefix = "tmp_verifier_"; private boolean runTearDownOnResultMismatch; + private boolean skipControl; private Duration regressionMinCpuTime = new Duration(5, TimeUnit.MINUTES); @@ -780,4 +781,16 @@ public VerifierConfig setRunTearDownOnResultMismatch(boolean runTearDownOnResult this.runTearDownOnResultMismatch = runTearDownOnResultMismatch; return this; } + + public boolean isSkipControl() + { + return skipControl; + } + + @Config("skip-control") + public VerifierConfig setSkipControl(boolean skipControl) + { + this.skipControl = skipControl; + 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 21f5f1a21ba0..5c78c43b7524 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 @@ -81,7 +81,8 @@ public void testDefaults() .setShadowControlTablePrefix("tmp_verifier_") .setControlTeardownRetries(1) .setTestTeardownRetries(1) - .setRunTearDownOnResultMismatch(false)); + .setRunTearDownOnResultMismatch(false) + .setSkipControl(false)); } @Test @@ -135,6 +136,7 @@ public void testExplicitPropertyMappings() .put("control.teardown-retries", "5") .put("test.teardown-retries", "7") .put("run-teardown-on-result-mismatch", "true") + .put("skip-control", "true") .buildOrThrow(); VerifierConfig expected = new VerifierConfig().setTestUsernameOverride("verifier-test") @@ -184,7 +186,8 @@ public void testExplicitPropertyMappings() .setShadowControlTablePrefix("schema.tmp_") .setControlTeardownRetries(5) .setTestTeardownRetries(7) - .setRunTearDownOnResultMismatch(true); + .setRunTearDownOnResultMismatch(true) + .setSkipControl(true); assertFullMapping(properties, expected); } From 8bc0e13527c0365b9cca13bfcc2265d7aa3bfa8d Mon Sep 17 00:00:00 2001 From: Andrii Rosa Date: Tue, 12 Apr 2022 21:20:45 -0400 Subject: [PATCH 2/2] Record runtime information for failed queries in verifier --- .../java/io/trino/verifier/Validator.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 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 af498ebc9c41..d5c7d153f4f6 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 @@ -489,13 +489,17 @@ private QueryResult executeQuery(String url, String username, String password, Q ExecutorService executor = newSingleThreadExecutor(); TimeLimiter limiter = SimpleTimeLimiter.create(executor); + long start = System.nanoTime(); String queryId = null; + Duration queryCpuTime = null; try (Connection connection = DriverManager.getConnection(url, username, password)) { trySetConnectionProperties(query, connection); for (Map.Entry entry : sessionProperties.entrySet()) { connection.unwrap(TrinoConnection.class).setSessionProperty(entry.getKey(), entry.getValue()); } + ProgressMonitor progressMonitor = new ProgressMonitor(); + List> results; try (Statement statement = connection.createStatement()) { Stopwatch stopwatch = Stopwatch.createStarted(); Statement limitedStatement = limiter.newProxy(statement, Statement.class, timeout.toMillis(), MILLISECONDS); @@ -503,13 +507,10 @@ private QueryResult executeQuery(String url, String username, String password, Q sql = "EXPLAIN " + sql; } - long start = System.nanoTime(); TrinoStatement trinoStatement = limitedStatement.unwrap(TrinoStatement.class); - ProgressMonitor progressMonitor = new ProgressMonitor(); trinoStatement.setProgressMonitor(progressMonitor); boolean isSelectQuery = limitedStatement.execute(sql); - List> results; if (isSelectQuery) { ResultSetConverter converter = limiter.newProxy( this::convertJdbcResultSet, @@ -523,14 +524,19 @@ private QueryResult executeQuery(String url, String username, String password, Q } trinoStatement.clearProgressMonitor(); - QueryStats queryStats = progressMonitor.getFinalQueryStats(); - if (queryStats == null) { + if (progressMonitor.getFinalQueryStats() == null) { throw new VerifierException("Cannot fetch query stats"); } - Duration queryCpuTime = new Duration(queryStats.getCpuTimeMillis(), MILLISECONDS); - queryId = queryStats.getQueryId(); - return new QueryResult(State.SUCCESS, null, nanosSince(start), queryCpuTime, queryId, results); } + finally { + QueryStats queryStats = progressMonitor.getFinalQueryStats(); + if (queryStats != null) { + queryCpuTime = new Duration(queryStats.getCpuTimeMillis(), MILLISECONDS); + queryId = queryStats.getQueryId(); + } + } + + return new QueryResult(State.SUCCESS, null, nanosSince(start), queryCpuTime, queryId, results); } catch (SQLException e) { Exception exception = e; @@ -539,13 +545,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, null, null, queryId, ImmutableList.of()); + return new QueryResult(state, exception, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); } catch (VerifierException e) { - return new QueryResult(State.TOO_MANY_ROWS, e, null, null, queryId, ImmutableList.of()); + return new QueryResult(State.TOO_MANY_ROWS, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); } catch (UncheckedTimeoutException e) { - return new QueryResult(State.TIMEOUT, e, null, null, queryId, ImmutableList.of()); + return new QueryResult(State.TIMEOUT, e, nanosSince(start), queryCpuTime, queryId, ImmutableList.of()); } finally { executor.shutdownNow();