Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-28703][SQL][TEST] Skip HiveExternalCatalogVersionsSuite and 3 tests in HiveSparkSubmitSuite at JDK9+ #25426

Closed
wants to merge 2 commits into from
Closed

[SPARK-28703][SQL][TEST] Skip HiveExternalCatalogVersionsSuite and 3 tests in HiveSparkSubmitSuite at JDK9+ #25426

wants to merge 2 commits into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Aug 13, 2019 •
edited by dongjoon-hyun

What changes were proposed in this pull request?

This PR skip more test when testing with JAVA_9 or later:

  1. Skip HiveExternalCatalogVersionsSuite when testing with JAVA_9 or later because our previous version does not support JAVA_9 or later.

  2. Skip 3 tests in HiveSparkSubmitSuite because the spark.sql.hive.metastore.version of these tests is lower than 2.0, however Datanucleus 3.x seem does not support JAVA_9 or later. Hive upgrade Datanucleus to 4.x from Hive 2.0(HIVE-6113):

[info]   Cause: org.datanucleus.exceptions.NucleusException: The java type java.lang.Long (jdbc-type="", sql-type="") cant be mapped for this datastore. No mapping is available.
[info]   at org.datanucleus.store.rdbms.mapping.RDBMSMappingManager.getDatastoreMappingClass(RDBMSMappingManager.java:1215)
[info]   at org.datanucleus.store.rdbms.mapping.RDBMSMappingManager.createDatastoreMapping(RDBMSMappingManager.java:1378)
[info]   at org.datanucleus.store.rdbms.table.AbstractClassTable.addDatastoreId(AbstractClassTable.java:392)
[info]   at org.datanucleus.store.rdbms.table.ClassTable.initializePK(ClassTable.java:1087)
[info]   at org.datanucleus.store.rdbms.table.ClassTable.preInitialize(ClassTable.java:247)

Please note that this exclude only the tests related to the old metastore library, some other tests of HiveSparkSubmitSuite still fail on JDK9+.

How was this patch tested?

manual tests:

Test with JDK 11:

[info] HiveExternalCatalogVersionsSuite:
[info] - backward compatibility !!! CANCELED !!! (37 milliseconds)

[info] HiveSparkSubmitSuite:
...
[info] - SPARK-8020: set sql conf in spark conf !!! CANCELED !!! (30 milliseconds)
[info]   org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast(JAVA_9) was true (HiveSparkSubmitSuite.scala:130)
...
[info] - SPARK-9757 Persist Parquet relation with decimal column !!! CANCELED !!! (1 millisecond)
[info]   org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast(JAVA_9) was true (HiveSparkSubmitSuite.scala:168)
...
[info] - SPARK-16901: set javax.jdo.option.ConnectionURL !!! CANCELED !!! (1 millisecond)
[info]   org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast(JAVA_9) was true (HiveSparkSubmitSuite.scala:260)
...

dongjoon-hyun
dongjoon-hyun reviewed Aug 13, 2019
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala Outdated
test("backward compatibility") {
// TODO Test backward compatibility on JAVA_9 once we have a version supports JAVA_9
Copy link
Member

Choose a reason for hiding this comment

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

Please file a JIRA and make this IDed TODO, @wangyum .
Otherwise, it's very difficult for the other contributors to pick up this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.
SPARK-28704: Test backward compatibility on JDK9+ once we have a version supports JDK9+

dongjoon-hyun
dongjoon-hyun reviewed Aug 13, 2019
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSparkSubmitSuite.scala
@@ -163,6 +165,7 @@ class HiveSparkSubmitSuite
}

test("SPARK-9757 Persist Parquet relation with decimal column") {
assume(!SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9))
Copy link
Member

Choose a reason for hiding this comment

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

We don't have spark.sql.hive.metastore.version=0.12 in this test case. Could you add more explanation about this into the PR description?

spark.sql.hive.metastore.version of these tests is lower than 2.0

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Lines 674 to 685 in d7b7a1a

object SPARK_9757 extends QueryTest {
import org.apache.spark.sql.functions._
protected var spark: SparkSession = _
def main(args: Array[String]): Unit = {
TestUtils.configTestLog4j("INFO")
val hiveWarehouseLocation = Utils.createTempDir()
val sparkContext = new SparkContext(
new SparkConf()
.set(HiveUtils.HIVE_METASTORE_VERSION.key, "0.13.1")

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Got it!

@dongjoon-hyun
Copy link
Member

LGTM except two comments, @wangyum .

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-28703][SQL][TEST] Skip test HiveExternalCatalogVersionsSuite and skip 3 tests in HiveSparkSubmitSuite when testing with JAVA_9 or later [SPARK-28703][SQL][TEST] Skip HiveExternalCatalogVersionsSuite and 3 tests in HiveSparkSubmitSuite at JDK9+ Aug 13, 2019
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Aug 13, 2019 •
edited

@wangyum . This PR works correctly, but the some of the remaining test cases in HiveSparkSubmitSuite still fail. Is this expected? Do we have another JIRA issue for HiveSparkSubmitSuite after this PR?

[info] *** 8 TESTS FAILED ***
[error] Failed: Total 11, Failed 8, Errors 0, Passed 3, Canceled 3

dongjoon-hyun
dongjoon-hyun approved these changes Aug 13, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment •
edited

Choose a reason for hiding this comment

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

+1, LGTM. This is tested locally with both JDK8/JDK11.
As the PR description says, this PR skips old metastore-related test cases. Although HiveSparkSubmitSuite still fail on JDK9+, this completes the scope and the goal of this PR.

Thank you, @wangyum .

Merged to master.

@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109009 has finished for PR 25426 at commit d7b7a1a.

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

@wangyum
Copy link
Member Author

wangyum commented Aug 13, 2019

Thank you @dongjoon-hyun We need another fix to fix the classLoader. After that:

[info] Run completed in 3 minutes, 8 seconds.
[info] Total number of tests run: 11
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 11, failed 0, canceled 3, ignored 0, pending 0
[info] All tests passed.

@wangyum wangyum deleted the SPARK-28703 branch August 13, 2019 04:54
@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109010 has finished for PR 25426 at commit 23c7210.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees
No one assigned
Labels
SQL TESTS
Projects
None yet
Milestone
No milestone
3 participants
@wangyum @dongjoon-hyun @SparkQA