Repository: hive Updated Branches: refs/heads/master 1542c88d5 -> 7d7c18396
HIVE-19077: Handle duplicate ptests requests standing in queue at the same time (Adam Szita via Peter Vary) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/7d7c1839 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/7d7c1839 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/7d7c1839 Branch: refs/heads/master Commit: 7d7c1839625b7be9846b75c373efb557d03cf3d8 Parents: 1542c88 Author: Adam Szita <[email protected]> Authored: Mon May 14 10:49:36 2018 +0200 Committer: Peter Vary <[email protected]> Committed: Mon May 14 10:49:36 2018 +0200 ---------------------------------------------------------------------- dev-support/jenkins-common.sh | 2 - dev-support/jenkins-execute-build.sh | 3 +- .../hive/ptest/api/client/JenkinsQueueUtil.java | 143 ------------------- .../hive/ptest/api/client/PTestClient.java | 11 +- .../org/apache/hive/ptest/execution/PTest.java | 3 +- .../hive/ptest/execution/TestCheckPhase.java | 22 ++- .../ptest/execution/TestTestCheckPhase.java | 36 ++++- .../src/test/resources/HIVE-19077.1.patch | 14 ++ 8 files changed, 68 insertions(+), 166 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/dev-support/jenkins-common.sh ---------------------------------------------------------------------- diff --git a/dev-support/jenkins-common.sh b/dev-support/jenkins-common.sh index 64f486f..0467d11 100644 --- a/dev-support/jenkins-common.sh +++ b/dev-support/jenkins-common.sh @@ -15,8 +15,6 @@ # limitations under the License. JIRA_ROOT_URL="https://issues.apache.org"; -JENKINS_URL="https://builds.apache.org"; -JENKINS_QUEUE_QUERY="/queue/api/json?tree=items[task[name],inQueueSince,actions[parameters[name,value]],why]" fail() { echo "$@" 1>&2 http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/dev-support/jenkins-execute-build.sh ---------------------------------------------------------------------- diff --git a/dev-support/jenkins-execute-build.sh b/dev-support/jenkins-execute-build.sh index 35392dd..f660fcb 100644 --- a/dev-support/jenkins-execute-build.sh +++ b/dev-support/jenkins-execute-build.sh @@ -51,8 +51,7 @@ call_ptest_server() { local PTEST_CLASSPATH="$PTEST_BUILD_DIR/hive/testutils/ptest2/target/hive-ptest-3.0-classes.jar:$PTEST_BUILD_DIR/hive/testutils/ptest2/target/lib/*" java -cp "$PTEST_CLASSPATH" org.apache.hive.ptest.api.client.PTestClient --command testStart \ - --outputDir "$PTEST_BUILD_DIR/hive/testutils/ptest2/target" --password "$JIRA_PASSWORD" \ - --jenkinsQueueUrl "$JENKINS_URL$JENKINS_QUEUE_QUERY" "$@" + --outputDir "$PTEST_BUILD_DIR/hive/testutils/ptest2/target" --password "$JIRA_PASSWORD" "$@" } # Unpack all test results http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/JenkinsQueueUtil.java ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/JenkinsQueueUtil.java b/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/JenkinsQueueUtil.java deleted file mode 100644 index f335164..0000000 --- a/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/JenkinsQueueUtil.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.hive.ptest.api.client; - -import java.io.IOException; -import java.security.KeyManagementException; -import java.security.NoSuchAlgorithmException; -import java.util.ArrayList; -import java.util.List; - -import org.apache.commons.cli.CommandLine; -import org.apache.http.HttpResponse; -import org.apache.http.StatusLine; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.ssl.SSLContexts; -import org.apache.http.util.EntityUtils; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.Lists; - -/** - * Utility class for the Precommit test job queue on Jenkins - */ -public class JenkinsQueueUtil { - - private static final String JSON_ITEMS_FIELD = "items"; - private static final String JSON_TASK_FIELD = "task"; - private static final String JSON_TASK_NAME_FIELD = "name"; - private static final String JSON_PARAMETERS_FIELD = "parameters"; - private static final String JSON_PARAMETER_NAME_FIELD = "name"; - private static final String JSON_PARAMETER_VALUE_FIELD = "value"; - - private static final String JOB_NAME = "PreCommit-HIVE-Build"; - private static final String ISSUE_FIELD_KEY = "ISSUE_NUM"; - private static final String JIRA_KEY_PREFIX = "HIVE-"; - - /** - * Looks up the current queue of the precommit job on a jenkins instance (specified by - * PTestClient.JENKINS_QUEUE_URL), and checks if current Jira is standing in queue already (i.e. - * will be executed in the future too) - * - * @param commandLine PTestClient's command line option values' list - * @return whether or not the Jira specified in the command line can be found in the job queue - */ - public static boolean isJiraAlreadyInQueue(CommandLine commandLine) { - if (!(commandLine.hasOption(PTestClient.JENKINS_QUEUE_URL) && - commandLine.hasOption(PTestClient.JIRA))) { - return false; - } - try { - System.out.println("Checking " + JOB_NAME + " queue..."); - String queueJson = httpGet(commandLine.getOptionValue(PTestClient.JENKINS_QUEUE_URL)); - List<String> jirasInQueue = parseJiras(queueJson); - if (jirasInQueue.size() > 0) { - System.out.println(JOB_NAME + " has the following jira(s) in queue: " + jirasInQueue); - } else { - return false; - } - - String jira = commandLine.getOptionValue(PTestClient.JIRA).replaceAll(JIRA_KEY_PREFIX,""); - if (jirasInQueue.contains(jira)) { - return true; - } - - } catch (IOException e) { - System.err.println("Error checking " + JOB_NAME + " build queue: " + e); - } - return false; - } - - /** - * Parses raw json to produce a list of Jira number strings. - * @param queueJson - * @return - * @throws IOException - */ - private static List<String> parseJiras(String queueJson) throws IOException { - List<String> jirasInQueue = new ArrayList<>(); - ObjectMapper objectMapper = new ObjectMapper(); - JsonNode rootNode = objectMapper.readTree(queueJson); - List<JsonNode> items = Lists.newArrayList(rootNode.findValue(JSON_ITEMS_FIELD).iterator()); - for (JsonNode item : items) { - String taskName = item.path(JSON_TASK_FIELD).path(JSON_TASK_NAME_FIELD).asText(); - if (JOB_NAME.equals(taskName)) { - List<JsonNode> parameters = Lists.newArrayList(item.findValue(JSON_PARAMETERS_FIELD)); - for (JsonNode parameter : parameters) { - if (ISSUE_FIELD_KEY.equals(parameter.path(JSON_PARAMETER_NAME_FIELD).asText())) { - jirasInQueue.add(parameter.path(JSON_PARAMETER_VALUE_FIELD).asText()); - } - } - } - } - return jirasInQueue; - } - - private static String httpGet(String url) - throws IOException { - - HttpGet request = new HttpGet(url); - try { - CloseableHttpClient httpClient = HttpClientBuilder - .create() - .setSslcontext(SSLContexts.custom().useProtocol("TLSv1.2").build()) - .setRetryHandler(new PTestClient.PTestHttpRequestRetryHandler()) - .build(); - request.addHeader("content-type", "application/json"); - HttpResponse httpResponse = httpClient.execute(request); - StatusLine statusLine = httpResponse.getStatusLine(); - if (statusLine.getStatusCode() != 200) { - throw new IllegalStateException(statusLine.getStatusCode() + " " + statusLine.getReasonPhrase()); - } - String response = EntityUtils.toString(httpResponse.getEntity(), "UTF-8"); - return response; - } catch (NoSuchAlgorithmException | KeyManagementException e) { - e.printStackTrace(); - throw new IOException(e.getMessage()); - } finally { - request.abort(); - } - } - - -} http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java b/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java index 9970c36..e878e18 100644 --- a/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java +++ b/testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java @@ -81,11 +81,10 @@ public class PTestClient { private static final String PASSWORD = "password"; private static final String PROFILE = "profile"; private static final String PATCH = "patch"; - public static final String JIRA = "jira"; + private static final String JIRA = "jira"; private static final String OUTPUT_DIR = "outputDir"; private static final String TEST_HANDLE = "testHandle"; private static final String CLEAR_LIBRARY_CACHE = "clearLibraryCache"; - public static final String JENKINS_QUEUE_URL = "jenkinsQueueUrl"; private static final int MAX_RETRIES = 10; private final String mApiEndPoint; private final String mLogsEndpoint; @@ -299,7 +298,6 @@ public class PTestClient { options.addOption(null, OUTPUT_DIR, true, "Directory to download and save test-results.tar.gz to. (Optional for testStart)"); options.addOption(null, CLEAR_LIBRARY_CACHE, false, "Before starting the test, delete the ivy and maven directories (Optional for testStart)"); options.addOption(null, LOGS_ENDPOINT, true, "URL to get the logs"); - options.addOption(null, JENKINS_QUEUE_URL, true, "URL for quering Jenkins job queue"); CommandLine commandLine = parser.parse(options, args); @@ -322,13 +320,6 @@ public class PTestClient { TEST_HANDLE }); - boolean jiraAlreadyInQueue = JenkinsQueueUtil.isJiraAlreadyInQueue(commandLine); - if (jiraAlreadyInQueue) { - System.out.println("Skipping ptest execution, as " + commandLine.getOptionValue(JIRA) + - " is scheduled in " + "queue in " + "the future too."); - System.exit(0); - } - result = client.testStart(commandLine.getOptionValue(PROFILE), commandLine.getOptionValue(TEST_HANDLE), commandLine.getOptionValue(JIRA), commandLine.getOptionValue(PATCH), commandLine.hasOption(CLEAR_LIBRARY_CACHE)); http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java b/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java index 2868ff0..4e6aa6d 100644 --- a/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java +++ b/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java @@ -159,7 +159,8 @@ public class PTest { } mHostExecutors = new CopyOnWriteArrayList<HostExecutor>(hostExecutors); mPhases = Lists.newArrayList(); - mPhases.add(new TestCheckPhase(mHostExecutors, localCommandFactory, templateDefaults, patchFile, logger, mAddedTests)); + mPhases.add(new TestCheckPhase(mHostExecutors, localCommandFactory, templateDefaults, + configuration.getPatch(), patchFile, logger, mAddedTests)); mPhases.add(new PrepPhase(mHostExecutors, localCommandFactory, templateDefaults, scratchDir, patchFile, logger)); mPhases.add(new YetusPhase(configuration, mHostExecutors, localCommandFactory, templateDefaults, mExecutionContext.getLocalWorkingDirectory(), scratchDir, logger, logDir, patchFile)); http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/TestCheckPhase.java ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/TestCheckPhase.java b/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/TestCheckPhase.java index 1107dcd..831a909 100644 --- a/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/TestCheckPhase.java +++ b/testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/TestCheckPhase.java @@ -18,6 +18,8 @@ */ package org.apache.hive.ptest.execution; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; @@ -26,26 +28,40 @@ import java.io.File; import java.io.FileReader; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; public class TestCheckPhase extends Phase { private final File mPatchFile; + private final String mPatchURL; private Set<String> modifiedTestFiles; + private static Cache<String, Boolean> patchUrls = CacheBuilder.newBuilder().expireAfterWrite + (7, TimeUnit.DAYS).maximumSize(10000).build(); private static final Pattern fileNameFromDiff = Pattern.compile("[/][^\\s]*"); private static final Pattern javaTest = Pattern.compile("Test.*java"); public TestCheckPhase(List<HostExecutor> hostExecutors, - LocalCommandFactory localCommandFactory, - ImmutableMap<String, String> templateDefaults, - File patchFile, Logger logger, Set<String> modifiedTestFiles) { + LocalCommandFactory localCommandFactory, + ImmutableMap<String, String> templateDefaults, + String patchUrl, File patchFile, Logger logger, Set<String> modifiedTestFiles) { super(hostExecutors, localCommandFactory, templateDefaults, logger); this.mPatchFile = patchFile; + this.mPatchURL = patchUrl; this.modifiedTestFiles = modifiedTestFiles; } @Override public void execute() throws Exception { + if (mPatchURL != null) { + boolean patchUrlWasSeen = patchUrls.asMap().containsKey(mPatchURL); + if (!patchUrlWasSeen) { + patchUrls.put(mPatchURL, true); + } else { + throw new Exception("Patch URL " + mPatchURL + " was found in seen patch url's cache and " + + "a test was probably run already on it. Aborting..."); + } + } if(mPatchFile != null) { logger.info("Reading patchfile " + mPatchFile.getAbsolutePath()); FileReader fr = null; http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestTestCheckPhase.java ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestTestCheckPhase.java b/testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestTestCheckPhase.java index 7183ee3..de3386a 100644 --- a/testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestTestCheckPhase.java +++ b/testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestTestCheckPhase.java @@ -18,7 +18,6 @@ */ package org.apache.hive.ptest.execution; -import org.approvaltests.Approvals; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -28,6 +27,10 @@ import java.net.URL; import java.util.HashSet; import java.util.Set; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + public class TestTestCheckPhase extends AbstractTestPhase { private TestCheckPhase phase; @@ -42,7 +45,7 @@ public class TestTestCheckPhase extends AbstractTestPhase { File patchFile = new File(url.getFile()); Set<String> addedTests = new HashSet<String>(); phase = new TestCheckPhase(hostExecutors, localCommandFactory, - templateDefaults, patchFile, logger, addedTests); + templateDefaults, url.toString(), patchFile, logger, addedTests); phase.execute(); Assert.assertEquals(addedTests.size(), 0); @@ -55,7 +58,7 @@ public class TestTestCheckPhase extends AbstractTestPhase { File patchFile = new File(url.getFile()); Set<String> addedTests = new HashSet<String>(); phase = new TestCheckPhase(hostExecutors, localCommandFactory, - templateDefaults, patchFile, logger, addedTests); + templateDefaults, url.toString(), patchFile, logger, addedTests); phase.execute(); Assert.assertEquals(addedTests.size(), 3); @@ -70,7 +73,7 @@ public class TestTestCheckPhase extends AbstractTestPhase { File patchFile = new File(url.getFile()); Set<String> addedTests = new HashSet<String>(); phase = new TestCheckPhase(hostExecutors, localCommandFactory, - templateDefaults, patchFile, logger, addedTests); + templateDefaults, url.toString(), patchFile, logger, addedTests); phase.execute(); Assert.assertEquals(addedTests.size(), 1); @@ -83,9 +86,32 @@ public class TestTestCheckPhase extends AbstractTestPhase { File patchFile = new File(url.getFile()); Set<String> addedTests = new HashSet<String>(); phase = new TestCheckPhase(hostExecutors, localCommandFactory, - templateDefaults, patchFile, logger, addedTests); + templateDefaults, url.toString(), patchFile, logger, addedTests); phase.execute(); Assert.assertEquals(addedTests.size(), 0); } + + @Test + public void testSamePatchMultipleTimes() throws Exception { + int executions = 0; + try { + URL url = this.getClass().getResource("/HIVE-19077.1.patch"); + File patchFile = new File(url.getFile()); + Set<String> addedTests = new HashSet<String>(); + phase = new TestCheckPhase(hostExecutors, localCommandFactory, + templateDefaults, url.toString(), patchFile, logger, addedTests); + phase.execute(); + executions++; + phase = new TestCheckPhase(hostExecutors, localCommandFactory, + templateDefaults, url.toString(), patchFile, logger, addedTests); + phase.execute(); + executions++; + fail("Should've thrown exception"); + } catch (Exception ex) { + assertTrue(ex.getMessage().contains("HIVE-19077.1.patch was found in seen patch url's cache " + + "and a test was probably run already on it. Aborting...")); + } + assertEquals(1, executions); + } } http://git-wip-us.apache.org/repos/asf/hive/blob/7d7c1839/testutils/ptest2/src/test/resources/HIVE-19077.1.patch ---------------------------------------------------------------------- diff --git a/testutils/ptest2/src/test/resources/HIVE-19077.1.patch b/testutils/ptest2/src/test/resources/HIVE-19077.1.patch new file mode 100644 index 0000000..cd7b133 --- /dev/null +++ b/testutils/ptest2/src/test/resources/HIVE-19077.1.patch @@ -0,0 +1,14 @@ +diff --git a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java +index c076d4e112a7edab2106f11fe6224247887313cf..8bcb464de540eda7c14a8c6783bb19a09071af7b 100644 +--- a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java ++++ b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java +@@ -25,7 +25,9 @@ + + import org.apache.hadoop.hive.ql.exec.ColumnInfo; + import org.apache.hadoop.hive.ql.exec.CommonJoinOperator; ++import org.apache.hadoop.hive.ql.exec.FilterOperator; + import org.apache.hadoop.hive.ql.exec.Operator; ++import org.apache.hadoop.hive.ql.exec.OperatorFactory; + import org.apache.hadoop.hive.ql.exec.RowSchema; + import org.apache.hadoop.hive.ql.exec.SelectOperator; + import org.apache.hadoop.hive.ql.exec.UnionOperator;
