[SPARK-5437] Fix DriverSuite and SparkSubmitSuite timeout issues #4230
Conversation
|
Test build #26181 has started for PR 4230 at commit
|
|
Test build #26181 has finished for PR 4230 at commit
|
|
Test PASSed. |
andrewor14
changed the title
This ensures two things: (1) both suites use timeouts, and (2) both suites actually terminate the respective processes if timeouts occur.
|
Test build #26188 has started for PR 4230 at commit
|
|
Test build #26188 has finished for PR 4230 at commit
|
|
Test PASSed. |
| @@ -28,31 +28,30 @@ import org.apache.spark.util.Utils | |||
|
|
|||
| class DriverSuite extends FunSuite with Timeouts { | |||
|
|
|||
| // Regression test for SPARK-530: "Spark driver process doesn't exit after finishing" | |||
There was a problem hiding this comment.
Since you're touching this, I kinda like the approach of adding the bug number to the test() call that I've seen in other places.
There was a problem hiding this comment.
I just moved this line, but I can move it there too
|
LGTM. |
|
Merging into master |
| Map("SPARK_TESTING" -> "1", "SPARK_HOME" -> sparkHome)) | ||
| failAfter(60 seconds) { process.waitFor() } | ||
| // Ensure we still kill the process in case it timed out | ||
| process.destroy() |
There was a problem hiding this comment.
@andrewor14 shouldn't the destroy be in a finally block? Otherwise the process isn't destroyed if you hit the timeout.
(I know this was committed a while back, and the test is even ignored now -- but I was looking at some flaky tests and this just happened to catch my eye, so I was curious)
There was a problem hiding this comment.
yes, it would be good to fix it, though I don't think it will solve the flakiness that we currently encounter
There was a problem hiding this comment.
by the way I'm addressing this at #6886
In DriverSuite, we currently set a timeout of 60 seconds. If after this time the process has not terminated, we leak the process because we never destroy it.
In SparkSubmitSuite, we currently do not have a timeout so the test can hang indefinitely.