SPARK-2893: Do not swallow Exceptions when running a custom kryo registrator

[GrahamDennis]

The previous behaviour of swallowing ClassNotFound exceptions when running a custom Kryo registrator could lead to difficult to debug problems later on at serialisation / deserialisation time, see SPARK-2878. Instead it is better to fail fast.

Added test case.

[AmplabJenkins]

Can one of the admins verify this patch?

[GrahamDennis]

Jenkins, test this please.

[GrahamDennis]

Jenkins, retest this please.

[ash211]

This loses the stacktrace on exception e -- can we include that in the second parameter of the SparkException as the cause?

[ash211]

@GrahamDennis only people on the whitelist can trigger a jenkins test -- you need an admin to trigger this test for you and/or put you on the whitelist

[GrahamDennis]

Fixed the exception message.

So that's why the tests weren't running -- Thanks @ash211!

[rxin]

small typo : nonexistent

[rxin]

can you change this to

    // Allow the user to register their own classes by setting spark.kryo.registrator
    for (regCls <- registrator) {
      logDebug("Running user registrator: " + regCls)
      try {
        val reg = Class.forName(regCls, true, classLoader).newInstance()
          .asInstanceOf[KryoRegistrator]
        reg.registerClasses(kryo)
      } catch {
        case e: Exception => 
          throw new SparkException(s"Failed to invoke $regCls", e)
      }
    }

[GrahamDennis]

@rxin: I've made the change you've suggested, but as there can be at most one custom kryo registrator (the for loop is extracting the value from an option), I don't see how this really improves things?

[rxin]

Because this way you get to log the class name :)

[rxin]

Jenkins, test this please.

[debasish83]

will this fix back-ported to 1.0.1 ? I am getting these nondeterministic exceptions on both 1.0.1 and 1.1.0-SNAPSHOT....

[SparkQA]

QA tests have started for PR 1827. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18530/consoleFull

[rxin]

Yes, back to branch-1.0. (not 1.0.1 since we cannot change a released version, but we can push 1.0.3).

[debasish83]

branch-1.0 is fine as well....

[SparkQA]

QA results for PR 1827:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18530/consoleFull

[GrahamDennis]

I forgot to update the test case when I changed the exception message generated. Fixed now.

[rxin]

you need to update this error message now.

[GrahamDennis]

This is the updated message. I beat you to it.

[GrahamDennis]

(I've run this test locally, and it passes).

[rxin]

Jenkins, retest this please.

[SparkQA]

QA tests have started for PR 1827. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18533/consoleFull

[SparkQA]

QA results for PR 1827:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18533/consoleFull

[rxin]

I think the test failure is not caused by this. Those two tests have been flaky. I'm going to merge this in master & branch-1.1 & branch-1.0. Thanks!

[GrahamDennis]

Great, thanks!