This is an automated email from the ASF dual-hosted git repository. dongjoon pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push: new d49780037b51 [SPARK-45299][TESTS] Remove JDK 8 workaround in UtilsSuite d49780037b51 is described below commit d49780037b5169d478a505ce9e637234f3eadb67 Author: Hyukjin Kwon <gurwls...@apache.org> AuthorDate: Mon Sep 25 20:17:43 2023 -0700 [SPARK-45299][TESTS] Remove JDK 8 workaround in UtilsSuite ### What changes were proposed in this pull request? This PR removes the legacy workaround for JDK 7 and below at SPARK-12486. The main code was cleaned up at SPARK-16182 but the test code was not cleaned up. ### Why are the changes needed? To remove legacy workaround. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Fixed unittests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43084 from HyukjinKwon/SPARK-45299. Lead-authored-by: Hyukjin Kwon <gurwls...@apache.org> Co-authored-by: Hyukjin Kwon <gurwls...@gmail.com> Signed-off-by: Dongjoon Hyun <dh...@apple.com> --- .../scala/org/apache/spark/util/UtilsSuite.scala | 75 ++++++++++------------ 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala index 2a91b45ef5b7..58ce15cfaf81 100644 --- a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala @@ -32,7 +32,7 @@ import scala.util.Random import com.google.common.io.Files import org.apache.commons.io.IOUtils -import org.apache.commons.lang3.{JavaVersion, SystemUtils} +import org.apache.commons.lang3.SystemUtils import org.apache.commons.math3.stat.inference.ChiSquareTest import org.apache.hadoop.conf.Configuration import org.apache.hadoop.fs.Path @@ -983,19 +983,13 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { // Verify that we can terminate a process even if it is in a bad state. This is only run // on UNIX since it does some OS specific things to verify the correct behavior. if (SystemUtils.IS_OS_UNIX) { - def getPid(p: Process): Int = { - val f = p.getClass().getDeclaredField("pid") - f.setAccessible(true) - f.get(p).asInstanceOf[Int] - } - - def pidExists(pid: Int): Boolean = { + def pidExists(pid: Long): Boolean = { val p = Runtime.getRuntime.exec(Array("kill", "-0", s"$pid")) p.waitFor() p.exitValue() == 0 } - def signal(pid: Int, s: String): Unit = { + def signal(pid: Long, s: String): Unit = { val p = Runtime.getRuntime.exec(Array("kill", s"-$s", s"$pid")) p.waitFor() } @@ -1003,8 +997,8 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { // Start up a process that runs 'sleep 10'. Terminate the process and assert it takes // less time and the process is no longer there. val startTimeNs = System.nanoTime() - val process = new ProcessBuilder("sleep", "10").start() - val pid = getPid(process) + var process = new ProcessBuilder("sleep", "10").start() + var pid = process.toHandle.pid() try { assert(pidExists(pid)) val terminated = Utils.terminateProcess(process, 5000) @@ -1018,37 +1012,34 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { signal(pid, "SIGKILL") } - if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_1_8)) { - // We'll make sure that forcibly terminating a process works by - // creating a very misbehaving process. It ignores SIGTERM and has been SIGSTOPed. On - // older versions of java, this will *not* terminate. - val file = File.createTempFile("temp-file-name", ".tmp") - file.deleteOnExit() - val cmd = - s""" - |#!/usr/bin/env bash - |trap "" SIGTERM - |sleep 10 - """.stripMargin - Files.write(cmd.getBytes(UTF_8), file) - file.getAbsoluteFile.setExecutable(true) - - val process = new ProcessBuilder(file.getAbsolutePath).start() - val pid = getPid(process) - assert(pidExists(pid)) - try { - signal(pid, "SIGSTOP") - val startNs = System.nanoTime() - val terminated = Utils.terminateProcess(process, 5000) - assert(terminated.isDefined) - process.waitFor(5, TimeUnit.SECONDS) - val duration = System.nanoTime() - startNs - // add a little extra time to allow a force kill to finish - assert(duration < TimeUnit.SECONDS.toNanos(6)) - assert(!pidExists(pid)) - } finally { - signal(pid, "SIGKILL") - } + // We'll make sure that forcibly terminating a process works by + // creating a very misbehaving process. It ignores SIGTERM and has been SIGSTOPed. On + // older versions of java, this will *not* terminate. + val file = File.createTempFile("temp-file-name", ".tmp") + file.deleteOnExit() + val cmd = + s""" + |#!/usr/bin/env bash + |trap "" SIGTERM + |sleep 10 + """.stripMargin + Files.write(cmd.getBytes(UTF_8), file) + file.getAbsoluteFile.setExecutable(true) + + process = new ProcessBuilder(file.getAbsolutePath).start() + pid = process.toHandle.pid() + try { + signal(pid, "SIGSTOP") + val startNs = System.nanoTime() + val terminated = Utils.terminateProcess(process, 5000) + assert(terminated.isDefined) + process.waitFor(5, TimeUnit.SECONDS) + val duration = System.nanoTime() - startNs + // add a little extra time to allow a force kill to finish + assert(duration < TimeUnit.SECONDS.toNanos(6)) + assert(!pidExists(pid)) + } finally { + signal(pid, "SIGKILL") } } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org