From dfdd0ceb2249e2f11ec6b88cf277e3b3e20f952d Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 23 Sep 2022 09:51:21 -0400 Subject: [PATCH] ARROW-17785: [Java] Suppress flakiness from gRPC in JDBC driver tests (#14210) I couldn't reproduce it, so I added a suppression instead. In both cases, the error is that the server is uncontactable. That shouldn't happen, but I changed the tests to also bind to port 0 instead of using a potentially flaky free port finder. Authored-by: David Li Signed-off-by: David Li --- java/flight/flight-sql-jdbc-driver/pom.xml | 6 -- .../client/ArrowFlightSqlClientHandler.java | 18 +++++- ...lightJdbcConnectionPoolDataSourceTest.java | 2 - .../jdbc/ArrowFlightJdbcDriverTest.java | 6 +- .../jdbc/ArrowFlightJdbcFactoryTest.java | 6 +- .../arrow/driver/jdbc/ConnectionTest.java | 6 +- .../arrow/driver/jdbc/ConnectionTlsTest.java | 2 - .../driver/jdbc/FlightServerTestRule.java | 61 +++---------------- .../arrow/driver/jdbc/ResultSetTest.java | 1 - .../driver/jdbc/TokenAuthenticationTest.java | 2 - 10 files changed, 36 insertions(+), 74 deletions(-) diff --git a/java/flight/flight-sql-jdbc-driver/pom.xml b/java/flight/flight-sql-jdbc-driver/pom.xml index 646a888a9653a..f374cd5b36195 100644 --- a/java/flight/flight-sql-jdbc-driver/pom.xml +++ b/java/flight/flight-sql-jdbc-driver/pom.xml @@ -96,12 +96,6 @@ 1.3 test - - me.alexpanov - free-port-finder - 1.1.1 - test - commons-io diff --git a/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java b/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java index afac6c164708b..7b059ab02f851 100644 --- a/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java +++ b/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java @@ -34,6 +34,7 @@ import org.apache.arrow.flight.FlightEndpoint; import org.apache.arrow.flight.FlightInfo; import org.apache.arrow.flight.FlightRuntimeException; +import org.apache.arrow.flight.FlightStatusCode; import org.apache.arrow.flight.FlightStream; import org.apache.arrow.flight.Location; import org.apache.arrow.flight.auth2.BearerCredentialWriter; @@ -49,12 +50,14 @@ import org.apache.arrow.util.Preconditions; import org.apache.arrow.vector.types.pojo.Schema; import org.apache.calcite.avatica.Meta.StatementType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A {@link FlightSqlClient} handler. */ public final class ArrowFlightSqlClientHandler implements AutoCloseable { - + private static final Logger LOGGER = LoggerFactory.getLogger(ArrowFlightSqlClientHandler.class); private final FlightSqlClient sqlClient; private final Set options = new HashSet<>(); @@ -189,7 +192,18 @@ public Schema getDataSetSchema() { @Override public void close() { - preparedStatement.close(getOptions()); + 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; + } } }; } diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcConnectionPoolDataSourceTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcConnectionPoolDataSourceTest.java index f4a5c87a23cc2..bdf2826c41e97 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcConnectionPoolDataSourceTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcConnectionPoolDataSourceTest.java @@ -44,8 +44,6 @@ public class ArrowFlightJdbcConnectionPoolDataSourceTest { .build(); FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() - .host("localhost") - .randomPort() .authentication(authentication) .producer(PRODUCER) .build(); diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java index 682c20c696ac3..f1958b4fc8a9f 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java @@ -54,8 +54,10 @@ public class ArrowFlightJdbcDriverTest { new UserPasswordAuthentication.Builder().user("user1", "pass1").user("user2", "pass2") .build(); - FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort() - .authentication(authentication).producer(PRODUCER).build(); + FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() + .authentication(authentication) + .producer(PRODUCER) + .build(); } private BufferAllocator allocator; diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactoryTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactoryTest.java index c482169852e5e..a355e7156f7db 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactoryTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactoryTest.java @@ -49,8 +49,10 @@ public class ArrowFlightJdbcFactoryTest { new UserPasswordAuthentication.Builder().user("user1", "pass1").user("user2", "pass2") .build(); - FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort() - .authentication(authentication).producer(PRODUCER).build(); + FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() + .authentication(authentication) + .producer(PRODUCER) + .build(); } private BufferAllocator allocator; diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java index 6fe7ba7129829..2472ab8fc581a 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java @@ -56,8 +56,10 @@ public class ConnectionTest { .user(userTest, passTest) .build(); - FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort() - .authentication(authentication).producer(PRODUCER).build(); + FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() + .authentication(authentication) + .producer(PRODUCER) + .build(); } private BufferAllocator allocator; diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java index a5f9938f04bcb..95d591766a836 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java @@ -63,8 +63,6 @@ public class ConnectionTlsTest { .build(); FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() - .host("localhost") - .randomPort() .authentication(authentication) .useEncryption(certKey.cert, certKey.key) .producer(PRODUCER) diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestRule.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestRule.java index b251b7df1645b..733145892ec3e 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestRule.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestRule.java @@ -50,8 +50,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import me.alexpanov.net.FreePortFinder; - /** * Utility class for unit tests that need to instantiate a {@link FlightServer} * and interact with it. @@ -95,8 +93,6 @@ public static FlightServerTestRule createStandardTestRule(final FlightSqlProduce .build(); return new Builder() - .host("localhost") - .randomPort() .authentication(authentication) .producer(producer) .build(); @@ -106,11 +102,6 @@ ArrowFlightJdbcDataSource createDataSource() { return ArrowFlightJdbcDataSource.createNewDataSource(properties); } - ArrowFlightJdbcDataSource createDataSource(String token) { - properties.put("token", token); - return ArrowFlightJdbcDataSource.createNewDataSource(properties); - } - public ArrowFlightJdbcConnectionPoolDataSource createConnectionPoolDataSource() { return ArrowFlightJdbcConnectionPoolDataSource.createNewDataSource(properties); } @@ -159,9 +150,8 @@ public Statement apply(Statement base, Description description) { return new Statement() { @Override public void evaluate() throws Throwable { - try (FlightServer flightServer = - getStartServer(location -> - initiateServer(location), 3)) { + try (FlightServer flightServer = getStartServer(location -> initiateServer(location), 3)) { + properties.put("port", flightServer.getPort()); LOGGER.info("Started " + FlightServer.class.getName() + " as " + flightServer); base.evaluate(); } finally { @@ -174,12 +164,9 @@ public void evaluate() throws Throwable { private FlightServer getStartServer(CheckedFunction newServerFromLocation, int retries) throws IOException { - final Deque exceptions = new ArrayDeque<>(); - for (; retries > 0; retries--) { - final Location location = Location.forGrpcInsecure(config.getHost(), config.getPort()); - final FlightServer server = newServerFromLocation.apply(location); + final FlightServer server = newServerFromLocation.apply(Location.forGrpcInsecure("localhost", 0)); try { Method start = server.getClass().getMethod("start"); start.setAccessible(true); @@ -189,9 +176,7 @@ private FlightServer getStartServer(CheckedFunction newS exceptions.add(e); } } - - exceptions.forEach( - e -> LOGGER.error("Failed to start a new " + FlightServer.class.getName() + ".", e)); + exceptions.forEach(e -> LOGGER.error("Failed to start FlightServer", e)); throw new IOException(exceptions.pop().getCause()); } @@ -223,44 +208,14 @@ public void close() throws Exception { * Builder for {@link FlightServerTestRule}. */ public static final class Builder { - private final Properties properties = new Properties(); + private final Properties properties; private FlightSqlProducer producer; private Authentication authentication; private CertKeyPair certKeyPair; - /** - * Sets the host for the server rule. - * - * @param host the host value. - * @return the Builder. - */ - public Builder host(final String host) { - properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.HOST.camelName(), - host); - return this; - } - - /** - * Sets a random port to be used by the server rule. - * - * @return the Builder. - */ - public Builder randomPort() { - properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.PORT.camelName(), - FreePortFinder.findFreeLocalPort()); - return this; - } - - /** - * Sets a specific port to be used by the server rule. - * - * @param port the port value. - * @return the Builder. - */ - public Builder port(final int port) { - properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.PORT.camelName(), - port); - return this; + public Builder() { + this.properties = new Properties(); + this.properties.put("host", "localhost"); } /** diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java index 33473b6fe2baa..b3002ec58416e 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java @@ -160,7 +160,6 @@ public void testShouldRunSelectQuerySettingLargeMaxRowLimit() throws Exception { try (Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery( CoreMockedSqlProducers.LEGACY_REGULAR_SQL_CMD)) { - final long maxRowsLimit = 3; statement.setLargeMaxRows(maxRowsLimit); diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/TokenAuthenticationTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/TokenAuthenticationTest.java index 56c8c178f2133..9fe506231ec6a 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/TokenAuthenticationTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/TokenAuthenticationTest.java @@ -36,8 +36,6 @@ public class TokenAuthenticationTest { static { FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder() - .host("localhost") - .randomPort() .authentication(new TokenAuthentication.Builder() .token("1234") .build())