-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect multiple jars on the classpath when init plugin [databricks] #9654
Merged
thirtiseven
merged 35 commits into
NVIDIA:branch-24.02
from
thirtiseven:detect_multiple_jars
Nov 28, 2023
Merged
Changes from 27 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
2f1fae2
Detect multiple jars on the classpath when init plugin
thirtiseven 0c5df62
clean up
thirtiseven 742b508
Apply suggestions from code review
thirtiseven 47ef387
print version info and also check jni/cudf
thirtiseven 171c016
print version info and also check jni/cudf
thirtiseven e29e37d
add config for allowing multiple jars
thirtiseven 0441f53
keep jar path in error messages
thirtiseven ded091e
address comments
thirtiseven 42fc474
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
thirtiseven 6bbedfb
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
thirtiseven d9264d8
address comments
thirtiseven 5c108bb
Merge branch 'NVIDIA:branch-23.12' into detect_multiple_jars
thirtiseven c2a0402
Use unique properties for intermediate jars
thirtiseven 14c536d
clean up
thirtiseven b099358
address comment
thirtiseven 74010e0
Apply suggestions from code review
thirtiseven fd129c9
address comments
thirtiseven 5e292c4
add the project.artifactId to build-info and check it
thirtiseven 15022d7
remove unnecessary copyright update
thirtiseven 28c8dcd
remove log
thirtiseven 3dc12e4
Add 2.13 support
thirtiseven 14fe6fc
use revision to check duplicate jars
thirtiseven 5dcc15f
fix 2.13 build
thirtiseven 08f4088
support both SAME_REVISION and NEVER mode
thirtiseven 0ca06b5
Avoid CI change and filter out test
thirtiseven c7ca0ad
check values for config
thirtiseven a833c73
use enum
thirtiseven f5bd0c9
fix two nits
thirtiseven 405bf05
Merge branch 'branch-23.12' into detect_multiple_jars
thirtiseven 7d11a4f
Do not print log if no multiple jar
thirtiseven c4eef25
ignore subdir when checking multiple jars
thirtiseven bf4477a
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
thirtiseven 39e0b56
wip ut
thirtiseven c3279f4
address comment
thirtiseven 70b1247
Merge branch 'branch-24.02' into detect_multiple_jars
thirtiseven File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ | |||||||||||
package com.nvidia.spark.rapids | ||||||||||||
|
||||||||||||
import java.lang.reflect.InvocationTargetException | ||||||||||||
import java.net.URL | ||||||||||||
import java.time.ZoneId | ||||||||||||
import java.util.Properties | ||||||||||||
|
||||||||||||
|
@@ -25,6 +26,7 @@ import scala.sys.process._ | |||||||||||
import scala.util.Try | ||||||||||||
|
||||||||||||
import ai.rapids.cudf.{Cuda, CudaException, CudaFatalException, CudfException, MemoryCleaner} | ||||||||||||
import com.nvidia.spark.rapids.RapidsConf.AllowMultipleJars | ||||||||||||
import com.nvidia.spark.rapids.filecache.{FileCache, FileCacheLocalityManager, FileCacheLocalityMsg} | ||||||||||||
import com.nvidia.spark.rapids.python.PythonWorkerSemaphore | ||||||||||||
import org.apache.commons.lang3.exception.ExceptionUtils | ||||||||||||
|
@@ -111,6 +113,52 @@ object RapidsPluginUtils extends Logging { | |||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
private def detectMultipleJar(propName: String, jarName: String, conf: RapidsConf): Unit = { | ||||||||||||
val classloader = ShimLoader.getShimClassLoader() | ||||||||||||
val possibleRapidsJarURLs = classloader.getResources(propName).asScala.toSet.toSeq.filterNot { | ||||||||||||
url => url.toString.contains("test") | ||||||||||||
} | ||||||||||||
val revisionMap: Map[String, Seq[URL]] = possibleRapidsJarURLs.map { url => | ||||||||||||
val versionInfo = scala.io.Source.fromURL(url).getLines().toSeq | ||||||||||||
lazy val revision = versionInfo | ||||||||||||
.filter(_.startsWith("revision=")) | ||||||||||||
.map(_.split("=").last) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you define a regex val revisionRegex = "revision=(.*)".r you could do it in one shot
Suggested change
|
||||||||||||
.headOption | ||||||||||||
.getOrElse("UNKNOWN") | ||||||||||||
(revision, url) | ||||||||||||
}.groupBy(_._1).mapValues(_.map(_._2)).toMap | ||||||||||||
lazy val rapidsJarsVersMsg = revisionMap.map { | ||||||||||||
case (revision, urls) => { | ||||||||||||
s"revison: $revision" + urls.map { | ||||||||||||
url => "\n\tjar URL: " + url.toString.split("!").head + "\n\t" + | ||||||||||||
scala.io.Source.fromURL(url).getLines().toSeq.mkString("\n\t") | ||||||||||||
}.mkString + "\n" | ||||||||||||
} | ||||||||||||
}.mkString | ||||||||||||
// scalastyle:off line.size.limit | ||||||||||||
lazy val msg = s"""Multiple $jarName jars found in the classpath: | ||||||||||||
|$rapidsJarsVersMsg | ||||||||||||
|Please make sure there is only one $jarName jar in the classpath. | ||||||||||||
|If it is impossible to fix the classpath you can suppress the error by setting ${RapidsConf.ALLOW_MULTIPLE_JARS.key} to SAME_REVISION or ALWAYS. | ||||||||||||
""".stripMargin | ||||||||||||
// scalastyle:on line.size.limit | ||||||||||||
|
||||||||||||
conf.allowMultipleJars match { | ||||||||||||
case AllowMultipleJars.ALWAYS => | ||||||||||||
logWarning(msg) | ||||||||||||
case AllowMultipleJars.SAME_REVISION => | ||||||||||||
require(revisionMap.size == 1, msg) | ||||||||||||
case AllowMultipleJars.NEVER => | ||||||||||||
require(revisionMap.size == 1 && revisionMap.values.forall(_.size == 1), msg) | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
def detectMultipleJars(conf: RapidsConf): Unit = { | ||||||||||||
detectMultipleJar(PLUGIN_PROPS_FILENAME, "rapids-4-spark", conf) | ||||||||||||
detectMultipleJar(JNI_PROPS_FILENAME, "spark-rapids-jni", conf) | ||||||||||||
detectMultipleJar(CUDF_PROPS_FILENAME, "cudf", conf) | ||||||||||||
} | ||||||||||||
|
||||||||||||
// This assumes Apache Spark logic, if CSPs are setting defaults differently, we may need | ||||||||||||
// to handle. | ||||||||||||
def estimateCoresOnExec(conf: SparkConf): Int = { | ||||||||||||
|
@@ -309,6 +357,7 @@ class RapidsDriverPlugin extends DriverPlugin with Logging { | |||||||||||
val sparkConf = pluginContext.conf | ||||||||||||
RapidsPluginUtils.fixupConfigsOnDriver(sparkConf) | ||||||||||||
val conf = new RapidsConf(sparkConf) | ||||||||||||
RapidsPluginUtils.detectMultipleJars(conf) | ||||||||||||
RapidsPluginUtils.logPluginMode(conf) | ||||||||||||
GpuCoreDumpHandler.driverInit(sc, conf) | ||||||||||||
|
||||||||||||
|
@@ -363,6 +412,9 @@ class RapidsExecutorPlugin extends ExecutorPlugin with Logging { | |||||||||||
val numCores = RapidsPluginUtils.estimateCoresOnExec(sparkConf) | ||||||||||||
val conf = new RapidsConf(extraConf.asScala.toMap) | ||||||||||||
|
||||||||||||
// Fail if there are multiple plugin jars in the classpath. | ||||||||||||
RapidsPluginUtils.detectMultipleJars(conf) | ||||||||||||
|
||||||||||||
// Compare if the cudf version mentioned in the classpath is equal to the version which | ||||||||||||
// plugin expects. If there is a version mismatch, throw error. This check can be disabled | ||||||||||||
// by setting this config spark.rapids.cudfVersionOverride=true | ||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: no point in lazy