Skip to content
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,82 @@ public FlightInfo getInfo(final String query) {
@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().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: suppress exceptions caused by flaky gRPC layer during shutdown
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Suppressed error {}", operationDescription, fre);
} else {
LOGGER.info("Suppressed benign error {}: {}", operationDescription, fre.getMessage());
}
Comment on lines +334 to +339
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the switch? Just always put it at debug level.

}

/** A prepared statement handler. */
public interface PreparedStatement extends AutoCloseable {
/**
Expand Down Expand Up @@ -386,14 +453,7 @@ public void close() {
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");
}
}
};
Expand Down