Skip to content

Commit

Permalink
[SPARK-33705][SQL][TEST] Fix HiveThriftHttpServerSuite flakiness
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?
TO FIX flaky tests:

https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132345/testReport/
```
org.apache.spark.sql.hive.thriftserver.HiveThriftHttpServerSuite.JDBC query execution
org.apache.spark.sql.hive.thriftserver.HiveThriftHttpServerSuite.Checks Hive version
org.apache.spark.sql.hive.thriftserver.HiveThriftHttpServerSuite.SPARK-24829 Checks cast as float
```

The root cause here is a jar conflict issue.
`NewCookie.isHttpOnly` is not defined in the `jsr311-api.jar` which conflicts
The transitive artifact `jsr311-api.jar` of `hadoop-client` is excluded at the maven side. See https://issues.apache.org/jira/browse/SPARK-27179.

The Jenkins PR builder and Github Action use `SBT` as the compiler tool.

First, the exclusion rule from maven is not followed by sbt, so I was able to see `jsr311-api.jar` from maven cache to be added to the classpath directly. **This seems to be a  bug of `sbt-pom-reader` plugin but I'm not that sure.**

Then I added an `ExcludeRule` for the `hive-thriftserver` module at the SBT side and did see the `jsr311-api.jar` gone, but the CI jobs still failed with the same error.

I added a trace log in ThriftHttpServlet

```s
ERROR ThriftHttpServlet: !!!!!!!!! Suspect???????? --->
file:/home/jenkins/workspace/SparkPullRequestBuilder/assembly/target/scala-2.12/jars/jsr311-api-1.1.1.jar
```
And the log pointed out that the assembly phase copied it to `assembly/target/scala-2.12/jars/` which will be added to the classpath too. With the help of SBT `dependencyTree` tool, I saw the `jsr311-api` again as a transitive of `jersery-core` from `yarn` module with a `test` scope. So **This seems to be another bug from the SBT side of the `sbt-assembly` plugin.**  It copied a test scope transitive artifact to the assembly output.

In this PR, I defined some rules in SparkBuild.scala to bypass the potential bugs from the SBT side.

First, exclude the `jsr311` from all over the project and then add it back separately to the YARN module for SBT.

Additionally, the HiveThriftServerSuites was reflected for reducing flakiness too, but not related to the bugs I have found so far.

### Why are the changes needed?

fix test here

### Does this PR introduce _any_ user-facing change?

NO
### How was this patch tested?

passing jenkins and ga

Closes #30643 from yaooqinn/HiveThriftHttpServerSuite.

Authored-by: Kent Yao <yaooqinn@hotmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
yaooqinn authored and cloud-fan committed Dec 14, 2020
1 parent b135db3 commit 4d47ac4
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 170 deletions.
2 changes: 1 addition & 1 deletion LICENSE-binary
Expand Up @@ -521,7 +521,6 @@ Common Development and Distribution License (CDDL) 1.1
------------------------------------------------------

javax.el:javax.el-api https://javaee.github.io/uel-ri/
javax.servlet:javax.servlet-api https://javaee.github.io/servlet-spec/
javax.servlet.jsp:jsp-api
javax.transaction:jta http://www.oracle.com/technetwork/java/index.html
javax.xml.bind:jaxb-api https://github.com/javaee/jaxb-v2
Expand Down Expand Up @@ -553,6 +552,7 @@ Eclipse Public License (EPL) 2.0
--------------------------------

jakarta.annotation:jakarta-annotation-api https://projects.eclipse.org/projects/ee4j.ca
jakarta.servlet:jakarta.servlet-api https://projects.eclipse.org/projects/ee4j.servlet
jakarta.ws.rs:jakarta.ws.rs-api https://github.com/eclipse-ee4j/jaxrs-api
org.glassfish.hk2.external:jakarta.inject

