[SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again #44343

panbingkun · 2023-12-14T03:01:50Z

What changes were proposed in this pull request?

The pr aims to

fix potential bug(ie: [SPARK-46302][TESTS] Skip maven daily testing as ivy uses some corrupted cache jar files #44208) and enhance user experience.
make the code more compliant with standards

Why are the changes needed?

We use the local maven repo as the first-level cache in ivy. The original intention was to reduce the time required to parse and obtain the ar, but when there are corrupted files in the local maven repo,The above mechanism will be directly interrupted and the prompt is very unfriendly, which will greatly confuse the user. Based on the original intention, we should skip the cache directly in similar situations.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually test.

Was this patch authored or co-authored using generative AI tooling?

No.

…maven repo, try to skip this cache

panbingkun · 2023-12-14T09:11:01Z

friendly ping @vsevolodstep-db @hvanhovell
If you have time, could you take some time to help review this PR?

LuciferYang · 2023-12-14T12:31:34Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

        // scalastyle:on println

-        val ivy = Ivy.newInstance(ivySettings)
+        var ivy = Ivy.newInstance(ivySettings)


We can give it a new name below, no need to change it to var here.

LuciferYang · 2023-12-14T12:31:40Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

        }
        // resolve dependencies
-        val rr: ResolveReport = ivy.resolve(md, resolveOptions)
+        var rr: ResolveReport = ivy.resolve(md, resolveOptions)


LuciferYang · 2023-12-14T12:32:50Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

+          if (failedReports.nonEmpty && noCacheIvySettings.isDefined) {
+            val failedArtifacts = failedReports.map(
+              fr => fr.getArtifact).mkString("[", ", ", "]")
+            logInfo(s"Download failed: $failedArtifacts, attempt to skip local maven cache.")


skip local-m2-cache?

LuciferYang · 2023-12-14T12:37:24Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

-          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)


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 now.

Perhaps it's difficult to obtain a perfect local-m2-cache now.

LuciferYang · 2023-12-14T12:39:49Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

   * @param ivySettings
   *   An IvySettings containing resolvers to use
+   * @param noCacheIvySettings
+   *  An no-cache IvySettings containing resolvers to use


Indentation

LuciferYang · 2023-12-14T13:35:09Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala

  def resolveMavenCoordinates(
      coordinates: String,
      ivySettings: IvySettings,
+      noCacheIvySettings: Option[IvySettings] = None,


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 ::::::::::::::::::::::::::::::::::::::::::::::

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

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.

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.

…maven repo, skip this cache and try again

panbingkun · 2023-12-17T04:33:37Z

common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala


  /**
-   * Extracts maven coordinates from a comma-delimited string
+   * Create a ChainResolver used by Ivy to search for and resolve dependencies.


The original code comments were incorrect.

LuciferYang

Personally, it is acceptable to directly remove or disable the use of local-m2-cache by default for me, as this only affects the speed of the first startup with spark.sql.hive.metastore.jars=maven

Besides, we should listen to other people's suggestions

LuciferYang · 2024-01-02T07:42:32Z

friendly ping @HyukjinKwon, do you know who would be a good fit to review this PR together? Thanks ~

LuciferYang · 2024-01-30T16:35:50Z

@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 ~

srowen · 2024-01-30T16:49:08Z

If this fixes the issue, great! any downside?

mridulm · 2024-01-30T19:57:44Z

+CC @shardulm94

panbingkun · 2024-01-31T10:34:08Z

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.

srowen · 2024-01-31T15:46:26Z

Merged to master

LuciferYang · 2024-02-01T05:34:06Z

@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.

panbingkun · 2024-02-02T02:59:25Z

@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.

Okay, let me do it.

…ocal maven repo, skip this cache and try again ### What changes were proposed in this pull request? The pr aims to - fix potential bug(ie: #44208) and enhance user experience. - make the code more compliant with standards Backport above to branch 3.5. Master branch pr: #44343 ### Why are the changes needed? We use the local maven repo as the first-level cache in ivy. The original intention was to reduce the time required to parse and obtain the ar, but when there are corrupted files in the local maven repo,The above mechanism will be directly interrupted and the prompt is very unfriendly, which will greatly confuse the user. Based on the original intention, we should skip the cache directly in similar situations. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #45017 from panbingkun/branch-3.5_SPARK-46400. Authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]>

…ocal maven repo, skip this cache and try again ### What changes were proposed in this pull request? The pr aims to - fix potential bug(ie: #44208) and enhance user experience. - make the code more compliant with standards Backport above to branch 3.4. Master branch pr: #44343 ### Why are the changes needed? We use the local maven repo as the first-level cache in ivy. The original intention was to reduce the time required to parse and obtain the ar, but when there are corrupted files in the local maven repo,The above mechanism will be directly interrupted and the prompt is very unfriendly, which will greatly confuse the user. Based on the original intention, we should skip the cache directly in similar situations. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #45018 from panbingkun/branch-3.4_SPARK-46400. Authored-by: panbingkun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

dongjoon-hyun · 2024-02-15T17:55:20Z

Thank you, @panbingkun and all!

…ocal maven repo, skip this cache and try again ### What changes were proposed in this pull request? The pr aims to - fix potential bug(ie: apache#44208) and enhance user experience. - make the code more compliant with standards Backport above to branch 3.4. Master branch pr: apache#44343 ### Why are the changes needed? We use the local maven repo as the first-level cache in ivy. The original intention was to reduce the time required to parse and obtain the ar, but when there are corrupted files in the local maven repo,The above mechanism will be directly interrupted and the prompt is very unfriendly, which will greatly confuse the user. Based on the original intention, we should skip the cache directly in similar situations. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45018 from panbingkun/branch-3.4_SPARK-46400. Authored-by: panbingkun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

…ocal maven repo, skip this cache and try again (apache#367) ### What changes were proposed in this pull request? The pr aims to - fix potential bug(ie: apache#44208) and enhance user experience. - make the code more compliant with standards Backport above to branch 3.5. Master branch pr: apache#44343 ### Why are the changes needed? We use the local maven repo as the first-level cache in ivy. The original intention was to reduce the time required to parse and obtain the ar, but when there are corrupted files in the local maven repo,The above mechanism will be directly interrupted and the prompt is very unfriendly, which will greatly confuse the user. Based on the original intention, we should skip the cache directly in similar situations. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45017 from panbingkun/branch-3.5_SPARK-46400. Authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]> Co-authored-by: panbingkun <[email protected]>

panbingkun added 2 commits December 14, 2023 11:00

[SPARK-46400][CORE][SQL] When there are corrupted files in the local …

8bdf9e1

…maven repo, try to skip this cache

Trigger build

a0848b8

panbingkun changed the title ~~[SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, retry to skip this cache~~ [SPARK-46400][CORE][SQL] When there are damaged files in the local maven repo, skip this cache and try again Dec 14, 2023

panbingkun changed the title ~~[SPARK-46400][CORE][SQL] When there are damaged files in the local maven repo, skip this cache and try again~~ [SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again Dec 14, 2023

panbingkun added 2 commits December 14, 2023 11:14

Trigger build

fd78f4d

Merge branch 'master' into SPARK-46400

5a9dafc

panbingkun mentioned this pull request Dec 14, 2023

[SPARK-46302][TESTS] Skip maven daily testing as ivy uses some corrupted cache jar files #44208

Closed

LuciferYang reviewed Dec 14, 2023

View reviewed changes

panbingkun added 4 commits December 15, 2023 09:08

update

a7f5497

[SPARK-46400][CORE][SQL] When there are corrupted files in the local …

44400c9

…maven repo, skip this cache and try again

Merge branch 'master' into SPARK-46400

bd705f1

Merge branch 'master' into SPARK-46400

264a084

github-actions bot added the SQL label Dec 17, 2023

panbingkun requested a review from LuciferYang December 17, 2023 04:31

panbingkun commented Dec 17, 2023

View reviewed changes

LuciferYang reviewed Dec 20, 2023

View reviewed changes

srowen closed this in f2a471e Jan 31, 2024

This was referenced Feb 4, 2024

[SPARK-46400][CORE][SQL][3.5] When there are corrupted files in the local maven repo, skip this cache and try again #45017

Closed

[SPARK-46400][CORE][SQL][3.4] When there are corrupted files in the local maven repo, skip this cache and try again #45018

Closed

[SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again #44343

[SPARK-46400][CORE][SQL] When there are corrupted files in the local maven repo, skip this cache and try again #44343

Uh oh!

Conversation

panbingkun commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

panbingkun commented Dec 14, 2023

Uh oh!

LuciferYang Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LuciferYang commented Jan 30, 2024

Uh oh!

srowen commented Jan 30, 2024

Uh oh!

mridulm commented Jan 30, 2024

Uh oh!

panbingkun commented Jan 31, 2024

Uh oh!

srowen commented Jan 31, 2024

Uh oh!

LuciferYang commented Feb 1, 2024

Uh oh!

panbingkun commented Feb 2, 2024

Uh oh!

dongjoon-hyun commented Feb 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

panbingkun commented Dec 14, 2023 •

edited

Loading

LuciferYang Dec 14, 2023 •

edited

Loading

LuciferYang Dec 14, 2023 •

edited

Loading

LuciferYang Dec 14, 2023 •

edited

Loading

LuciferYang left a comment •

edited

Loading

LuciferYang commented Jan 2, 2024 •

edited

Loading

dongjoon-hyun commented Feb 15, 2024 •

edited

Loading