venkata91 commented on PR #59277: URL: https://github.com/apache/spark/pull/59277#issuecomment-6049326811
Thanks @dongjoon-hyun for the thorough review. Agreed with [1]–[4]: invalidating after the fact can't coordinate with an in-flight shutdown, and it leaves the old client able to interfere. Reworked to your option (1): - New session-scoped conf `spark.connect.session.allowReconnectAfterClose` (default `false`), set by the client on a live session. - `removeSessionHolder` always tombstones first. `shutdownSessionHolder` drops the tombstone only after `close()` completes, and only if the release allows reconnect or the session opted in. This also closes the same window for the existing `allow_reconnect=true` path. - Reverted the client and proto changes. On the use case: our Spark Connect gateway derives a stable session id from (user, app, job type, cluster). That lets notebooks reattach after a kernel restart and lets the proxy route by session id. Once such a session expires, every reconnect hits `SESSION_CLOSED` for as long as the tombstone lives, and the only workaround is to change the id. These clients already rerun from scratch after expiry, so a clean new session under the same id is what we want. PTAL when you get a chance. 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