Expand Down
6 changes: 3 additions & 3 deletions core/pom.xml
Expand Up @@ -161,9 +161,9 @@
<scope>compile</scope>
</dependency>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<version>${javaxservlet.version}</version>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>${jakartaservlet.version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
Expand Down
2 changes: 1 addition & 1 deletion dev/deps/spark-deps-hadoop-2.7-hive-2.3
Expand Up @@ -119,14 +119,14 @@ jackson-xc/1.9.13//jackson-xc-1.9.13.jar
jakarta.activation-api/1.2.1//jakarta.activation-api-1.2.1.jar
jakarta.annotation-api/1.3.5//jakarta.annotation-api-1.3.5.jar
jakarta.inject/2.6.1//jakarta.inject-2.6.1.jar
jakarta.servlet-api/4.0.3//jakarta.servlet-api-4.0.3.jar
jakarta.validation-api/2.0.2//jakarta.validation-api-2.0.2.jar
jakarta.ws.rs-api/2.1.6//jakarta.ws.rs-api-2.1.6.jar
jakarta.xml.bind-api/2.3.2//jakarta.xml.bind-api-2.3.2.jar
janino/3.0.16//janino-3.0.16.jar
javassist/3.25.0-GA//javassist-3.25.0-GA.jar
javax.inject/1//javax.inject-1.jar
javax.jdo/3.2.0-m3//javax.jdo-3.2.0-m3.jar
javax.servlet-api/3.1.0//javax.servlet-api-3.1.0.jar
javolution/5.5.1//javolution-5.5.1.jar
jaxb-api/2.2.2//jaxb-api-2.2.2.jar
jaxb-runtime/2.3.2//jaxb-runtime-2.3.2.jar
Expand Down
1 change: 1 addition & 0 deletions dev/deps/spark-deps-hadoop-3.2-hive-2.3
Expand Up @@ -118,6 +118,7 @@ jackson-module-scala_2.12/2.11.4//jackson-module-scala_2.12-2.11.4.jar
jakarta.activation-api/1.2.1//jakarta.activation-api-1.2.1.jar
jakarta.annotation-api/1.3.5//jakarta.annotation-api-1.3.5.jar
jakarta.inject/2.6.1//jakarta.inject-2.6.1.jar
jakarta.servlet-api/4.0.3//jakarta.servlet-api-4.0.3.jar
jakarta.validation-api/2.0.2//jakarta.validation-api-2.0.2.jar
jakarta.ws.rs-api/2.1.6//jakarta.ws.rs-api-2.1.6.jar
jakarta.xml.bind-api/2.3.2//jakarta.xml.bind-api-2.3.2.jar
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -138,7 +138,7 @@
<parquet.version>1.10.1</parquet.version>
<orc.version>1.6.6</orc.version>
<jetty.version>9.4.28.v20200408</jetty.version>
<javaxservlet.version>3.1.0</javaxservlet.version>
<jakartaservlet.version>4.0.3</jakartaservlet.version>
<chill.version>0.9.5</chill.version>
<ivy.version>2.4.0</ivy.version>
<oro.version>2.0.8</oro.version>
Expand Down
27 changes: 26 additions & 1 deletion project/SparkBuild.scala
Expand Up @@ -395,6 +395,8 @@ object SparkBuild extends PomBuild {

enable(KubernetesIntegrationTests.settings)(kubernetesIntegrationTests)

enable(YARN.settings)(yarn)

/**
* Adds the ability to run the spark shell directly from SBT without building an assembly
* jar.
Expand Down Expand Up @@ -654,7 +656,21 @@ object DependencyOverrides {
*/
object ExcludedDependencies {
lazy val settings = Seq(
libraryDependencies ~= { libs => libs.filterNot(_.name == "groovy-all") }
libraryDependencies ~= { libs => libs.filterNot(_.name == "groovy-all") },
// SPARK-33705: Due to sbt compiler issues, it brings exclusions defined in maven pom back to
// the classpath directly and assemble test scope artifacts to assembly/target/scala-xx/jars,
// which is also will be added to the classpath of some unit tests that will build a subprocess
// to run `spark-submit`, e.g. HiveThriftServer2Test.
//
// These artifacts are for the jersey-1 API but Spark use jersey-2 ones, so it cause test
// flakiness w/ jar conflicts issues.
//
// Also jersey-1 is only used by yarn module(see resource-managers/yarn/pom.xml) for testing
// purpose only. Here we exclude them from the whole project scope and add them w/ yarn only.
excludeDependencies ++= Seq(
ExclusionRule(organization = "com.sun.jersey"),
ExclusionRule("javax.servlet", "javax.servlet-api"),
ExclusionRule("javax.ws.rs", "jsr311-api"))
)
}

Expand Down Expand Up @@ -758,6 +774,15 @@ object Hive {
)
}

object YARN {
lazy val settings = Seq(
excludeDependencies --= Seq(
ExclusionRule(organization = "com.sun.jersey"),
ExclusionRule("javax.servlet", "javax.servlet-api"),
ExclusionRule("javax.ws.rs", "jsr311-api"))
)
}

object Assembly {
import sbtassembly.AssemblyUtils._
import sbtassembly.AssemblyPlugin.autoImport._
Expand Down
7 changes: 0 additions & 7 deletions resource-managers/yarn/pom.xml
Expand Up @@ -88,13 +88,6 @@
<artifactId>hadoop-client</artifactId>
</dependency>

<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>4.0.3</version>
<scope>test</scope>
</dependency>

<!-- Explicit listing of transitive deps that are shaded. Otherwise, odd compiler crashes. -->
<dependency>
<groupId>com.google.guava</groupId>
Expand Down

0 comments on commit 4d47ac4

Please sign in to comment.