Repository navigation
[SPARK-44462][4.3][SS][CONNECT] Fix the session passed to foreachBatch in Spark Connect - #59200
Closed
LuciferYang wants to merge 2 commits into
Closed
LuciferYang wants to merge 2 commits into
LuciferYang wants to merge 2 commits into
Conversation
…Spark Connect Spark Connect's Python `foreachBatch` cached the batch DataFrame in the root session's `SessionHolder`, but the batch DataFrame belongs to `StreamExecution.sparkSessionForStream`, a clone of the session that started the query. This PR gives that clone its own `SessionHolder` and routes per-batch work through it, so the batch runs under the configuration the streaming engine pins on the clone rather than the root session's. The behavior is gated behind a new SQL config `spark.sql.connect.streaming.foreachBatch.useClonedSession` (default `true`); setting it to `false` restores the previous root-session behavior. On the server, `StreamingForeachBatchHelper` adds: - `ForeachBatchSessionManager`. On the first batch it registers `batchDf.sparkSession` under a server-generated session id through the new `SparkConnectSessionManager.registerExistingSession`, with `customInactiveTimeoutMs = -1` so it never expires by inactivity (the query owns its lifetime). The id is pinned for the whole query, so every batch resolves to the same holder. `close()` is terminal. When `useClonedSession` is `false` it registers no cloned holder and returns the root holder instead. - `ForeachBatchCleaner`, replacing `RunnerCleaner`. It closes the Python runner (when there is one) and the cloned holder. `CleanerCache.registerCleanerForQuery` also closes eagerly if the query is already inactive at registration time, since `onQueryTerminated` never fires for such a query. It is registered before `registerNewStreamingQuery` so a throw there cannot strand the never-expiring cloned holder. - A stale-DataFrame check keyed by streaming query id, so a batch DataFrame left in the cache by a previous batch is evicted. The server writes `(dfId, batchId, sessionId)` per batch, where the session id is the cloned session's when the fix is on and the root session's when it is off. On the Python side, `foreach_batch_worker` bootstraps on the root session id to unpickle the user function before the first batch, then creates a session on the provided id and repoints `SparkSession._default_session` / `_active_session` at it, so `SparkSession.active()` inside the user function returns the session the batch runs on. The Scala `foreachBatch` path receives the classic DataFrame directly and never resolves a remote batch relation, so it does not register a cloned `SessionHolder`. The cloned session id is logged under a `CLONED_SESSION_ID` key so the structured field matches the value. `StreamExecution` runs the query on a clone of the session it was started with, and the engine forces onto that clone the configs stateful correctness depends on: `spark.sql.cbo.enabled=false` and `spark.sql.requireAllClusterKeysForDistribution=false` so stateful operators are not rearranged, AQE off for continuous, real-time, and stateful workloads, and the stateful shuffle partition count pinned at start. `populateStartOffsets` also restores the checkpointed conf from the offset log onto the same session. Connect resolved the batch DataFrame against the root `SessionHolder`, so operations the user function performed on it were planned and executed under the root session's configuration, with none of those settings in effect. The Python worker also kept the root session active, so the Connect session mapping never expired and the query kept running after the client disappeared. Yes, for Python `foreachBatch` under Spark Connect, with `spark.sql.connect.streaming.foreachBatch.useClonedSession` at its default of `true`. The batch DataFrame is now bound to the cloned session, matching classic `foreachBatch`, which already runs the user function on the clone (`ForeachBatchSink.addBatch` builds the batch Dataset with `ofRows(data.sparkSession, node)`). Because the batch DataFrame no longer shares a session with the client: - `SparkSession.active()` inside the function returns the batch session rather than the root one. - A temporary view created from the batch DataFrame is not visible to the root session. - Referencing the batch DataFrame from `spark.sql` on a captured root session fails with `DATAFRAME_NOT_FOUND`, and joining it with a DataFrame captured from the root session fails with `SESSION_NOT_SAME`. Combining the batch DataFrame with the root session inside the function is not supported by design: the isolation that fixes the configuration bug is the same thing that separates the two sessions. Run such operations through the batch DataFrame's own session (`batch_df.sparkSession`), and pass results out of the function through a table or a global temporary view instead of a session-local temporary view. To keep the previous behavior, set `spark.sql.connect.streaming.foreachBatch.useClonedSession` to `false`: the batch DataFrame is then bound to the root session again and can be combined with it, at the cost of not running under the stream session's configuration. The Structured Streaming migration guide documents this. New tests in `StreamingForeachBatchParityTests`: - `test_batch_uses_stream_session_for_stateful_aqe` pins that the batch runs on a different session than the root, with AQE off on the batch session while the root has it on. - `test_batch_dataframe_in_root_session_sql_is_unsupported` and `test_batch_dataframe_join_with_root_session_dataframe_is_unsupported` pin the contract under the default: `spark.sql` referencing the batch DataFrame from the root session fails with `DATAFRAME_NOT_FOUND`, and joining it with a captured root-session DataFrame fails with `SESSION_NOT_SAME`. - `test_batch_dataframe_in_batch_session_sql_is_supported` and `test_batch_dataframe_join_in_batch_session_is_supported` pin the documented migration: the same work run through `batch_df.sparkSession` succeeds. - `test_temp_view_is_isolated_from_root_session` pins that a temp view created inside the function resolves through the cloned session but not the root session, with a sentinel table proving the function ran. - `test_disabling_fix_restores_root_session_behavior` pins that with the config set to `false`, joining the batch DataFrame with a captured root-session DataFrame succeeds again. New tests in `StreamingForeachBatchHelperSuite`: `ForeachBatchSessionManager.close()` is terminal and a closing root session is refused; a cleaner registered for an already terminated query is closed on the spot. Updated tests: `test_nested_dataframes` now uses `saveAsTable` instead of `createOrReplaceTempView`, and `SparkConnectSessionHolderSuite` destructures the new 3-tuple from `pythonForeachBatchWrapper`. Pass GitHub Actions. Generated-by: Claude Code Closes apache#55410 from LuciferYang/SPARK-44462-foreachBatch-session-fix. Lead-authored-by: YangJie <yangjie01@baidu.com> Co-authored-by: reviewer <reviewer@local> Signed-off-by: Jungtaek Lim <kabhwan.opensource@gmail.com> (cherry picked from commit 362ee55) Signed-off-by: Jungtaek Lim <kabhwan.opensource@gmail.com> (cherry picked from commit b7503aa)
HyukjinKwon
approved these changes
Oct 2, 2026
LuciferYang
added a commit
that referenced
this pull request
Oct 2, 2026
…gisters the cloned session ### What changes were proposed in this pull request? Followup to #55410. The two `ForeachBatchSessionManager` cleanup tests in `StreamingForeachBatchHelperSuite` checked that `close()` is terminal and that the registered cleaner runs, but neither asserted the side effect `close()` exists to guarantee: that the cloned `SessionHolder` is removed from `SparkConnectService.sessionManager`. Because `close()` sets its `closed` flag independently of the `closeSession` call, a future change that dropped or broke the unregister step would still pass both tests while re-introducing the never-expiring-holder leak. This adds assertions that the cloned `SessionKey` is present in the session manager before `close()` and absent after. ### Why are the changes needed? To make the cloned-session leak-prevention contract regression-sensitive. Spotted in review of #59200, the branch-4.3 backport. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GitHub Actions. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #59202 from LuciferYang/SPARK-44462-assert-cloned-session-unregister. Lead-authored-by: YangJie <yangjie01@baidu.com> Co-authored-by: reviewer <reviewer@local> Signed-off-by: yangjie01 <yangjie01@baidu.com>
LuciferYang
added a commit
that referenced
this pull request
Oct 2, 2026
…gisters the cloned session ### What changes were proposed in this pull request? Followup to #55410. The two `ForeachBatchSessionManager` cleanup tests in `StreamingForeachBatchHelperSuite` checked that `close()` is terminal and that the registered cleaner runs, but neither asserted the side effect `close()` exists to guarantee: that the cloned `SessionHolder` is removed from `SparkConnectService.sessionManager`. Because `close()` sets its `closed` flag independently of the `closeSession` call, a future change that dropped or broke the unregister step would still pass both tests while re-introducing the never-expiring-holder leak. This adds assertions that the cloned `SessionKey` is present in the session manager before `close()` and absent after. ### Why are the changes needed? To make the cloned-session leak-prevention contract regression-sensitive. Spotted in review of #59200, the branch-4.3 backport. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GitHub Actions. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #59202 from LuciferYang/SPARK-44462-assert-cloned-session-unregister. Lead-authored-by: YangJie <yangjie01@baidu.com> Co-authored-by: reviewer <reviewer@local> Signed-off-by: yangjie01 <yangjie01@baidu.com> (cherry picked from commit 38422ae) Signed-off-by: yangjie01 <yangjie01@baidu.com>
Contributor
|
Thanks! Merging to 4.3. |
HeartSaVioR
pushed a commit
that referenced
this pull request
Oct 2, 2026
…h in Spark Connect ### What changes were proposed in this pull request? Spark Connect's Python `foreachBatch` cached the batch DataFrame in the root session's `SessionHolder`, but the batch DataFrame belongs to `StreamExecution.sparkSessionForStream`, a clone of the session that started the query. This PR gives that clone its own `SessionHolder` and routes per-batch work through it, so the batch runs under the configuration the streaming engine pins on the clone rather than the root session's. The behavior is gated behind a new SQL config `spark.sql.connect.streaming.foreachBatch.useClonedSession` (default `true`); setting it to `false` restores the previous root-session behavior. On the server, `StreamingForeachBatchHelper` adds: - `ForeachBatchSessionManager`. On the first batch it registers `batchDf.sparkSession` under a server-generated session id through the new `SparkConnectSessionManager.registerExistingSession`, with `customInactiveTimeoutMs = -1` so it never expires by inactivity (the query owns its lifetime). The id is pinned for the whole query, so every batch resolves to the same holder. `close()` is terminal. When `useClonedSession` is `false` it registers no cloned holder and returns the root holder instead. - `ForeachBatchCleaner`, replacing `RunnerCleaner`. It closes the Python runner (when there is one) and the cloned holder. `CleanerCache.registerCleanerForQuery` also closes eagerly if the query is already inactive at registration time, since `onQueryTerminated` never fires for such a query. It is registered before `registerNewStreamingQuery` so a throw there cannot strand the never-expiring cloned holder. - A stale-DataFrame check keyed by streaming query id, so a batch DataFrame left in the cache by a previous batch is evicted. The server writes `(dfId, batchId, sessionId)` per batch, where the session id is the cloned session's when the fix is on and the root session's when it is off. On the Python side, `foreach_batch_worker` bootstraps on the root session id to unpickle the user function before the first batch, then creates a session on the provided id and repoints `SparkSession._default_session` / `_active_session` at it, so `SparkSession.active()` inside the user function returns the session the batch runs on. The Scala `foreachBatch` path receives the classic DataFrame directly and never resolves a remote batch relation, so it does not register a cloned `SessionHolder`. The cloned session id is logged under a `CLONED_SESSION_ID` key so the structured field matches the value. ### Why are the changes needed? `StreamExecution` runs the query on a clone of the session it was started with, and the engine forces onto that clone the configs stateful correctness depends on: `spark.sql.cbo.enabled=false` and `spark.sql.requireAllClusterKeysForDistribution=false` so stateful operators are not rearranged, AQE off for continuous, real-time, and stateful workloads, and the stateful shuffle partition count pinned at start. `populateStartOffsets` also restores the checkpointed conf from the offset log onto the same session. Connect resolved the batch DataFrame against the root `SessionHolder`, so operations the user function performed on it were planned and executed under the root session's configuration, with none of those settings in effect. The Python worker also kept the root session active, so the Connect session mapping never expired and the query kept running after the client disappeared. ### Does this PR introduce _any_ user-facing change? Yes, for Python `foreachBatch` under Spark Connect, with `spark.sql.connect.streaming.foreachBatch.useClonedSession` at its default of `true`. The batch DataFrame is now bound to the cloned session, matching classic `foreachBatch`, which already runs the user function on the clone (`ForeachBatchSink.addBatch` builds the batch Dataset with `ofRows(data.sparkSession, node)`). Because the batch DataFrame no longer shares a session with the client: - `SparkSession.active()` inside the function returns the batch session rather than the root one. - A temporary view created from the batch DataFrame is not visible to the root session. - Referencing the batch DataFrame from `spark.sql` on a captured root session fails with `DATAFRAME_NOT_FOUND`, and joining it with a DataFrame captured from the root session fails with `SESSION_NOT_SAME`. Combining the batch DataFrame with the root session inside the function is not supported by design: the isolation that fixes the configuration bug is the same thing that separates the two sessions. Run such operations through the batch DataFrame's own session (`batch_df.sparkSession`), and pass results out of the function through a table or a global temporary view instead of a session-local temporary view. To keep the previous behavior, set `spark.sql.connect.streaming.foreachBatch.useClonedSession` to `false`: the batch DataFrame is then bound to the root session again and can be combined with it, at the cost of not running under the stream session's configuration. The Structured Streaming migration guide documents this. ### How was this patch tested? New tests in `StreamingForeachBatchParityTests`: - `test_batch_uses_stream_session_for_stateful_aqe` pins that the batch runs on a different session than the root, with AQE off on the batch session while the root has it on. - `test_batch_dataframe_in_root_session_sql_is_unsupported` and `test_batch_dataframe_join_with_root_session_dataframe_is_unsupported` pin the contract under the default: `spark.sql` referencing the batch DataFrame from the root session fails with `DATAFRAME_NOT_FOUND`, and joining it with a captured root-session DataFrame fails with `SESSION_NOT_SAME`. - `test_batch_dataframe_in_batch_session_sql_is_supported` and `test_batch_dataframe_join_in_batch_session_is_supported` pin the documented migration: the same work run through `batch_df.sparkSession` succeeds. - `test_temp_view_is_isolated_from_root_session` pins that a temp view created inside the function resolves through the cloned session but not the root session, with a sentinel table proving the function ran. - `test_disabling_fix_restores_root_session_behavior` pins that with the config set to `false`, joining the batch DataFrame with a captured root-session DataFrame succeeds again. New tests in `StreamingForeachBatchHelperSuite`: `ForeachBatchSessionManager.close()` is terminal and a closing root session is refused; a cleaner registered for an already terminated query is closed on the spot. Updated tests: `test_nested_dataframes` now uses `saveAsTable` instead of `createOrReplaceTempView`, and `SparkConnectSessionHolderSuite` destructures the new 3-tuple from `pythonForeachBatchWrapper`. Pass GitHub Actions. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #59200 from LuciferYang/SPARK-44462-branch-4.3. Lead-authored-by: YangJie <yangjie01@baidu.com> Co-authored-by: reviewer <reviewer@local> Signed-off-by: Jungtaek Lim <kabhwan.opensource@gmail.com>
Contributor
|
Merge Summary:
|
Contributor
Author
|
Thank you @HeartSaVioR and @HyukjinKwon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Spark Connect's Python
foreachBatchcached the batch DataFrame in the root session'sSessionHolder, but the batch DataFrame belongs toStreamExecution.sparkSessionForStream, a clone of the session that started the query. This PR gives that clone its ownSessionHolderand routes per-batch work through it, so the batch runs under the configuration the streaming engine pins on the clone rather than the root session's. The behavior is gated behind a new SQL configspark.sql.connect.streaming.foreachBatch.useClonedSession(defaulttrue); setting it tofalserestores the previous root-session behavior.On the server,
StreamingForeachBatchHelperadds:ForeachBatchSessionManager. On the first batch it registersbatchDf.sparkSessionunder a server-generated session id through the newSparkConnectSessionManager.registerExistingSession, withcustomInactiveTimeoutMs = -1so it never expires by inactivity (the query owns its lifetime). The id is pinned for the whole query, so every batch resolves to the same holder.close()is terminal. WhenuseClonedSessionisfalseit registers no cloned holder and returns the root holder instead.ForeachBatchCleaner, replacingRunnerCleaner. It closes the Python runner (when there is one) and the cloned holder.CleanerCache.registerCleanerForQueryalso closes eagerly if the query is already inactive at registration time, sinceonQueryTerminatednever fires for such a query. It is registered beforeregisterNewStreamingQueryso a throw there cannot strand the never-expiring cloned holder.The server writes
(dfId, batchId, sessionId)per batch, where the session id is the cloned session's when the fix is on and the root session's when it is off. On the Python side,foreach_batch_workerbootstraps on the root session id to unpickle the user function before the first batch, then creates a session on the provided id and repointsSparkSession._default_session/_active_sessionat it, soSparkSession.active()inside the user function returns the session the batch runs on.The Scala
foreachBatchpath receives the classic DataFrame directly and never resolves a remote batch relation, so it does not register a clonedSessionHolder. The cloned session id is logged under aCLONED_SESSION_IDkey so the structured field matches the value.Why are the changes needed?
StreamExecutionruns the query on a clone of the session it was started with, and the engine forces onto that clone the configs stateful correctness depends on:spark.sql.cbo.enabled=falseandspark.sql.requireAllClusterKeysForDistribution=falseso stateful operators are not rearranged, AQE off for continuous, real-time, and stateful workloads, and the stateful shuffle partition count pinned at start.populateStartOffsetsalso restores the checkpointed conf from the offset log onto the same session. Connect resolved the batch DataFrame against the rootSessionHolder, so operations the user function performed on it were planned and executed under the root session's configuration, with none of those settings in effect. The Python worker also kept the root session active, so the Connect session mapping never expired and the query kept running after the client disappeared.Does this PR introduce any user-facing change?
Yes, for Python
foreachBatchunder Spark Connect, withspark.sql.connect.streaming.foreachBatch.useClonedSessionat its default oftrue. The batch DataFrame is now bound to the cloned session, matching classicforeachBatch, which already runs the user function on the clone (ForeachBatchSink.addBatchbuilds the batch Dataset withofRows(data.sparkSession, node)). Because the batch DataFrame no longer shares a session with the client:SparkSession.active()inside the function returns the batch session rather than the root one.spark.sqlon a captured root session fails withDATAFRAME_NOT_FOUND, and joining it with a DataFrame captured from the root session fails withSESSION_NOT_SAME.Combining the batch DataFrame with the root session inside the function is not supported by design: the isolation that fixes the configuration bug is the same thing that separates the two sessions. Run such operations through the batch DataFrame's own session (
batch_df.sparkSession), and pass results out of the function through a table or a global temporary view instead of a session-local temporary view. To keep the previous behavior, setspark.sql.connect.streaming.foreachBatch.useClonedSessiontofalse: the batch DataFrame is then bound to the root session again and can be combined with it, at the cost of not running under the stream session's configuration. The Structured Streaming migration guide documents this.How was this patch tested?
New tests in
StreamingForeachBatchParityTests:test_batch_uses_stream_session_for_stateful_aqepins that the batch runs on a different session than the root, with AQE off on the batch session while the root has it on.test_batch_dataframe_in_root_session_sql_is_unsupportedandtest_batch_dataframe_join_with_root_session_dataframe_is_unsupportedpin the contract under the default:spark.sqlreferencing the batch DataFrame from the root session fails withDATAFRAME_NOT_FOUND, and joining it with a captured root-session DataFrame fails withSESSION_NOT_SAME.test_batch_dataframe_in_batch_session_sql_is_supportedandtest_batch_dataframe_join_in_batch_session_is_supportedpin the documented migration: the same work run throughbatch_df.sparkSessionsucceeds.test_temp_view_is_isolated_from_root_sessionpins that a temp view created inside the function resolves through the cloned session but not the root session, with a sentinel table proving the function ran.test_disabling_fix_restores_root_session_behaviorpins that with the config set tofalse, joining the batch DataFrame with a captured root-session DataFrame succeeds again.New tests in
StreamingForeachBatchHelperSuite:ForeachBatchSessionManager.close()is terminal and a closing root session is refused; a cleaner registered for an already terminated query is closed on the spot.Updated tests:
test_nested_dataframesnow usessaveAsTableinstead ofcreateOrReplaceTempView, andSparkConnectSessionHolderSuitedestructures the new 3-tuple frompythonForeachBatchWrapper.Pass GitHub Actions.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code