venkata91 commented on code in PR #59277:
URL: https://github.com/apache/spark/pull/59277#discussion_r4265122237
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala:
##########
@@ -150,6 +150,28 @@ object Connect {
.intConf
.createWithDefaultString("1000")
+ val CONNECT_SESSION_ALLOW_RECONNECT_AFTER_CLOSE =
+ buildConf("spark.connect.session.allowReconnectAfterClose")
+ .doc(
+ "When true, the session id can be reused after this session is closed,
for example " +
+ "on inactivity expiry. The id stays reserved, and requests fail with
" +
+ "INVALID_HANDLE.SESSION_CLOSED, until the close finishes. After
that, a client that " +
+ "used the closed session gets INVALID_HANDLE.SESSION_CHANGED if it
sends its " +
+ "observed server-side session id, as the built-in Python and Scala
clients do. " +
Review Comment:
Good catch, thanks. Fixed in 8496d1ff41b: the Python and Scala
`ArtifactManager`s now set `client_observed_server_side_session_id` on
`AddArtifactsRequest` (batch, begin-chunk and chunk) and
`ArtifactStatusesRequest` whenever the client has observed a server-side
session id. No proto change was needed because the server handlers already read
the field. So the doc statement now holds for artifact RPCs as well.
Tests:
- `ArtifactSuite`: checks that artifact requests carry the field once an id
has been observed, and not before.
- `test_artifact.py`: the same check for the Python client.
- `SparkConnectSessionManagerSuite`: after an opted-in expiry, a stale
`ArtifactStatuses` request through the handler gets `SESSION_CHANGED` and no
session is created.
Separately, 3c52f0e14b5 re-checks the closed fence inside the atomic
`computeIfAbsent` for `cloneSession` and `registerExistingSession`, matching
`getOrCreateIsolatedSession`.
--
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]
