[SPARK-26235][Core] Change log level for ClassNotFoundException/NoClassDefFoundError in SparkSubmit to Error

[gengliangwang]

What changes were proposed in this pull request?

In my local setup, I set log4j root category as ERROR (https://stackoverflow.com/questions/27781187/how-to-stop-info-messages-displaying-on-spark-console , first item show up if we google search "set spark log level".) When I run such command

spark-submit --class foo bar.jar 

Nothing shows up, and the script exits.

After quick investigation, I think the log level for ClassNotFoundException/NoClassDefFoundError in SparkSubmit should be ERROR instead of WARN. Since the whole process exit because of the exception/error.

Before #20925, the message is not controlled by log4j.rootCategory.

How was this patch tested?

Manual check.

[gengliangwang]

cc @vanzin

This is really trivial, but can be helpful to user in certain case.

[vanzin]

Actually the problem is here:

      case e: ClassNotFoundException =>
        logWarning(s"Failed to load $childMainClass.", e)

That particular logWarning is not overridden in the SparkSubmit object:

  override def main(args: Array[String]): Unit = {
    val submit = new SparkSubmit() {
      ...
      override protected def logWarning(msg: => String): Unit = printMessage(s"Warning: $msg")

Probably should instead use the same logWarning call as the other handler below:

      case e: NoClassDefFoundError =>
        logWarning(s"Failed to load $childMainClass: ${e.getMessage()}")

Which should then always print the error regardless of your log4j configuration.

[gengliangwang]

@vanzin The logWarning call as the other handler below is also not overridden:

      case e: NoClassDefFoundError =>
        logWarning(s"Failed to load $childMainClass: ${e.getMessage()}")

My basic point is, the exception/error message should be in the category of ERROR, even previously Spark prints as warning to System.err.

[SparkQA]

Test build #99514 has finished for PR 23189 at commit 158d421.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

The logWarning call as the other handler below is also not overridden:

It is. I even copied & pasted the code. I made the change locally and this is what happens:

$ ./bin/spark-submit --class foo bar.jar 
Warning: Failed to load foo: foo

With your change, if you are using a file appender instead of a console appender, you'll still not see the error on the terminal. For spark-submit, since it's a user-run script, it's good to print something to the terminal in these cases, regardless of the logging configuration.

If you want to also write the full exception to the logs, that's fine, but the change I suggested is more correct here considering the use.

[gengliangwang]

@vanzin Ah, I see. Thanks for pointing it out!

But I am now thinking overriding logError as calling printMessage("Error..."). What do you think?

[vanzin]

Sounds ok.

[gengliangwang]

Here I tried append e.getMessage to the output, but it seems the error message equals the class name childMainClass. e.g.

Error: Failed to load class foo: foo.

So I think we can output without the exception message.

[SparkQA]

Test build #99525 has finished for PR 23189 at commit 1662a8b.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
• logError(s\"Failed to load class $childMainClass.\")

[gengliangwang]

retest this please.

[SparkQA]

Test build #99545 has finished for PR 23189 at commit 1662a8b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
• logError(s\"Failed to load class $childMainClass.\")

[srowen]

Merged to master