Ngone51 commented on code in PR #50594:
URL: https://github.com/apache/spark/pull/50594#discussion_r2062779606
##########
core/src/test/scala/org/apache/spark/util/UninterruptibleThreadSuite.scala:
##########
@@ -115,6 +116,45 @@ class UninterruptibleThreadSuite extends SparkFunSuite {
assert(interruptStatusBeforeExit)
}
+ test("no runUninterruptibly") {
+ @volatile var hasInterruptedException = false
+ val t = new UninterruptibleThread("test") {
+ override def run(): Unit = {
+ if (sleep(0)) {
+ hasInterruptedException = true
+ }
+ }
+ }
+ t.interrupt()
+ t.start()
+ t.join()
+ assert(hasInterruptedException === true)
+ }
+
+ test("SPARK-51821 uninterruptibleLock deadlock") {
+ val latch = new CountDownLatch(1)
+ val task = new UninterruptibleThread("task thread") {
+ override def run(): Unit = {
+ val channel = new AbstractInterruptibleChannel() {
+ override def implCloseChannel(): Unit = {
+ begin()
+ latch.countDown()
+ try {
+ Thread.sleep(Long.MaxValue)
+ } catch {
+ case _: InterruptedException =>
Thread.currentThread().interrupt()
+ }
+ }
+ }
+ channel.close()
Review Comment:
Yes, I think we should avoid using `UninterruptibleThread` when it is not
really needed. The problem is that executor can't distinguish the tasks for
different workloads. So we have to compromise with `UninterruptibleThread` as
the default task thread and call `runUninterruptibly` when it is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
