Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented May 26, 2021

What changes were proposed in this pull request?

This PR fixes HiveExternalCatalogVersionsSuite.
With this change, only . version is set to spark.sql.hive.metastore.version.

Why are the changes needed?

I'm personally checking whether all the tests pass with Java 11 for the current master and I found HiveExternalCatalogVersionsSuite fails.
The reason is that Spark 3.0.2 and 3.1.1 doesn't accept 2.3.8 as a hive metastore version.

HiveExternalCatalogVersionsSuite downloads Spark releases from https://dist.apache.org/repos/dist/release/spark/ and run test for each release. The Spark releases are 3.0.2 and 3.1.1 for the current master for now.

object PROCESS_TABLES extends QueryTest with SQLTestUtils {
val releaseMirror = sys.env.getOrElse("SPARK_RELEASE_MIRROR",
"https://dist.apache.org/repos/dist/release")
// Tests the latest version of every release line.
val testingVersions: Seq[String] = {
import scala.io.Source
val versions: Seq[String] = try Utils.tryWithResource(
Source.fromURL(s"$releaseMirror/spark")) { source =>
source.mkString
.split("\n")
.filter(_.contains("""<a href="spark-"""))
.filterNot(_.contains("preview"))
.map("""<a href="spark-(\d.\d.\d)/">""".r.findFirstMatchIn(_).get.group(1))
.filter(_ < org.apache.spark.SPARK_VERSION)
} catch {
// Do not throw exception during object initialization.
case NonFatal(_) => Nil
}
versions
.filter(v => v.startsWith("3") || !TestUtils.isPythonVersionAtLeast38())
.filter(v => v.startsWith("3") || !SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9))

With Java 11, the suite run with a hive metastore version which corresponds to the builtin Hive version and it's 2.3.8 for the current master.

val hiveVersion = if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)) {
HiveUtils.builtinHiveVersion
} else {
"1.2.1"
}

But branch-3.0 and branch-3.1 doesn't accept 2.3.8, the suite with Java 11 fails.

Another solution would be backporting SPARK-34271 (#31371) but after a discussion, we prefer to fix the test,

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests with CI.

@github-actions github-actions bot added the SQL label May 26, 2021
@SparkQA
Copy link

SparkQA commented May 26, 2021

Test build #138959 has finished for PR 32670 at commit 39d3d28.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM (Pending CIs). Thank you so much, @sarutak !

@SparkQA
Copy link

SparkQA commented May 26, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43479/

@SparkQA
Copy link

SparkQA commented May 26, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43480/

@SparkQA
Copy link

SparkQA commented May 26, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43479/

@SparkQA
Copy link

SparkQA commented May 26, 2021

Test build #138960 has finished for PR 32670 at commit b03940c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 26, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43480/

@HyukjinKwon
Copy link
Member

uhoh, seems test failure related 😢

@SparkQA
Copy link

SparkQA commented May 26, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43488/

@HyukjinKwon
Copy link
Member

Merged to master.

@SparkQA
Copy link

SparkQA commented May 26, 2021

Test build #138969 has finished for PR 32670 at commit a2465a1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138969/

@sarutak sarutak deleted the fix-version-suite-for-java11 branch June 4, 2021 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants