From 09fcb3eda1797d1d8c039255c7c5032cf1be9ab2 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 30 Sep 2021 12:01:51 +0800 Subject: [PATCH 01/28] Pass sql/core releated cases --- .../scala/org/apache/spark/SparkContext.scala | 2 + .../apache/spark/util/JavaModuleUtils.scala | 84 ++++ .../spark/deploy/SparkSubmitTestUtils.scala | 6 +- .../spark/launcher/LauncherBackendSuite.scala | 13 +- pom.xml | 26 +- project/SparkBuild.scala | 20 +- sql/catalyst/pom.xml | 2 +- .../util/DateTimeFormatterHelper.scala | 4 + sql/core/pom.xml | 2 +- .../results/postgreSQL/text.sql-jdk17.out | 360 ++++++++++++++++++ .../apache/spark/sql/SQLQueryTestSuite.scala | 14 +- .../WholeStageCodegenSparkSubmitSuite.scala | 6 +- 12 files changed, 524 insertions(+), 15 deletions(-) create mode 100644 core/src/main/scala/org/apache/spark/util/JavaModuleUtils.scala create mode 100644 sql/core/src/test/resources/sql-tests/results/postgreSQL/text.sql-jdk17.out diff --git a/core/src/main/scala/org/apache/spark/SparkContext.scala b/core/src/main/scala/org/apache/spark/SparkContext.scala index e27499a155223..41e61874f1a07 100644 --- a/core/src/main/scala/org/apache/spark/SparkContext.scala +++ b/core/src/main/scala/org/apache/spark/SparkContext.scala @@ -399,6 +399,8 @@ class SparkContext(config: SparkConf) extends Logging { // This should be set as early as possible. SparkContext.fillMissingMagicCommitterConfsIfNeeded(_conf) + JavaModuleUtils.supplementJava17ModuleOptsIfNeeded(_conf) + _driverLogger = DriverLogger(_conf) val resourcesFileOpt = conf.get(DRIVER_RESOURCES_FILE) diff --git a/core/src/main/scala/org/apache/spark/util/JavaModuleUtils.scala b/core/src/main/scala/org/apache/spark/util/JavaModuleUtils.scala new file mode 100644 index 0000000000000..b77ef847342b0 --- /dev/null +++ b/core/src/main/scala/org/apache/spark/util/JavaModuleUtils.scala @@ -0,0 +1,84 @@ +/* + * 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.spark.util + +import scala.collection.mutable + +import org.apache.commons.lang3.{JavaVersion, SystemUtils} + +import org.apache.spark.SparkConf +import org.apache.spark.internal.config.{DRIVER_JAVA_OPTIONS, EXECUTOR_JAVA_OPTIONS, OptionalConfigEntry} + +object JavaModuleUtils { + + private val javaModuleOptions = Set("--add-opens java.base/java.nio=ALL-UNNAMED", + "--add-opens java.base/sun.nio.ch=ALL-UNNAMED", + "--add-opens java.base/java.lang.invoke=ALL-UNNAMED", + "--add-opens java.base/java.nio=ALL-UNNAMED", + "--add-opens java.base/sun.nio.ch=ALL-UNNAMED", + "--add-opens java.base/java.lang.invoke=ALL-UNNAMED", + "--add-opens java.base/java.util=ALL-UNNAMED", + "--add-opens java.base/sun.security.action=ALL-UNNAMED", + "--add-opens java.base/sun.util.calendar=ALL-UNNAMED", + "--add-opens java.base/java.lang=ALL-UNNAMED", + "--add-opens java.base/sun.nio.cs=ALL-UNNAMED", + "--add-opens java.base/java.net=ALL-UNNAMED", + "--add-opens java.base/java.io=ALL-UNNAMED", + "--add-opens java.base/java.util.concurrent=ALL-UNNAMED", + "--add-exports java.base/jdk.internal.util.random=ALL-UNNAMED") + + def isJavaVersionAtLeast17: Boolean = SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_17) + + def defaultModuleOptions(): String = javaModuleOptions.mkString(" ", " ", " ") + + + def supplementJava17ModuleOptsIfNeeded(conf: SparkConf): Unit = { + + def supplementModuleOpts(configEntry: OptionalConfigEntry[String]): Unit = { + val v = conf.get(configEntry) match { + case Some(opts) => JavaModuleUtils.defaultModuleOptions() + opts + case None => JavaModuleUtils.defaultModuleOptions() + } + conf.set(configEntry.key, v) + } + + if (Utils.isTesting && isJavaVersionAtLeast17) { + supplementModuleOpts(DRIVER_JAVA_OPTIONS) + supplementModuleOpts(EXECUTOR_JAVA_OPTIONS) + } + } + + def supplementJava17ModuleOptsIfNeeded(args: Seq[String]): Seq[String] = { + + def supplementModuleOpts(buffer: mutable.Buffer[String], key: String): Unit = { + val index = buffer.indexWhere(_.startsWith(s"$key}=")) + if (index != -1) { + buffer.update(index, buffer(index) + JavaModuleUtils.defaultModuleOptions()) + } else { + buffer.prependAll(Seq("--conf", s"$key=${JavaModuleUtils.defaultModuleOptions()}")) + } + } + + if (Utils.isTesting && isJavaVersionAtLeast17) { + val buffer = args.toBuffer + supplementModuleOpts(buffer, DRIVER_JAVA_OPTIONS.key) + supplementModuleOpts(buffer, EXECUTOR_JAVA_OPTIONS.key) + buffer.toSeq + } else args + } +} diff --git a/core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala b/core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala index 2ab2e17df03a8..41d35be768c56 100644 --- a/core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala +++ b/core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala @@ -30,7 +30,7 @@ import org.scalatest.time.SpanSugar._ import org.apache.spark.ProcessTestUtils.ProcessOutputCapturer import org.apache.spark.SparkFunSuite -import org.apache.spark.util.Utils +import org.apache.spark.util.{JavaModuleUtils, Utils} trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits { @@ -53,7 +53,9 @@ trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits { } else { new File(new File(sparkHome, "bin"), "spark-submit") } - val commands = Seq(sparkSubmit.getCanonicalPath) ++ args + + val commands = Seq(sparkSubmit.getCanonicalPath) ++ + JavaModuleUtils.supplementJava17ModuleOptsIfNeeded(args) val commandLine = commands.mkString("'", "' '", "'") val builder = new ProcessBuilder(commands: _*).directory(new File(sparkHome)) diff --git a/core/src/test/scala/org/apache/spark/launcher/LauncherBackendSuite.scala b/core/src/test/scala/org/apache/spark/launcher/LauncherBackendSuite.scala index 473782ee28d1c..4ed50719928ad 100644 --- a/core/src/test/scala/org/apache/spark/launcher/LauncherBackendSuite.scala +++ b/core/src/test/scala/org/apache/spark/launcher/LauncherBackendSuite.scala @@ -27,7 +27,7 @@ import org.scalatest.matchers.should.Matchers._ import org.apache.spark._ import org.apache.spark.internal.config.UI.UI_ENABLED -import org.apache.spark.util.Utils +import org.apache.spark.util.{JavaModuleUtils, Utils} class LauncherBackendSuite extends SparkFunSuite with Matchers { @@ -46,7 +46,7 @@ class LauncherBackendSuite extends SparkFunSuite with Matchers { private def testWithMaster(master: String): Unit = { val env = new java.util.HashMap[String, String]() env.put("SPARK_PRINT_LAUNCH_COMMAND", "1") - val handle = new SparkLauncher(env) + val launcher = new SparkLauncher(env) .setSparkHome(sys.props("spark.test.home")) .setConf(SparkLauncher.DRIVER_EXTRA_CLASSPATH, System.getProperty("java.class.path")) .setConf(UI_ENABLED.key, "false") @@ -54,7 +54,14 @@ class LauncherBackendSuite extends SparkFunSuite with Matchers { .setMaster(master) .setAppResource(SparkLauncher.NO_RESOURCE) .setMainClass(TestApp.getClass.getName().stripSuffix("$")) - .startApplication() + + if(JavaModuleUtils.isJavaVersionAtLeast17) { + launcher.setConf(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, + s"${JavaModuleUtils.defaultModuleOptions()} -Dtest.appender=console") + } else { + launcher.setConf(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, s"-Dtest.appender=console") + } + val handle = launcher.startApplication() try { eventually(timeout(30.seconds), interval(100.milliseconds)) { diff --git a/pom.xml b/pom.xml index 72410ecfe820b..eac9c51af3558 100644 --- a/pom.xml +++ b/pom.xml @@ -208,6 +208,8 @@ ${java.home} + + org.apache.spark.tags.ChromeUITest @@ -2686,7 +2688,7 @@ **/*Suite.java ${project.build.directory}/surefire-reports - -ea -Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} -Dio.netty.tryReflectionSetAccessible=true + -ea -Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true org.apache.spark.tags.ChromeUITest @@ -285,6 +283,22 @@ 128m yyyy-MM-dd HH:mm:ss z + + + + -XX:+IgnoreUnrecognizedVMOptions + --add-opens=java.base/java.lang=ALL-UNNAMED + --add-opens=java.base/java.lang.invoke=ALL-UNNAMED + --add-opens=java.base/java.io=ALL-UNNAMED + --add-opens=java.base/java.net=ALL-UNNAMED + --add-opens=java.base/java.nio=ALL-UNNAMED + --add-opens=java.base/java.util=ALL-UNNAMED + --add-opens=java.base/java.util.concurrent=ALL-UNNAMED + --add-opens=java.base/sun.nio.ch=ALL-UNNAMED + --add-opens=java.base/sun.nio.cs=ALL-UNNAMED + --add-opens=java.base/sun.security.action=ALL-UNNAMED + --add-opens=java.base/sun.util.calendar=ALL-UNNAMED + @@ -3433,25 +3447,6 @@ - - jdk-17 - - - --add-opens java.base/java.lang=ALL-UNNAMED - --add-opens java.base/java.lang.invoke=ALL-UNNAMED - --add-opens java.base/java.io=ALL-UNNAMED - --add-opens java.base/java.net=ALL-UNNAMED - --add-opens java.base/java.nio=ALL-UNNAMED - --add-opens java.base/java.util=ALL-UNNAMED - --add-opens java.base/java.util.concurrent=ALL-UNNAMED - --add-opens java.base/sun.nio.ch=ALL-UNNAMED - --add-opens java.base/sun.nio.cs=ALL-UNNAMED - --add-opens java.base/sun.security.action=ALL-UNNAMED - --add-opens java.base/sun.util.calendar=ALL-UNNAMED - - - - -ea -Xmx5g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true From 0bb8df749355b0caff0b3aa35384f64fc542573f Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 30 Sep 2021 14:55:33 +0800 Subject: [PATCH 09/28] add commnets --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index d29e2a04e083e..bff15897a14a7 100644 --- a/pom.xml +++ b/pom.xml @@ -2754,9 +2754,9 @@ . SparkTestSuite.txt -ea -Xmx5g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true From 0861c5d72f1c4007a9282319f79b9b888ff7732f Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 30 Sep 2021 15:10:12 +0800 Subject: [PATCH 10/28] add comments --- .../spark/sql/catalyst/util/DateTimeFormatterHelper.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeFormatterHelper.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeFormatterHelper.scala index 68371ab2b0315..c5f5908639dff 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeFormatterHelper.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeFormatterHelper.scala @@ -323,6 +323,10 @@ private object DateTimeFormatterHelper { (isParsing && unsupportedLettersForParsing.contains(c))) { throw new IllegalArgumentException(s"Illegal pattern character: $c") } + // SPARK-36796: `select date_format('2018-11-17 13:33:33.333', 'B')` failed with Java 8, + // but use Java 17 will return `in the afternoon` because 'B' is used to represent + // `Pattern letters to output a day period` in Java 17 and disabled it here for + // compatibility with Java 8 behavior. for (c <- patternPart if unknownPatternLetters.contains(c)) { throw new IllegalArgumentException(s"Unknown pattern letter: $c") } From 86517e6f6bf4d88ed30cab78635c7c46d2f119c3 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 30 Sep 2021 15:32:37 +0800 Subject: [PATCH 11/28] add comments --- .../test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala index 12c512a3749da..944f945e112f8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala @@ -503,6 +503,10 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper } } + // SPARK-36796: `select format_string('%0$s', 'Hello')` in `postgreSQL/text.sql` has different + // behavior between Java 8 and Java 17, but it seems that the behavior of Java 17 is expected: + // `PostgreSQL throw ERROR: format specifies argument 0, but arguments are numbered from 1`, + // so do independent verification here. if (JavaModuleUtils.isJavaVersionAtLeast17) { val jdk17SpecialCases = Set("postgreSQL/text.sql") cases.map { From 498126083d2f2f42163b12a234ea53858bff0a8e Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 30 Sep 2021 16:03:34 +0800 Subject: [PATCH 12/28] add commnet --- .../execution/WholeStageCodegenSparkSubmitSuite.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/WholeStageCodegenSparkSubmitSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/WholeStageCodegenSparkSubmitSuite.scala index 8577112ee79dc..631479c277065 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/WholeStageCodegenSparkSubmitSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/WholeStageCodegenSparkSubmitSuite.scala @@ -47,6 +47,17 @@ class WholeStageCodegenSparkSubmitSuite extends SparkSubmitTestUtils "--driver-memory", "1g", "--conf", "spark.ui.enabled=false", "--conf", "spark.master.rest.enabled=false", + // SPARK-36796: The results of `Platform.BYTE_ARRAY_OFFSET` using different Java versions + // and different args as follows table: + // +------------------------------+--------+---------+ + // | |Java 8 |Java 17 | + // +------------------------------+--------+---------+ + // |-XX:-UseCompressedOops | 24 | 16 | + // |-XX:+UseCompressedOops | 16 | 16 | + // |-XX:-UseCompressedClassPointers| 24 | 24 | + // |-XX:+UseCompressedClassPointers| 16 | 16 | + // +-------------------------------+-------+---------+ + // So SPARK-36796 replace `UseCompressedOops` with `UseCompressedClassPointers`. "--conf", "spark.driver.extraJavaOptions=-XX:-UseCompressedClassPointers", "--conf", "spark.executor.extraJavaOptions=-XX:+UseCompressedClassPointers", "--conf", "spark.sql.adaptive.enabled=false", From 4aa36437d07c152546975c6e32278fe2b605759f Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Mon, 11 Oct 2021 11:11:07 +0800 Subject: [PATCH 13/28] resolve conflicts --- pom.xml | 11 +++-------- project/SparkBuild.scala | 4 ++-- sql/catalyst/pom.xml | 2 +- sql/core/pom.xml | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/pom.xml b/pom.xml index bff15897a14a7..1008c7ccfe767 100644 --- a/pom.xml +++ b/pom.xml @@ -2652,7 +2652,7 @@ -Xss128m -Xms4g - -Xmx4g + -Xmx6g -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} @@ -2702,7 +2702,7 @@ **/*Suite.java ${project.build.directory}/surefire-reports - -ea -Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true + -ea -Xmx6g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true - -ea -Xmx5g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true + -ea -Xmx6g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true