You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2019/09/19 17:30:57 UTC
[impala] 04/04: IMPALA-8944: Update and re-enable S3PlannerTest
This is an automated email from the ASF dual-hosted git repository.
stakiar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit feed25084a999fe0a4e7b58b5264fce5829c43e7
Author: stakiar <st...@cloudera.com>
AuthorDate: Fri Sep 13 11:58:18 2019 -0700
IMPALA-8944: Update and re-enable S3PlannerTest
Addresses several test infra issues that were preventing the
S3PlannerTest from running successfully. Disables a few tests that are
no longer working, and removes some planner checks that are no longer
applicable when running on S3. Specifically, this patch removes the
checks in PlannerTestBase#checkScanRangeLocations when running against
S3, because the planner no longer generates scan ranges; generation is
deferred to the scheduler (IMPALA-5931).
Replaces the old logic of specifying S3-specific fe/ tests with a
combination of JUnit Categories and Maven Profiles. The previous method
was broken and assumed that all S3-specific fe/ tests started with S3*.
The new approach removes that restriction and only requires S3-specific
JUnit tests to be tagged with the Java annotation
'@Category(S3Tests.class)' (entire classes or individual tests can be
tagged with the annotation).
Testing:
* Ran fe/ tests with TARGET_FILESYSTEM=s3
Change-Id: I1690b6c5346376c1111fd4845c72062cc237e0f9
Reviewed-on: http://gerrit.cloudera.org:8080/14248
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
bin/run-all-tests.sh | 2 +-
fe/pom.xml | 8 +++++++
.../org/apache/impala/planner/PlannerTest.java | 13 +++++++++++
.../org/apache/impala/planner/PlannerTestBase.java | 19 +++++++++-------
.../org/apache/impala/planner/S3PlannerTest.java | 25 ++++++++++++++++++----
.../java/org/apache/impala/planner/S3Tests.java | 25 ++++++++++++++++++++++
6 files changed, 79 insertions(+), 13 deletions(-)
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index aeb2fcb..27ffc67 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -195,7 +195,7 @@ do
MVN_ARGS=""
if [[ "${TARGET_FILESYSTEM}" == "s3" ]]; then
# When running against S3, only run the S3 frontend tests.
- MVN_ARGS="-Dtest=S3* "
+ MVN_ARGS="-Ps3-tests "
fi
if [[ "$CODE_COVERAGE" == true ]]; then
MVN_ARGS+="-DcodeCoverage"
diff --git a/fe/pom.xml b/fe/pom.xml
index 9f96474..b3e8268 100644
--- a/fe/pom.xml
+++ b/fe/pom.xml
@@ -611,6 +611,7 @@ under the License.
<reportsDirectory>${surefire.reports.dir}</reportsDirectory>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
<argLine>${surefireJacocoArg}</argLine>
+ <groups>${testcase.groups}</groups>
</configuration>
</plugin>
@@ -1264,6 +1265,13 @@ under the License.
<buildOutputDirectory>${project.build.directory}/${eclipse.output.directory}</buildOutputDirectory>
</properties>
</profile>
+
+ <profile>
+ <id>s3-tests</id>
+ <properties>
+ <testcase.groups>org.apache.impala.planner.S3Tests</testcase.groups>
+ </properties>
+ </profile>
</profiles>
<dependencyManagement>
diff --git a/fe/src/test/java/org/apache/impala/planner/PlannerTest.java b/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
index 481973e..7cabb86 100644
--- a/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
+++ b/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
@@ -29,6 +29,7 @@ import org.apache.impala.catalog.HBaseColumn;
import org.apache.impala.catalog.Type;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.RuntimeEnv;
+import org.apache.impala.datagenerator.HBaseTestDataRegionAssignment;
import org.apache.impala.service.Frontend.PlanCtx;
import org.apache.impala.testutil.TestUtils;
import org.apache.impala.testutil.TestUtils.IgnoreValueFilter;
@@ -40,6 +41,7 @@ import org.apache.impala.thrift.TQueryOptions;
import org.apache.impala.thrift.TRuntimeFilterMode;
import org.junit.Assert;
import org.junit.Assume;
+import org.junit.BeforeClass;
import org.junit.Test;
import com.google.common.base.Preconditions;
@@ -49,6 +51,17 @@ import com.google.common.collect.Lists;
// All planner tests, except for S3 specific tests should go here.
public class PlannerTest extends PlannerTestBase {
+ @BeforeClass
+ public static void setUp() throws Exception {
+ PlannerTestBase.setUp();
+ // Rebalance the HBase tables. This is necessary because some tests rely on HBase
+ // tables being arranged in a deterministic way. See IMPALA-7061 for details.
+ HBaseTestDataRegionAssignment assignment = new HBaseTestDataRegionAssignment();
+ assignment.performAssignment("functional_hbase.alltypessmall");
+ assignment.performAssignment("functional_hbase.alltypesagg");
+ assignment.close();
+ }
+
/**
* Scan node cardinality test
*/
diff --git a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
index 2dc9f81..58692a6 100644
--- a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
+++ b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
@@ -40,7 +40,6 @@ import org.apache.impala.catalog.CatalogException;
import org.apache.impala.common.FrontendTestBase;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.RuntimeEnv;
-import org.apache.impala.datagenerator.HBaseTestDataRegionAssignment;
import org.apache.impala.service.Frontend.PlanCtx;
import org.apache.impala.testutil.TestFileParser;
import org.apache.impala.testutil.TestFileParser.Section;
@@ -115,12 +114,6 @@ public class PlannerTestBase extends FrontendTestBase {
String logDir = System.getenv("IMPALA_FE_TEST_LOGS_DIR");
if (logDir == null) logDir = "/tmp";
outDir_ = Paths.get(logDir, "PlannerTest");
-
- // Rebalance the HBase tables
- HBaseTestDataRegionAssignment assignment = new HBaseTestDataRegionAssignment();
- assignment.performAssignment("functional_hbase.alltypessmall");
- assignment.performAssignment("functional_hbase.alltypesagg");
- assignment.close();
}
@Before
@@ -423,7 +416,9 @@ public class PlannerTestBase extends FrontendTestBase {
TExecRequest singleNodeExecRequest = testPlan(testCase, Section.PLAN, queryCtx.deepCopy(),
testOptions, errorLog, actualOutput);
validateTableIds(singleNodeExecRequest);
- checkScanRangeLocations(testCase, singleNodeExecRequest, errorLog, actualOutput);
+ if (scanRangeLocationsCheckEnabled()) {
+ checkScanRangeLocations(testCase, singleNodeExecRequest, errorLog, actualOutput);
+ }
checkColumnLineage(testCase, singleNodeExecRequest, errorLog, actualOutput);
checkLimitCardinality(query, singleNodeExecRequest, errorLog);
// Test distributed plan.
@@ -933,4 +928,12 @@ public class PlannerTestBase extends FrontendTestBase {
runPlannerTestFile(testFile, dbName, defaultQueryOptions(),
Collections.<PlannerTestOption>emptySet());
}
+
+ /**
+ * Returns true if {@link #checkScanRangeLocations(TestCase, TExecRequest,
+ * StringBuilder, StringBuilder)} should be run, returns false if it should not be run.
+ */
+ protected boolean scanRangeLocationsCheckEnabled() {
+ return true;
+ }
}
diff --git a/fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java b/fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java
index 364eea3..1627166 100644
--- a/fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java
+++ b/fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java
@@ -24,12 +24,15 @@ import java.net.URI;
import org.apache.hadoop.fs.Path;
import org.junit.Assert;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
+import org.junit.experimental.categories.Category;
-import org.apache.impala.planner.PlannerTestBase;
+import com.google.common.collect.ImmutableSet;
// S3 specific planner tests go here, and will run against tables in S3. These tests
// are run only when test.fs.s3a.name is set in the configuration.
+@Category(S3Tests.class)
public class S3PlannerTest extends PlannerTestBase {
// The path that will replace the value of TEST_FS_S3A_NAME in file paths.
@@ -43,7 +46,8 @@ public class S3PlannerTest extends PlannerTestBase {
* If not, then skip this test. Also remember the scheme://bucket for later.
*/
@Before
- public void setUpTest() {
+ public void setUpTest() throws Exception {
+ super.setUpTest();
String targetFs = System.getenv("TARGET_FILESYSTEM");
// Skip if the config property was not set. i.e. not running against S3.
assumeTrue(targetFs != null && targetFs.equals("s3"));
@@ -70,6 +74,7 @@ public class S3PlannerTest extends PlannerTestBase {
/**
* Verify that S3 scan ranges are generated correctly.
*/
+ @Ignore("IMPALA-8944, IMPALA-5931")
@Test
public void testS3ScanRanges() {
runPlannerTestFile("s3");
@@ -105,6 +110,7 @@ public class S3PlannerTest extends PlannerTestBase {
runPlannerTestFile("nested-collections");
}
+ @Ignore("IMPALA-8949")
@Test
public void testJoinOrder() {
runPlannerTestFile("join-order");
@@ -125,6 +131,7 @@ public class S3PlannerTest extends PlannerTestBase {
runPlannerTestFile("inline-view-limit");
}
+ @Ignore("IMPALA-8949")
@Test
public void testSubqueryRewrite() {
runPlannerTestFile("subquery-rewrite");
@@ -155,17 +162,27 @@ public class S3PlannerTest extends PlannerTestBase {
runPlannerTestFile("data-source-tables");
}
+ @Ignore("IMPALA-8949")
@Test
public void testTpch() {
- runPlannerTestFile("tpch-all");
+ runPlannerTestFile("tpch-all", "tpch",
+ ImmutableSet.of(PlannerTestOption.INCLUDE_RESOURCE_HEADER,
+ PlannerTestOption.VALIDATE_RESOURCES));
}
+ @Ignore("IMPALA-8949")
@Test
public void testTpcds() {
// Uses ss_sold_date_sk as the partition key of store_sales to allow static partition
// pruning. The original predicates were rephrased in terms of the ss_sold_date_sk
// partition key, with the query semantics identical to the original queries.
- runPlannerTestFile("tpcds-all", "tpcds");
+ runPlannerTestFile("tpcds-all", "tpcds",
+ ImmutableSet.of(PlannerTestOption.INCLUDE_RESOURCE_HEADER,
+ PlannerTestOption.VALIDATE_RESOURCES));
}
+ @Override
+ public boolean scanRangeLocationsCheckEnabled() {
+ return false;
+ }
}
diff --git a/fe/src/test/java/org/apache/impala/planner/S3Tests.java b/fe/src/test/java/org/apache/impala/planner/S3Tests.java
new file mode 100644
index 0000000..59b37d4
--- /dev/null
+++ b/fe/src/test/java/org/apache/impala/planner/S3Tests.java
@@ -0,0 +1,25 @@
+// 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.impala.planner;
+
+/**
+ * JUnit category marker for S3 tests. Mark any S3 specific fe/ tests with this category.
+ * Any tests that require the TARGET_FILESYSTEM=s3 should be in this category. When
+ * TARGET_FILESYSTEM=s3, bin/run-all-tests.sh will only run tests in this category.
+ */
+public interface S3Tests {}