-
Notifications
You must be signed in to change notification settings - Fork 29k
[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
Conversation
|
Test build #90273 has finished for PR 21251 at commit
|
|
cc @gatorsmile |
vanzin
left a comment
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.
Just minor things.
| ivySettings: IvySettings, | ||
| ivyConfName: String): Unit = { | ||
| val currentResolutionFiles = Seq[File]( | ||
| new File(ivySettings.getDefaultCache, |
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: you could move new File(ivySettings.getDefaultCache to the foreach loop instead.
| isTest = true) | ||
| val r = """.*org.apache.spark-spark-submit-parent-.*""".r | ||
| assert(ivySettings.getDefaultCache.listFiles.map(_.getName) | ||
| .forall { case n @ r() => false case _ => true }, |
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: I think using !<list of files>.exists(r.findFirstIn(_).isDefined) would be slightly clearer than you version.
| "1.0")) | ||
|
|
||
| /** | ||
| * clear ivy resolution from current launch. The resolution file is usually at |
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.
|
Test build #90396 has finished for PR 21251 at commit
|
|
retest this please |
|
Test build #90405 has finished for PR 21251 at commit
|
|
LGTM. Merging to master / 2.3. |
… artifacts at the same time ## What changes were proposed in this pull request? When multiple clients attempt to resolve artifacts via the `--packages` parameter, they could run into race condition when they each attempt to modify the dummy `org.apache.spark-spark-submit-parent-default.xml` file created in the default ivy cache dir. This PR changes the behavior to encode UUID in the dummy module descriptor so each client will operate on a different resolution file in the ivy cache dir. In addition, this patch changes the behavior of when and which resolution files are cleaned to prevent accumulation of resolution files in the default ivy cache dir. Since this PR is a successor of #18801, close #18801. Many codes were ported from #18801. **Many efforts were put here. I think this PR should credit to Victsm .** ## How was this patch tested? added UT into `SparkSubmitUtilsSuite` Author: Kazuaki Ishizaki <[email protected]> Closes #21251 from kiszk/SPARK-10878. (cherry picked from commit d3c426a) Signed-off-by: Marcelo Vanzin <[email protected]>
… artifacts at the same time ## What changes were proposed in this pull request? When multiple clients attempt to resolve artifacts via the `--packages` parameter, they could run into race condition when they each attempt to modify the dummy `org.apache.spark-spark-submit-parent-default.xml` file created in the default ivy cache dir. This PR changes the behavior to encode UUID in the dummy module descriptor so each client will operate on a different resolution file in the ivy cache dir. In addition, this patch changes the behavior of when and which resolution files are cleaned to prevent accumulation of resolution files in the default ivy cache dir. Since this PR is a successor of apache#18801, close apache#18801. Many codes were ported from apache#18801. **Many efforts were put here. I think this PR should credit to Victsm .** ## How was this patch tested? added UT into `SparkSubmitUtilsSuite` Author: Kazuaki Ishizaki <[email protected]> Closes apache#21251 from kiszk/SPARK-10878.
… artifacts at the same time When multiple clients attempt to resolve artifacts via the `--packages` parameter, they could run into race condition when they each attempt to modify the dummy `org.apache.spark-spark-submit-parent-default.xml` file created in the default ivy cache dir. This PR changes the behavior to encode UUID in the dummy module descriptor so each client will operate on a different resolution file in the ivy cache dir. In addition, this patch changes the behavior of when and which resolution files are cleaned to prevent accumulation of resolution files in the default ivy cache dir. Since this PR is a successor of apache#18801, close apache#18801. Many codes were ported from apache#18801. **Many efforts were put here. I think this PR should credit to Victsm .** added UT into `SparkSubmitUtilsSuite` Author: Kazuaki Ishizaki <[email protected]> Closes apache#21251 from kiszk/SPARK-10878. (cherry picked from commit d3c426a) RB=1313943 G=superfriends-reviewers R=fli,mshen,yezhou,edlu A=fli
What changes were proposed in this pull request?
When multiple clients attempt to resolve artifacts via the
--packagesparameter, they could run into race condition when they each attempt to modify the dummyorg.apache.spark-spark-submit-parent-default.xmlfile created in the default ivy cache dir.This PR changes the behavior to encode UUID in the dummy module descriptor so each client will operate on a different resolution file in the ivy cache dir. In addition, this patch changes the behavior of when and which resolution files are cleaned to prevent accumulation of resolution files in the default ivy cache dir.
Since this PR is a successor of #18801, close #18801. Many codes were ported from #18801. Many efforts were put here. I think this PR should credit to @Victsm .
How was this patch tested?
added UT into
SparkSubmitUtilsSuite