[SPARK-45856] Move ArtifactManager from Spark Connect into SparkSession (sql/core)

[vicennial]

What changes were proposed in this pull request?

The significant changes in this PR include:

• SparkConnectArtifactManager is renamed to ArtifactManager and moved out of Spark Connect and into sql/core (available in SparkSession through SessionState) along with all corresponding tests and confs.
• While ArtifactManager in part of SparkSession, we keep the legacy behaviour for non-connect spark while utilising the ArtifactManager in connect pathways
  • This is done by exposing a new method withResources in the artifact manager that sets the context class loader (for driver-side operations) and propagates the JobArtifactState such that the resources reach the executor.
  • Spark Connect pathways utilise this method through the SessionHolder#withActive
  • When withResources is not used, neither the custom context classloader nor the JobArtifactState is propagated and hence, non Spark Connect pathways remain with legacy behaviour.

Why are the changes needed?

The ArtifactManager that currently lies in the connect package can be moved into the wider sql/core package (e.g SparkSession) to expand the scope. This is possible because the ArtifactManager is tied solely to the SparkSession#sessionUUID and hence can be cleanly detached from Spark Connect and be made generally available.

Does this PR introduce any user-facing change?

No. Existing behaviour is kept intact for both non-connect and connect spark.

How was this patch tested?

Existing tests.

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

No.

[vicennial]

PTAL @hvanhovell @HyukjinKwon

[hvanhovell]

LGTM

[dongjoon-hyun]

According to the JIRA, this is only for Apache Spark 4.0.0, right?

[dongjoon-hyun]

We still need to support configurations in a way because we didn't deprecate yet in the Apache Spark community.

spark.connect.copyFromLocalToFs.allowDestLocal

[dongjoon-hyun]

This should be 4.0.0 because this PR is for Apache Spark 4.0.0, @vicennial .

[dongjoon-hyun]

Could you re-trigger the failed pipelines?

[vicennial]

@dongjoon-hyun Thank you for the review!

I've updated the version and for the deprecated conf spark.connect.copyFromLocalToFs.allowDestLocal, I've added it to the deprecatedSQLConfigs list so that it would generate a warning if used. Currently, both configs would work but if the new config is set (currently optional), it would override the deprecated config. Eventually, we should remove the deprecated conf and convert the new config from optional to have a default value.

[vicennial]

Hmm, JavaDocGeneration is failing in 2 tests and from the logs, its not clear why...
I will merge master and see if that helps

[vicennial]

@dongjoon-hyun The CI is green now :)

[HyukjinKwon]

Merged to master.

[fhalde]

Hi, we are super interested in having the isolated classloader per spark session ability for our usecase. i believe this today is only achievable if jobs are run from a connect client. we want to avoid using connect client

but with this pr merged, it should be possible to have isolated classloaders per spark session on the executors right?

our use-case involves starting a spark driver and dynamically loading/adding jars and running transformations present within the jars. without isolated classloaders per session, on the executor side we would risk classpath conflict

@HyukjinKwon if your pr addresses my concern, i can back port it to 3.5.0 in my fork

[vicennial]

@fhalde Yes, with this PR, it would be possible to have isolated classloaders per spark session on the executors without going through Spark Connect.

The withResources method should be used to wrap all executions (like in Spark Connect here) and note that, all artifacts (i.e Jars, classfiles) would need to be added through the ArtifactManager (directly adding these to SparkContext would not work), refer to Spark Connect's AddArtifactsHandler to see how we use the API.

[LuciferYang]

@vicennial I would like to reconfirm, the ProblemFilters added by SPARK-45856 will never need to undergo a mima check in versions after Spark 4.0, is that correct? Or is this just the ProblemFilters added for the mima check between Spark 4.0 and Spark 3.5?I found that it has been placed in defaultExcludes.