You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2023/12/14 03:01:53 UTC
[PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, retry to skip this cache [spark]
panbingkun opened a new pull request, #44343:
URL: https://github.com/apache/spark/pull/44343
### What changes were proposed in this pull request?
The pr aims to fix bug.
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
Manually test.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426727164
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -405,6 +415,7 @@ private[spark] object MavenUtils extends Logging {
def resolveMavenCoordinates(
coordinates: String,
ivySettings: IvySettings,
+ noCacheIvySettings: Option[IvySettings] = None,
Review Comment:
Where was a non-None `noCacheIvySettings` passed in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1917441238
@srowen Do you have any suggestions on this pr? As far as I remember, the current maven daily test still can't test `HiveExternalCatalogVersionsSuite` due to this issue. Thanks ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426738937
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -405,6 +415,7 @@ private[spark] object MavenUtils extends Logging {
def resolveMavenCoordinates(
coordinates: String,
ivySettings: IvySettings,
+ noCacheIvySettings: Option[IvySettings] = None,
Review Comment:
I added the input for `noCacheIvySettings` in `IsolatedClientLoader` and built a new client. Although the retry was successful, there will be a large number of error messages like the following:
```
:: problems summary ::
:::: ERRORS
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
unknown resolver local-m2-cache
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1429012219
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -405,6 +415,7 @@ private[spark] object MavenUtils extends Logging {
def resolveMavenCoordinates(
coordinates: String,
ivySettings: IvySettings,
+ noCacheIvySettings: Option[IvySettings] = None,
Review Comment:
> I added the input for `noCacheIvySettings` in `IsolatedClientLoader` and built a new client. Although the retry was successful, there will be a large number of error messages like the following:
>
> ```
> :: problems summary ::
> :::: ERRORS
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
>
> unknown resolver local-m2-cache
> ```
The above issues have been resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1429012274
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -432,7 +443,7 @@ private[spark] object MavenUtils extends Logging {
printStream.println(s"The jars for the packages stored in: $packagesDirectory")
// scalastyle:on println
- val ivy = Ivy.newInstance(ivySettings)
+ var ivy = Ivy.newInstance(ivySettings)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1919377283
Merged to master
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426656195
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
+ if (failedReports.nonEmpty && noCacheIvySettings.isDefined) {
+ val failedArtifacts = failedReports.map(
+ fr => fr.getArtifact).mkString("[", ", ", "]")
+ logInfo(s"Download failed: $failedArtifacts, attempt to skip local maven cache.")
+ ivy = Ivy.newInstance(noCacheIvySettings.get)
Review Comment:
if `noCacheIvySettings` is None, there will be `None.get`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426665614
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
Review Comment:
My concern is that if the submission machine has a .m2 dir, it will most likely enter the retry process of without local-m2-cache because the local-m2-cache produced from building spark distribution also has this [issues](https://github.com/apache/spark/pull/44208#pullrequestreview-1777666633) now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426727164
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -405,6 +415,7 @@ private[spark] object MavenUtils extends Logging {
def resolveMavenCoordinates(
coordinates: String,
ivySettings: IvySettings,
+ noCacheIvySettings: Option[IvySettings] = None,
Review Comment:
Where was a non-None `noCacheIvySettings` passed in? I build a new client with this pr , but also found the error message:
```
:: problems summary ::
:::: WARNINGS
[NOT FOUND ] org.apache.hive#hive-exec;2.3.9!hive-exec.jar (0ms)
==== local-m2-cache: tried
file:/Users/yangjie01/.m2/repository/org/apache/hive/hive-exec/2.3.9/hive-exec-2.3.9.jar
[NOT FOUND ] asm#asm;3.2!asm.jar (1ms)
==== local-m2-cache: tried
file:/Users/yangjie01/.m2/repository/asm/asm/3.2/asm-3.2.jar
::::::::::::::::::::::::::::::::::::::::::::::
:: FAILED DOWNLOADS ::
:: ^ see resolution messages for details ^ ::
::::::::::::::::::::::::::::::::::::::::::::::
:: asm#asm;3.2!asm.jar
:: org.apache.hive#hive-exec;2.3.9!hive-exec.jar
::::::::::::::::::::::::::::::::::::::::::::::
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426659839
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -432,7 +443,7 @@ private[spark] object MavenUtils extends Logging {
printStream.println(s"The jars for the packages stored in: $packagesDirectory")
// scalastyle:on println
- val ivy = Ivy.newInstance(ivySettings)
+ var ivy = Ivy.newInstance(ivySettings)
Review Comment:
We can give it a new name below, no need to change it to `var` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1429012852
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -128,24 +128,30 @@ private[spark] object MavenUtils extends Logging {
}
/**
- * Extracts maven coordinates from a comma-delimited string
+ * Create a ChainResolver used by Ivy to search for and resolve dependencies.
Review Comment:
The original code comments were incorrect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1920549192
@panbingkun Could you submit a fix for branch-3.5 and branch-3.4 respectively? When testing `HiveExternalCatalogVersionsSuite ` using maven, it requires downloading Spark-3.5 and Spark-3.4 with this fix in order to complete the creation of the data table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426668094
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -394,6 +402,8 @@ private[spark] object MavenUtils extends Logging {
* Comma-delimited string of maven coordinates
* @param ivySettings
* An IvySettings containing resolvers to use
+ * @param noCacheIvySettings
+ * An no-cache IvySettings containing resolvers to use
Review Comment:
Indentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1873698520
friendly ping @HyukjinKwon, do you know who would be a good fit to review this PR together?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426659839
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -432,7 +443,7 @@ private[spark] object MavenUtils extends Logging {
printStream.println(s"The jars for the packages stored in: $packagesDirectory")
// scalastyle:on println
- val ivy = Ivy.newInstance(ivySettings)
+ var ivy = Ivy.newInstance(ivySettings)
Review Comment:
We can give it a new name below, no need to change it to 'var' here.
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426665614
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
Review Comment:
My concern is that if the submission machine has a .m2 dir, it will most likely enter the retry process of removing local-m2-cache...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen closed pull request #44343: [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again
URL: https://github.com/apache/spark/pull/44343
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1946772230
Thank you, @panbingkun and ll!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1917469919
If this fixes the issue, great! any downside?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426665614
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
Review Comment:
My concern is that if the submission machine has a .m2 dir, it will most likely enter the retry process of without local-m2-cache because the local-m2-cache produced from building spark distribution also has this [issues](https://github.com/apache/spark/pull/44208#pullrequestreview-1777666633) now.
Perhaps it's difficult to obtain a perfect local-m2-cache now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1855458411
friendly ping @vsevolodstep-db @hvanhovell
If you have time, could you take some time to help review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426656195
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
+ if (failedReports.nonEmpty && noCacheIvySettings.isDefined) {
+ val failedArtifacts = failedReports.map(
+ fr => fr.getArtifact).mkString("[", ", ", "]")
+ logInfo(s"Download failed: $failedArtifacts, attempt to skip local maven cache.")
+ ivy = Ivy.newInstance(noCacheIvySettings.get)
Review Comment:
if `noCacheIvySettings` is None, there will be `None.get`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426665614
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
Review Comment:
My concern is that if the submission machine has a .m2 dir, it will most likely enter the retry process of removing local-m2-cache because the local-m2-cache produced from building spark distributionalso has this issues now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1426661075
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -454,9 +465,23 @@ private[spark] object MavenUtils extends Logging {
md.addExcludeRule(createExclusion(e + ":*", ivySettings, ivyConfName))
}
// resolve dependencies
- val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+ var rr: ResolveReport = ivy.resolve(md, resolveOptions)
if (rr.hasError) {
- throw new RuntimeException(rr.getAllProblemMessages.toString)
+ // SPARK-46302: When there are some corrupted jars in the maven repo,
+ // we try to continue without the cache
+ val failedReports = rr.getArtifactsReports(DownloadStatus.FAILED, true)
+ if (failedReports.nonEmpty && noCacheIvySettings.isDefined) {
+ val failedArtifacts = failedReports.map(
+ fr => fr.getArtifact).mkString("[", ", ", "]")
+ logInfo(s"Download failed: $failedArtifacts, attempt to skip local maven cache.")
Review Comment:
skip local-m2-cache?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #44343:
URL: https://github.com/apache/spark/pull/44343#discussion_r1427444877
##########
common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala:
##########
@@ -405,6 +415,7 @@ private[spark] object MavenUtils extends Logging {
def resolveMavenCoordinates(
coordinates: String,
ivySettings: IvySettings,
+ noCacheIvySettings: Option[IvySettings] = None,
Review Comment:
> Where was a non-None `noCacheIvySettings` passed in? I build a new client with this pr , but also found the error message:
>
> ```
> :: problems summary ::
> :::: WARNINGS
> [NOT FOUND ] org.apache.hive#hive-exec;2.3.9!hive-exec.jar (0ms)
>
> ==== local-m2-cache: tried
>
> file:/Users/yangjie01/.m2/repository/org/apache/hive/hive-exec/2.3.9/hive-exec-2.3.9.jar
>
> [NOT FOUND ] asm#asm;3.2!asm.jar (1ms)
>
> ==== local-m2-cache: tried
>
> file:/Users/yangjie01/.m2/repository/asm/asm/3.2/asm-3.2.jar
>
> ::::::::::::::::::::::::::::::::::::::::::::::
>
> :: FAILED DOWNLOADS ::
>
> :: ^ see resolution messages for details ^ ::
>
> ::::::::::::::::::::::::::::::::::::::::::::::
>
> :: asm#asm;3.2!asm.jar
>
> :: org.apache.hive#hive-exec;2.3.9!hive-exec.jar
>
> ::::::::::::::::::::::::::::::::::::::::::::::
> ```
Yes, this is my bad, it should be passed in when `IsolatedClientLoader` calls it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1918831724
> If this fixes the issue, great! any downside?
The advantage: when there is a `corrupted jar` in Local Maven, it will retry in the `skip` `Local Maven cache` and will no longer fail to exit due to this reason.
The disadvantage: during the second retry, `local Maven` will no longer be used as the `cache`, which will increase some download time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #44343:
URL: https://github.com/apache/spark/pull/44343#issuecomment-1922709655
> @panbingkun Could you submit a fix for branch-3.5 and branch-3.4 respectively? When testing `HiveExternalCatalogVersionsSuite ` using maven, it requires downloading [Spark-3](https://issues.apache.org/jira/browse/SPARK-3).5 and [Spark-3](https://issues.apache.org/jira/browse/SPARK-3).4 with this fix in order to complete the creation of the data table.
Okay, let me do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org