[SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partition.mode for HiveSQLInsertTestSuite to avoid flakiness #30843

yaooqinn · 2020-12-18T16:18:00Z

What changes were proposed in this pull request?

We need to set spark.conf.set("hive.exec.dynamic.partition.mode", "nonstrict") before executing this suite; otherwise, test("insert with column list - follow table output order + partitioned table") will fail.
The reason why it does not fail because some test cases [running before this suite] do not change the default value of hive.exec.dynamic.partition.mode back to strict. However, the order of test suite execution is not deterministic.

Why are the changes needed?

avoid flakiness in tests

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

…ion.mode for Hive related TestSuites to avoid flakiness

yaooqinn · 2020-12-18T16:18:38Z

cc @cloud-fan @gatorsmile @maropu thanks

SparkQA · 2020-12-18T17:02:42Z

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

SparkQA · 2020-12-18T17:37:35Z

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

SparkQA · 2020-12-18T18:19:13Z

Test build #133024 has finished for PR 30843 at commit a5b3846.

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

HyukjinKwon · 2020-12-19T05:58:50Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSQLInsertTestSuite.scala

@@ -21,5 +21,23 @@ import org.apache.spark.sql.SQLInsertTestSuite
 import org.apache.spark.sql.hive.test.TestHiveSingleton

 class HiveSQLInsertTestSuite extends SQLInsertTestSuite with TestHiveSingleton {
+
+  private var originalPartitionMode = ""


I think you can just use null instead of an empty string.

private var originalPartitionMode: String = _

here and

spark.conf.get("hive.exec.dynamic.partition.mode", null)

later.

You can also do something like:

Option(spark.conf.get("hive.exec.dynamic.partition.mode", null))

too. and later:

originalPartitionMode .map(v => spark.conf.set("hive.exec.dynamic.partition.mode", v) .getOrElse(spark.conf.unset("hive.exec.dynamic.partition.mode"))

up to you

I forked this from HiveCharVarchar Test Suite. Shall we change there
too

No in this context because this is a follow-up of SPARK-32976. To touch other test suite, we need to make another JIRA.

dongjoon-hyun

+1, LGTM. Merged to master/3.1.

…ion.mode for HiveSQLInsertTestSuite to avoid flakiness ### What changes were proposed in this pull request? As #29893 (comment) mentioned: > We need to set spark.conf.set("hive.exec.dynamic.partition.mode", "nonstrict") before executing this suite; otherwise, test("insert with column list - follow table output order + partitioned table") will fail. The reason why it does not fail because some test cases [running before this suite] do not change the default value of hive.exec.dynamic.partition.mode back to strict. However, the order of test suite execution is not deterministic. ### Why are the changes needed? avoid flakiness in tests ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests Closes #30843 from yaooqinn/SPARK-32976-F. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit dd44ba5) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

yaooqinn · 2020-12-19T16:02:58Z

Er... Shall we wait for the Jenkins or GA? 😁

dongjoon-hyun · 2020-12-19T16:12:32Z

Oh.. My bad.

dongjoon-hyun · 2020-12-19T16:14:39Z

Thanks. I double-checked it by manually verifying quickly.

$ build/sbt -Phive "hive/testOnly *.HiveSQLInsertTestSuite"
...
[info] HiveSQLInsertTestSuite:
...
[info] Run completed in 21 seconds, 835 milliseconds.
[info] Total number of tests run: 8
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 8, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[info] Passed: Total 8, Failed 0, Errors 0, Passed 8
[success] Total time: 256 s (04:16), completed Dec 19, 2020, 8:13:26 AM

SparkQA · 2020-12-19T16:22:07Z

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

SparkQA · 2020-12-19T17:02:22Z

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

SparkQA · 2020-12-19T17:51:11Z

Test build #133074 has finished for PR 30843 at commit bafdbe2.

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

cloud-fan · 2020-12-21T08:23:28Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSQLInsertTestSuite.scala

@@ -21,5 +21,20 @@ import org.apache.spark.sql.SQLInsertTestSuite
 import org.apache.spark.sql.hive.test.TestHiveSingleton

 class HiveSQLInsertTestSuite extends SQLInsertTestSuite with TestHiveSingleton {
+
+  private val originalPartitionMode = spark.conf.getOption("hive.exec.dynamic.partition.mode")


in sql/core, the SparkSession is created for each test suite, and we can't use spark in the class constructor. In sql/hive it's OK as all test suites share one global SparkSession, but let's avoid using this pattern that only works in sql/hive.

Oh right, this should be avoided

[SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partit…

a5b3846

…ion.mode for Hive related TestSuites to avoid flakiness

github-actions bot added the SQL label Dec 18, 2020

yaooqinn changed the title ~~[SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partition.mode for Hive related TestSuites to avoid flakiness~~ [SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partition.mode for HiveSQLInsertTestSuite to avoid flakiness Dec 18, 2020

HyukjinKwon reviewed Dec 19, 2020

View reviewed changes

address comments

bafdbe2

dongjoon-hyun approved these changes Dec 19, 2020

View reviewed changes

dongjoon-hyun closed this in dd44ba5 Dec 19, 2020

cloud-fan reviewed Dec 21, 2020

View reviewed changes

[SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partition.mode for HiveSQLInsertTestSuite to avoid flakiness #30843

[SPARK-32976][SQL][FOLLOWUP] SET and RESTORE hive.exec.dynamic.partition.mode for HiveSQLInsertTestSuite to avoid flakiness #30843

Uh oh!

Conversation

yaooqinn commented Dec 18, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

yaooqinn commented Dec 18, 2020

Uh oh!

SparkQA commented Dec 18, 2020

Uh oh!

SparkQA commented Dec 18, 2020

Uh oh!

SparkQA commented Dec 18, 2020

Uh oh!

HyukjinKwon Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

yaooqinn Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

yaooqinn commented Dec 19, 2020

Uh oh!

dongjoon-hyun commented Dec 19, 2020

Uh oh!

dongjoon-hyun commented Dec 19, 2020

Uh oh!

SparkQA commented Dec 19, 2020

Uh oh!

SparkQA commented Dec 19, 2020

Uh oh!

SparkQA commented Dec 19, 2020

Uh oh!

cloud-fan Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

yaooqinn Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!