SPARK-5841: remove DiskBlockManager shutdown hook on stop

[MattWhelan]

After a call to stop, the shutdown hook is redundant, and causes a
memory leak.

[AmplabJenkins]

Can one of the admins verify this patch?

[srowen]

LGTM

[srowen]

ok to test

[SparkQA]

Test build #27565 has started for PR 4627 at commit d5f5c7f.

• This patch merges cleanly.

[SparkQA]

Test build #27565 has finished for PR 4627 at commit d5f5c7f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class ParquetRelation2(

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27565/
Test PASSed.

[JoshRosen]

It looks like removeShutdownHook here can get called during shutdown, which leads to some exceptions that might prevent other cleanup from taking place:

Exception in thread "delete Spark local dirs" java.lang.IllegalStateException: Shutdown in progress
        at java.lang.ApplicationShutdownHooks.remove(ApplicationShutdownHooks.java:82)
        at java.lang.Runtime.removeShutdownHook(Runtime.java:239)
        at org.apache.spark.storage.DiskBlockManager.stop(DiskBlockManager.scala:151)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply$mcV$sp(DiskBlockManager.scala:141)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply(DiskBlockManager.scala:139)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply(DiskBlockManager.scala:139)
        at org.apache.spark.util.Utils$.logUncaughtExceptions(Utils.scala:1613)
        at org.apache.spark.storage.DiskBlockManager$$anon$1.run(DiskBlockManager.scala:139)
Exception in thread "delete Spark local dirs" java.lang.IllegalStateException: Shutdown in progress
        at java.lang.ApplicationShutdownHooks.remove(ApplicationShutdownHooks.java:82)
        at java.lang.Runtime.removeShutdownHook(Runtime.java:239)
        at org.apache.spark.storage.DiskBlockManager.stop(DiskBlockManager.scala:151)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply$mcV$sp(DiskBlockManager.scala:141)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply(DiskBlockManager.scala:139)
        at org.apache.spark.storage.DiskBlockManager$$anon$1$$anonfun$run$1.apply(DiskBlockManager.scala:139)
        at org.apache.spark.util.Utils$.logUncaughtExceptions(Utils.scala:1613)
        at org.apache.spark.storage.DiskBlockManager$$anon$1.run(DiskBlockManager.scala:139)

@srowen @MattWhelan Can you fix this or file a JIRA?

[srowen]

Ah, how did you trigger that BTW? Yes I will help get it patched. I imagine the stop logic must be factored into a method called both by the hook and by stop, which can still unregister the hook first.

[JoshRosen]

I saw it during shutdown from a failed Jenkins test run: https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-pre-YARN/hadoop.version=1.0.4,label=centos/1616/console

[MattWhelan]

Kinda weird that it would pass sometimes and fail others. I'll submit a fix tomorrow.

[pwendell]

@JoshRosen should we revert this in 1.3 then? I might create a release candidate soon to kick off community testing.

[JoshRosen]

@pwendell I suppose we could revert. A bandaid patch would be to just wrap this in a try block and ignore exceptions thrown when removing the hook (see also: Utils.inShutdown()). What do you think about just hotfixing in a Try?

[srowen]

@pwendell @JoshRosen @MattWhelan Let me propose a 'real' fix I think. This should be possible to make correct with a few more lines of code.