You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "longvu-db (via GitHub)" <gi...@apache.org> on 2026/07/28 18:18:33 UTC
[PR] [CONNECT] Skip redundant DSv2 table refreshes in Spark Connect [spark]
longvu-db opened a new pull request, #57609:
URL: https://github.com/apache/spark/pull/57609
### What changes were proposed in this pull request?
This PR implements the server-side SQL configuration approach suggested in https://github.com/apache/spark/pull/57043#issuecomment-5097384606.
It adds the internal `spark.sql.execution.skipV2TableRefresh` configuration, defaulting to `false`, and sets it to `true` whenever the Spark Connect server creates or clones a session. `QueryExecution` uses this setting when deriving its effective `refreshPhaseEnabled` value, so the optimization covers every Connect path that creates a query execution, including SQL, DataFrame operations, checkpoint, local checkpoint, and future call sites.
Spark Connect's plan cache is handled conservatively. A cache hit can contain an already-resolved `DataSourceV2Relation`, for which analysis alone cannot reload current table metadata. Such cached plans are cloned and tagged so their `QueryExecution` retains the refresh phase. Freshly analyzed Connect plans still skip it.
The existing `QueryExecution` constructor signature and `refreshPhaseEnabled` accessor remain binary-compatible.
### Why are the changes needed?
Spark Connect normally analyzes a plan immediately before execution. The DSv2 table refresh phase then performs another catalog `loadTable` request without changing the outcome, adding an avoidable catalog round trip and latency.
Using `spark.api.mode` on the server is not reliable because it is a client-side builder/launcher directive and is not propagated to Spark Connect server sessions. A server-side session setting explicitly records the execution environment and avoids having to update every Connect call site.
The plan-cache exception preserves correctness for resolved DSv2 relations that may outlive the table metadata captured during their original analysis.
### Does this PR introduce _any_ user-facing change?
No. The new configuration is internal. Spark Classic keeps the refresh phase enabled, while Spark Connect avoids redundant catalog refresh requests for freshly analyzed plans.
### How was this patch tested?
Added a counting DSv2 catalog and tests that assert the exact number of `loadTable` calls, not only query results. Coverage includes:
- Classic defaults and explicit refresh skipping.
- Connect-created sessions and cloned sessions.
- DataFrame `table`, `drop`, SQL, checkpoint, and local checkpoint paths.
- Repeated execution through the Connect plan cache, where cached resolved DSv2 plans retain the refresh.
- Existing Connect DSv2 freshness scenarios.
The following suites passed (119 tests total):
- `QueryExecutionSuite`: 29 tests.
- `SparkConnectSessionManagerSuite`: 11 tests.
- `SparkConnectCloneSessionSuite`: 9 tests.
- `SparkConnectServerTestSuite`: 21 tests.
- `DataSourceV2DataFrameConnectSuite`: 49 tests.
Compilation passed for `catalyst/Test/compile`, `sql/Test/compile`, and `connect/Test/compile`. Scalastyle passed for the catalyst, SQL, and Connect modules.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex
--
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] [CONNECT] Skip redundant DSv2 table refreshes in Spark Connect [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #57609:
URL: https://github.com/apache/spark/pull/57609#discussion_r3712982195
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala:
##########
@@ -729,8 +733,21 @@ case object RemoveShuffleFiles extends ShuffleCleanupMode
object QueryExecution {
private val _nextExecutionId = new AtomicLong(0)
+ private[sql] val REQUIRES_V2_TABLE_REFRESH =
+ TreeNodeTag[Unit]("requires_v2_table_refresh")
+
private def nextExecutionId: Long = _nextExecutionId.getAndIncrement
+ private def defaultRefreshPhaseEnabled(
+ sparkSession: SparkSession,
+ logical: LogicalPlan): Boolean = {
+ val skipRefresh = sparkSession.sessionState.conf.getConf(SQLConf.SKIP_V2_TABLE_REFRESH)
+ val requiresRefresh = logical.collectWithSubqueries {
+ case plan if plan.getTagValue(REQUIRES_V2_TABLE_REFRESH).isDefined => true
Review Comment:
This runs on every `QueryExecution` construction, and `collectWithSubqueries { ... }.nonEmpty` materializes a `Seq[Boolean]` of all matches just to test existence. `collectFirstWithSubqueries { ... }.isDefined` short-circuits at the first tagged node with the same result -- worth it on a per-query path.
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SessionHolder.scala:
##########
@@ -617,7 +619,16 @@ case class SessionHolder(userId: String, sessionId: String, session: SparkSessio
Option(cache.getIfPresent(rel)) match {
case Some(plan) =>
logDebug(s"Using cached plan for relation '$rel': $plan")
- Some(plan)
+ val hasResolvedV2Table = plan.collectWithSubqueries {
+ case _: DataSourceV2Relation => true
Review Comment:
Same existence-check pattern here: `collectFirstWithSubqueries { case _: DataSourceV2Relation => true }.isDefined` short-circuits instead of collecting all matches, identical boolean semantics.
--
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] [CONNECT] Skip redundant DSv2 table refreshes in Spark Connect [spark]
Posted by "github-actions[bot] (via GitHub) via reviews" <re...@spark.apache.org>.
github-actions[bot] closed pull request #57609: [CONNECT] Skip redundant DSv2 table refreshes in Spark Connect
URL: https://github.com/apache/spark/pull/57609
--
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] [CONNECT] Skip redundant DSv2 table refreshes in Spark Connect [spark]
Posted by "github-actions[bot] (via GitHub) via reviews" <re...@spark.apache.org>.
github-actions[bot] commented on PR #57609:
URL: https://github.com/apache/spark/pull/57609#issuecomment-6871760624
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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