This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-java.git
The following commit(s) were added to refs/heads/main by this push:
new 2329174e0 GH-863: [JDBC] Suppress benign exceptions from gRPC layer on
ArrowFlightSqlClientHandler#close (#910)
2329174e0 is described below
commit 2329174e0bf35733b5ac99b7e36465a601ae20e8
Author: Pedro Matias <[email protected]>
AuthorDate: Thu Dec 4 09:33:49 2025 +0000
GH-863: [JDBC] Suppress benign exceptions from gRPC layer on
ArrowFlightSqlClientHandler#close (#910)
## What's Changed
When using the Flight SQL JDBC driver with connection pooling and a
catalog parameter, ArrowFlightSqlClientHandler.close() performs a
CloseSession RPC that can fail during gRPC channel shutdown.
These transient failures (UNAVAILABLE or INTERNAL with "Connection
closed after GOAWAY") cause noisy errors in pooling frameworks like
Apache Commons DBCP.
With this PR these exceptions will instead be suppressed and logged,
following the procedure that was used for
[ARROW-17785](https://issues.apache.org/jira/browse/ARROW-17785)
### Are these changes tested?
Yes
Closes #863
---------
Co-authored-by: Vando Pereira <[email protected]>
---
.../jdbc/client/ArrowFlightSqlClientHandler.java | 81 +++++++++++++++++---
.../client/ArrowFlightSqlClientHandlerTest.java | 88 ++++++++++++++++++++++
2 files changed, 160 insertions(+), 9 deletions(-)
diff --git
a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java
b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java
index a3f690037..5dc7e0e2e 100644
---
a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java
+++
b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java
@@ -262,15 +262,85 @@ public final class ArrowFlightSqlClientHandler implements
AutoCloseable {
@Override
public void close() throws SQLException {
if (catalog.isPresent()) {
- sqlClient.closeSession(new CloseSessionRequest(), getOptions());
+ try {
+ sqlClient.closeSession(new CloseSessionRequest(), getOptions());
+ } catch (FlightRuntimeException fre) {
+ handleBenignCloseException(
+ fre, "Failed to close Flight SQL session.", "closing Flight SQL
session");
+ }
}
try {
AutoCloseables.close(sqlClient);
+ } catch (FlightRuntimeException fre) {
+ handleBenignCloseException(
+ fre, "Failed to clean up client resources.", "closing Flight SQL
client");
} catch (final Exception e) {
throw new SQLException("Failed to clean up client resources.", e);
}
}
+ /**
+ * Handles FlightRuntimeException during close operations, suppressing
benign gRPC shutdown errors
+ * while re-throwing genuine failures.
+ *
+ * @param fre the FlightRuntimeException to handle
+ * @param sqlErrorMessage the SQLException message to use for genuine
failures
+ * @param operationDescription description of the operation for logging
+ * @throws SQLException if the exception represents a genuine failure
+ */
+ private void handleBenignCloseException(
+ FlightRuntimeException fre, String sqlErrorMessage, String
operationDescription)
+ throws SQLException {
+ if (isBenignCloseException(fre)) {
+ logSuppressedCloseException(fre, operationDescription);
+ } else {
+ throw new SQLException(sqlErrorMessage, fre);
+ }
+ }
+
+ /**
+ * Handles FlightRuntimeException during close operations, suppressing
benign gRPC shutdown errors
+ * while re-throwing genuine failures as FlightRuntimeException.
+ *
+ * @param fre the FlightRuntimeException to handle
+ * @param operationDescription description of the operation for logging
+ * @throws FlightRuntimeException if the exception represents a genuine
failure
+ */
+ private void handleBenignCloseException(FlightRuntimeException fre, String
operationDescription)
+ throws FlightRuntimeException {
+ if (isBenignCloseException(fre)) {
+ logSuppressedCloseException(fre, operationDescription);
+ } else {
+ throw fre;
+ }
+ }
+
+ /**
+ * Determines if a FlightRuntimeException represents a benign close
operation error that should be
+ * suppressed.
+ *
+ * @param fre the FlightRuntimeException to check
+ * @return true if the exception should be suppressed, false otherwise
+ */
+ private boolean isBenignCloseException(FlightRuntimeException fre) {
+ return fre.status().code().equals(FlightStatusCode.UNAVAILABLE)
+ || (fre.status().code().equals(FlightStatusCode.INTERNAL)
+ && fre.getMessage() != null
+ && fre.getMessage().contains("Connection closed after GOAWAY"));
+ }
+
+ /**
+ * Logs a suppressed close exception with appropriate level based on debug
settings.
+ *
+ * @param fre the FlightRuntimeException being suppressed
+ * @param operationDescription description of the operation for logging
+ */
+ private void logSuppressedCloseException(
+ FlightRuntimeException fre, String operationDescription) {
+ // ARROW-17785 and GH-863: suppress exceptions caused by flaky gRPC layer
during shutdown
+ LOGGER.debug("Suppressed error {}", operationDescription, fre);
+ }
+
/** A prepared statement handler. */
public interface PreparedStatement extends AutoCloseable {
/**
@@ -386,14 +456,7 @@ public final class ArrowFlightSqlClientHandler implements
AutoCloseable {
try {
preparedStatement.close(getOptions());
} catch (FlightRuntimeException fre) {
- // ARROW-17785: suppress exceptions caused by flaky gRPC layer
- if (fre.status().code().equals(FlightStatusCode.UNAVAILABLE)
- || (fre.status().code().equals(FlightStatusCode.INTERNAL)
- && fre.getMessage().contains("Connection closed after
GOAWAY"))) {
- LOGGER.warn("Supressed error closing PreparedStatement", fre);
- return;
- }
- throw fre;
+ handleBenignCloseException(fre, "closing PreparedStatement");
}
}
};
diff --git
a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandlerTest.java
b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandlerTest.java
new file mode 100644
index 000000000..d5973ab5d
--- /dev/null
+++
b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandlerTest.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.driver.jdbc.client;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Optional;
+import org.apache.arrow.flight.CallOption;
+import org.apache.arrow.flight.CallStatus;
+import org.apache.arrow.flight.CloseSessionRequest;
+import org.apache.arrow.flight.FlightStatusCode;
+import org.apache.arrow.flight.sql.FlightSqlClient;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class ArrowFlightSqlClientHandlerTest {
+
+ @ParameterizedTest
+ @MethodSource
+ public void testCloseHandlesFlightRuntimeException(
+ boolean throwFromCloseSession, CallStatus callStatus, boolean
shouldSuppress)
+ throws Exception {
+ FlightSqlClient sqlClient = mock(FlightSqlClient.class);
+ String cacheKey = "cacheKey";
+ Optional<String> catalog =
+ throwFromCloseSession ? Optional.of("test_catalog") : Optional.empty();
+ final Collection<CallOption> credentialOptions = new ArrayList<>();
+ ArrowFlightSqlClientHandler.Builder builder = new
ArrowFlightSqlClientHandler.Builder();
+
+ if (throwFromCloseSession) {
+ doThrow(callStatus.toRuntimeException())
+ .when(sqlClient)
+ .closeSession(any(CloseSessionRequest.class),
any(CallOption[].class));
+ } else {
+ doThrow(callStatus.toRuntimeException()).when(sqlClient).close();
+ }
+
+ ArrowFlightSqlClientHandler sqlClientHandler =
+ new ArrowFlightSqlClientHandler(
+ cacheKey, sqlClient, builder, credentialOptions, catalog, null);
+
+ if (shouldSuppress) {
+ assertDoesNotThrow(sqlClientHandler::close);
+ } else {
+ assertThrows(SQLException.class, sqlClientHandler::close);
+ }
+ }
+
+ private static Object[] testCloseHandlesFlightRuntimeException() {
+ CallStatus benignInternalError =
+ new CallStatus(FlightStatusCode.INTERNAL, null, "Connection closed
after GOAWAY", null);
+ CallStatus notBenignInternalError =
+ new CallStatus(FlightStatusCode.INTERNAL, null, "Not a benign internal
error", null);
+ CallStatus unavailableError = new CallStatus(FlightStatusCode.UNAVAILABLE,
null, null, null);
+ CallStatus unknownError = new CallStatus(FlightStatusCode.UNKNOWN, null,
null, null);
+ return new Object[] {
+ new Object[] {true, benignInternalError, true},
+ new Object[] {false, benignInternalError, true},
+ new Object[] {true, notBenignInternalError, false},
+ new Object[] {false, notBenignInternalError, false},
+ new Object[] {true, unavailableError, true},
+ new Object[] {false, unavailableError, true},
+ new Object[] {true, unknownError, false},
+ new Object[] {false, unknownError, false},
+ };
+ }
+}
