Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] #5716

razajafri · 2022-06-02T00:04:46Z

This PR uses the ShimLoader to load the AvroScan class to make sure it's on the classpath. It also uses the SparkClassLoader to warn the user if he wants to use the avro lib but RAPIDS plugin won't see the avro lib.

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-02T04:44:14Z

build

jbrennan333 · 2022-06-02T17:18:21Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

@@ -121,6 +129,10 @@ object ExternalSource {
  }

  def getScans: Map[Class[_ <: Scan], ScanRule[_ <: Scan]] = {
+    if (showErrorMessage) {
+      println("WARNING: Avro library isn't loaded by the RAPIDS plugin. Please use --jars to " +


Should this be using System.err.println()?

It should be using the logging framework logWarning/Error

Sorry, this was a quick test that I forgot to replace with logError. Thanks

jbrennan333 · 2022-06-02T17:25:24Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

@@ -121,6 +129,10 @@ object ExternalSource {
  }

  def getScans: Map[Class[_ <: Scan], ScanRule[_ <: Scan]] = {
+    if (showErrorMessage) {
+      println("WARNING: Avro library isn't loaded by the RAPIDS plugin. Please use --jars to " +
+          "load the plugin if you haven't done so")


Since this is optional, I think the warning could be worded better.
Maybe something like: "WARNING: Avro library not found by the RAPIDS plugin. If needed, please use --jars to add it"
Should we include the full classpath in the error message?

I can change that

gerashegalov · 2022-06-02T17:58:26Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

    /** spark-avro is an optional package for Spark, so the RAPIDS Accelerator
     * must run successfully without it. */
-    Try(loader.loadClass("org.apache.spark.sql.v2.avro.AvroScan")) match {
+    Try(ShimLoader.loadClass("org.apache.spark.sql.v2.avro.AvroScan")) match {


My preference is not to change this logic until we have a grasp of the root cause.

Let us check if #5723 changes anything

I was thinking the same thing. I can quickly test that

razajafri · 2022-06-02T19:12:34Z

Looks like #5723 is a better solution for the `NoClassDefFoundError".

I will close this in it's favor

razajafri · 2022-06-03T00:46:31Z

Looks like #5723 is a better solution for the `NoClassDefFoundError".

I will close this in it's favor

Reopening as #5723 doesn't fix the bug as I originally thought

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-03T16:27:21Z

@gerashegalov @jbrennan333
Can we take another look at this?

jbrennan333

This looks good to me other than the nit. I defer to @gerashegalov for approval.

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

gerashegalov · 2022-06-03T18:06:22Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

@@ -121,6 +130,10 @@ object ExternalSource {
  }

  def getScans: Map[Class[_ <: Scan], ScanRule[_ <: Scan]] = {
+    if (showErrorMessage) {


I think we should consolidate all this logic in hasSparkAvroJar. I'll make a PR to your PR with a suggestion razajafri#1

gerashegalov · 2022-06-03T18:12:01Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

@@ -121,6 +130,10 @@ object ExternalSource {
  }

  def getScans: Map[Class[_ <: Scan], ScanRule[_ <: Scan]] = {
+    if (showErrorMessage) {
+      logWarning("WARNING: Avro library not found by the RAPIDS plugin. If needed, please use " +
+          "--jars to add it")


we need to document that --jars or --packages is the preferred way of deploying the plugin jar, specifically for Avro.

Let us drop WARNING because it's already part of the logging category. The message should talk about the plugin jars, since this is the actual problem, not Avro:

"Avro library not found by the RAPIDS plugin. The Plugin jars are likely deployed using a static classpath spark.driver/executor.extraClassPath. Consider using --jars or --packages instead."

Signed-off-by: Gera Shegalov <[email protected]>

gerashegalov

We need a note in docs and adjust the error message

Review suggestions

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-04T00:10:44Z

build

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-04T00:15:23Z

build

sameerz · 2022-06-06T12:33:27Z

build

gerashegalov · 2022-06-06T15:19:28Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala

+    Utils.classIsLoadable(avroScanClassName) && {
+      Try(ShimLoader.loadClass(avroScanClassName)).map(_ => true)
+        .getOrElse {
+          logWarning("WARNING: Avro library not found by the RAPIDS plugin. If needed, please use " +


Address PR checks

scalastyle check flagged on the PR

error file=/home/runner/work/spark-rapids/spark-rapids/sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala message=File line length exceeds 100 characters line=42

Previous comment not addressed yet
#5716 (comment)

gerashegalov · 2022-06-06T15:30:27Z

docs/FAQ.md

@@ -57,6 +57,10 @@ More information about cards that support forward compatibility can be found

 ### How can I check if the RAPIDS Accelerator is installed and which version is running?

+Using the `--jars` or `--packages` option followed by the file path or maven path to RAPIDS jar is the preferred way to run RAPIDS accelerator.


Unfortunately we have lots of places in the doc advertising extraClassPath https://github.com/NVIDIA/spark-rapids/search?q=%22extraClassPath%22&type=code

Not sure if this is the right place to document it. At any rate it deserves a dedicated FAQ question for the warning being introduced in this PR.

What to do when I see the warning ...

@sameerz @viadea
Please advise on the documentation comment by @gerashegalov

Signed-off-by: Raza Jafri <[email protected]>

gerashegalov · 2022-06-06T18:04:10Z

docs/FAQ.md

@@ -57,6 +57,10 @@ More information about cards that support forward compatibility can be found

 ### How can I check if the RAPIDS Accelerator is installed and which version is running?

+Using the `--jars` or `--packages` option followed by the file path or maven path to RAPIDS jar is the preferred way to run RAPIDS accelerator.
+If RAPIDS jar is copied directly to the `$SPARK_HOME/jars` folder, it might result in the ClassLoader not setting up classes


Having rapids jars as part of extraClassPath triggers the same Exception as copying to $SPARK_HOME/jars.
I would not even mention latter because it seems like a user error.

Not sure what we want to call extraClassPath officially. I used the to use the term static classpath because they are prepended independently of job classes (--jars, --packages). @tgravescs might want to chime in

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-06T20:01:42Z

build

razajafri · 2022-06-06T20:03:40Z

build

docs/FAQ.md

Signed-off-by: Raza Jafri <[email protected]>

razajafri · 2022-06-06T21:04:39Z

@tgravescs @gerashegalov PTAL

gerashegalov · 2022-06-06T23:03:53Z

build

razajafri added 2 commits June 1, 2022 15:28

use the shim to load the avro class

3002cc3

Signed-off-by: Raza Jafri <[email protected]>

Use ShimLoader to load avro class and to check

72cb9eb

Signed-off-by: Raza Jafri <[email protected]>

sameerz added the bug Something isn't working label Jun 2, 2022

sameerz added this to the May 23 - Jun 3 milestone Jun 2, 2022

res-life mentioned this pull request Jun 2, 2022

Dynamically load hive and avro using reflection to avoid potential class not found exception [databricks] #5723

Merged

jbrennan333 reviewed Jun 2, 2022

View reviewed changes

tgravescs requested review from gerashegalov and removed request for gerashegalov June 2, 2022 17:27

gerashegalov reviewed Jun 2, 2022

View reviewed changes

razajafri closed this Jun 2, 2022

razajafri reopened this Jun 3, 2022

addressed review comments

a094989

Signed-off-by: Raza Jafri <[email protected]>

jbrennan333 reviewed Jun 3, 2022

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ExternalSource.scala Outdated Show resolved Hide resolved

gerashegalov reviewed Jun 3, 2022

View reviewed changes

Review suggestions

40c4a25

Signed-off-by: Gera Shegalov <[email protected]>

gerashegalov requested changes Jun 3, 2022

View reviewed changes

gerashegalov mentioned this pull request Jun 3, 2022

Review suggestions razajafri/spark-rapids#1

Merged

razajafri and others added 2 commits June 3, 2022 14:27

Merge pull request #1 from gerashegalov/5716-gera

3d59fc6

Review suggestions

added documentation

166de13

Signed-off-by: Raza Jafri <[email protected]>

modified documentation

bc268a4

Signed-off-by: Raza Jafri <[email protected]>

gerashegalov requested changes Jun 6, 2022

View reviewed changes

addressed build failure

bc57ff1

Signed-off-by: Raza Jafri <[email protected]>

gerashegalov reviewed Jun 6, 2022

View reviewed changes

sameerz modified the milestones: May 23 - Jun 3, Jun 6 - Jun 17 Jun 6, 2022

addressed comments

4baafc8

Signed-off-by: Raza Jafri <[email protected]>

Merge branch 'branch-22.06' into NV-3639073-avro-fix

91667d9

tgravescs reviewed Jun 6, 2022

View reviewed changes

docs/FAQ.md Show resolved Hide resolved

added the exact error and warning

8bd589e

Signed-off-by: Raza Jafri <[email protected]>

tgravescs approved these changes Jun 6, 2022

View reviewed changes

gerashegalov approved these changes Jun 6, 2022

View reviewed changes

gerashegalov linked an issue Jun 6, 2022 that may be closed by this pull request

[BUG] NoClassDefFoundError for spark-avro when the Plugin is deployed via extraClassPath/driver-class-path #5758

Open

gerashegalov removed a link to an issue Jun 6, 2022

[BUG] NoClassDefFoundError for spark-avro when the Plugin is deployed via extraClassPath/driver-class-path #5758

Open

gerashegalov mentioned this pull request Jun 6, 2022

[BUG] 22.08 ClassLoader-related issues #5757

Open

7 tasks

gerashegalov changed the title ~~Use ShimLoader to load the Avro lib [databricks]~~ Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] Jun 6, 2022

sameerz merged commit 8137990 into NVIDIA:branch-22.06 Jun 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] #5716

Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] #5716

razajafri commented Jun 2, 2022

razajafri commented Jun 2, 2022

jbrennan333 Jun 2, 2022

gerashegalov Jun 2, 2022 •

edited

Loading

razajafri Jun 2, 2022

jbrennan333 Jun 2, 2022

razajafri Jun 2, 2022

gerashegalov Jun 2, 2022

gerashegalov Jun 2, 2022

razajafri Jun 2, 2022

razajafri commented Jun 2, 2022

razajafri commented Jun 3, 2022

razajafri commented Jun 3, 2022

jbrennan333 left a comment

gerashegalov Jun 3, 2022 •

edited

Loading

gerashegalov Jun 3, 2022

gerashegalov Jun 3, 2022

gerashegalov left a comment

razajafri commented Jun 4, 2022

razajafri commented Jun 4, 2022

sameerz commented Jun 6, 2022

gerashegalov Jun 6, 2022

gerashegalov Jun 6, 2022

razajafri Jun 6, 2022

gerashegalov Jun 6, 2022

razajafri commented Jun 6, 2022

razajafri commented Jun 6, 2022

razajafri commented Jun 6, 2022

gerashegalov commented Jun 6, 2022

		@@ -57,6 +57,10 @@ More information about cards that support forward compatibility can be found

		### How can I check if the RAPIDS Accelerator is installed and which version is running?

		Using the `--jars` or `--packages` option followed by the file path or maven path to RAPIDS jar is the preferred way to run RAPIDS accelerator.

Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] #5716

Disable Avro support when spark-avro classes not loadable by Shim classloader [databricks] #5716

Conversation

razajafri commented Jun 2, 2022

razajafri commented Jun 2, 2022

Choose a reason for hiding this comment

gerashegalov Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

razajafri commented Jun 2, 2022

razajafri commented Jun 3, 2022

razajafri commented Jun 3, 2022

jbrennan333 left a comment

Choose a reason for hiding this comment

gerashegalov Jun 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gerashegalov left a comment

Choose a reason for hiding this comment

razajafri commented Jun 4, 2022

razajafri commented Jun 4, 2022

sameerz commented Jun 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

razajafri commented Jun 6, 2022

razajafri commented Jun 6, 2022

razajafri commented Jun 6, 2022

gerashegalov commented Jun 6, 2022

gerashegalov Jun 2, 2022 •

edited

Loading

gerashegalov Jun 3, 2022 •

edited

Loading