-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-10878][core] Fix race condition when multiple clients resolves artifacts at the same time #21251
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
[SPARK-10878][core] Fix race condition when multiple clients resolves artifacts at the same time #21251
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import java.lang.reflect.{InvocationTargetException, Modifier, UndeclaredThrowab | |
| import java.net.URL | ||
| import java.security.PrivilegedExceptionAction | ||
| import java.text.ParseException | ||
| import java.util.UUID | ||
|
|
||
| import scala.annotation.tailrec | ||
| import scala.collection.mutable.{ArrayBuffer, HashMap, Map} | ||
|
|
@@ -1204,7 +1205,36 @@ private[spark] object SparkSubmitUtils { | |
|
|
||
| /** A nice function to use in tests as well. Values are dummy strings. */ | ||
| def getModuleDescriptor: DefaultModuleDescriptor = DefaultModuleDescriptor.newDefaultInstance( | ||
| ModuleRevisionId.newInstance("org.apache.spark", "spark-submit-parent", "1.0")) | ||
| // Include UUID in module name, so multiple clients resolving maven coordinate at the same time | ||
| // do not modify the same resolution file concurrently. | ||
| ModuleRevisionId.newInstance("org.apache.spark", | ||
| s"spark-submit-parent-${UUID.randomUUID.toString}", | ||
| "1.0")) | ||
|
|
||
| /** | ||
| * clear ivy resolution from current launch. The resolution file is usually at | ||
| * ~/.ivy2/org.apache.spark-spark-submit-parent-$UUID-default.xml, | ||
| * ~/.ivy2/resolved-org.apache.spark-spark-submit-parent-$UUID-1.0.xml, and | ||
| * ~/.ivy2/resolved-org.apache.spark-spark-submit-parent-$UUID-1.0.properties. | ||
| * Since each launch will have its own resolution files created, delete them after | ||
| * each resolution to prevent accumulation of these files in the ivy cache dir. | ||
| */ | ||
| private def clearIvyResolutionFiles( | ||
| mdId: ModuleRevisionId, | ||
| ivySettings: IvySettings, | ||
| ivyConfName: String): Unit = { | ||
| val currentResolutionFiles = Seq[File]( | ||
| new File(ivySettings.getDefaultCache, | ||
|
Contributor
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. nit: you could move |
||
| s"${mdId.getOrganisation}-${mdId.getName}-$ivyConfName.xml"), | ||
| new File(ivySettings.getDefaultCache, | ||
| s"resolved-${mdId.getOrganisation}-${mdId.getName}-${mdId.getRevision}.xml"), | ||
| new File(ivySettings.getDefaultCache, | ||
| s"resolved-${mdId.getOrganisation}-${mdId.getName}-${mdId.getRevision}.properties") | ||
| ) | ||
| currentResolutionFiles.foreach { file => | ||
| file.delete() | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolves any dependencies that were supplied through maven coordinates | ||
|
|
@@ -1255,14 +1285,6 @@ private[spark] object SparkSubmitUtils { | |
|
|
||
| // A Module descriptor must be specified. Entries are dummy strings | ||
| val md = getModuleDescriptor | ||
| // clear ivy resolution from previous launches. The resolution file is usually at | ||
| // ~/.ivy2/org.apache.spark-spark-submit-parent-default.xml. In between runs, this file | ||
| // leads to confusion with Ivy when the files can no longer be found at the repository | ||
| // declared in that file/ | ||
| val mdId = md.getModuleRevisionId | ||
| val previousResolution = new File(ivySettings.getDefaultCache, | ||
| s"${mdId.getOrganisation}-${mdId.getName}-$ivyConfName.xml") | ||
| if (previousResolution.exists) previousResolution.delete | ||
|
|
||
| md.setDefaultConf(ivyConfName) | ||
|
|
||
|
|
@@ -1283,7 +1305,10 @@ private[spark] object SparkSubmitUtils { | |
| packagesDirectory.getAbsolutePath + File.separator + | ||
| "[organization]_[artifact]-[revision](-[classifier]).[ext]", | ||
| retrieveOptions.setConfs(Array(ivyConfName))) | ||
| resolveDependencyPaths(rr.getArtifacts.toArray, packagesDirectory) | ||
| val paths = resolveDependencyPaths(rr.getArtifacts.toArray, packagesDirectory) | ||
| val mdId = md.getModuleRevisionId | ||
| clearIvyResolutionFiles(mdId, ivySettings, ivyConfName) | ||
| paths | ||
| } finally { | ||
| System.setOut(sysOut) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -256,4 +256,20 @@ class SparkSubmitUtilsSuite extends SparkFunSuite with BeforeAndAfterAll { | |
| assert(jarPath.indexOf("mydep") >= 0, "should find dependency") | ||
| } | ||
| } | ||
|
|
||
| test("SPARK-10878: test resolution files cleaned after resolving artifact") { | ||
| val main = new MavenCoordinate("my.great.lib", "mylib", "0.1") | ||
|
|
||
| IvyTestUtils.withRepository(main, None, None) { repo => | ||
| val ivySettings = SparkSubmitUtils.buildIvySettings(Some(repo), Some(tempIvyPath)) | ||
| val jarPath = SparkSubmitUtils.resolveMavenCoordinates( | ||
| main.toString, | ||
| ivySettings, | ||
| isTest = true) | ||
| val r = """.*org.apache.spark-spark-submit-parent-.*""".r | ||
| assert(ivySettings.getDefaultCache.listFiles.map(_.getName) | ||
| .forall { case n @ r() => false case _ => true }, | ||
|
Contributor
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. nit: I think using |
||
| "resolution files should be cleaned") | ||
| } | ||
| } | ||
| } | ||
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: start comment with capital letter.