dongjoon-hyun commented on code in PR #53260:
URL: https://github.com/apache/spark/pull/53260#discussion_r2577758931
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectStatement.scala:
##########
@@ -35,14 +35,21 @@ class SparkConnectStatement(conn: SparkConnectConnection)
extends Statement {
override def close(): Unit = synchronized {
if (!closed) {
if (operationId != null) {
- conn.spark.interruptOperation(operationId)
+ try {
+ conn.spark.interruptOperation(operationId)
+ } catch {
+ case _: java.net.ConnectException =>
+ // Ignore ConnectExceptions during cleanup as the operation may
have already completed
+ // or the server may be unavailable. The important part is marking
this statement
+ // as closed to prevent further use.
Review Comment:
To @pan3793, I merged this PR to RC2 because I agreed three ways.
1. We need `closed = true` fix as you agreed to unblock RC2 release. This is
a blocker issue for JDBC module itself in RC2 because it's an obvious mistake
on a normal use case. However, `java.net.ConnectionException` is not a
blocker-level issue because it's not a normal case.
2. In the server-client operation, I believe `java.net.ConnectException` can
be ignored in `close()` semantic because this PR is not proposing the
double-`close` semantic. IIUC, this is a reasonable behavior to mark `onClose`
flag during the `close` operator even with the faulty resource.
3. As a result, I believe this is helpful. Since `closed = true` is fixed by
this PR, if a user or some next test case tries a new operation,
- It should fail from the beginning at `isClosed` check instead of doing
something. This is a correct behavior after invoking `close` operation.
- Or, It should open a new server since the connection is closed.
If we want to some capability for audit, we can do `logInfo` or `logWarning`
here instead of propagating errors.
--
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]
