Skip to content

Commit dfdd0ce

Browse files
lidavidmlidavidm
authored
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 <[email protected]> Signed-off-by: David Li <[email protected]>
1 parent bed6f8e commit dfdd0ce

10 files changed

Lines changed: 36 additions & 74 deletions

File tree

‎java/flight/flight-sql-jdbc-driver/pom.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,6 @@
9696
<version>1.3</version>
9797
<scope>test</scope>
9898
</dependency>
99-
<dependency>
100-
<groupId>me.alexpanov</groupId>
101-
<artifactId>free-port-finder</artifactId>
102-
<version>1.1.1</version>
103-
<scope>test</scope>
104-
</dependency>
10599

106100
<dependency>
107101
<groupId>commons-io</groupId>

‎java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.arrow.flight.FlightEndpoint;
3535
import org.apache.arrow.flight.FlightInfo;
3636
import org.apache.arrow.flight.FlightRuntimeException;
37+
import org.apache.arrow.flight.FlightStatusCode;
3738
import org.apache.arrow.flight.FlightStream;
3839
import org.apache.arrow.flight.Location;
3940
import org.apache.arrow.flight.auth2.BearerCredentialWriter;
@@ -49,12 +50,14 @@
4950
import org.apache.arrow.util.Preconditions;
5051
import org.apache.arrow.vector.types.pojo.Schema;
5152
import org.apache.calcite.avatica.Meta.StatementType;
53+
import org.slf4j.Logger;
54+
import org.slf4j.LoggerFactory;
5255

5356
/**
5457
* A {@link FlightSqlClient} handler.
5558
*/
5659
public final class ArrowFlightSqlClientHandler implements AutoCloseable {
57-
60+
private static final Logger LOGGER = LoggerFactory.getLogger(ArrowFlightSqlClientHandler.class);
5861
private final FlightSqlClient sqlClient;
5962
private final Set<CallOption> options = new HashSet<>();
6063

@@ -189,7 +192,18 @@ public Schema getDataSetSchema() {
189192

190193
@Override
191194
public void close() {
192-
preparedStatement.close(getOptions());
195+
try {
196+
preparedStatement.close(getOptions());
197+
} catch (FlightRuntimeException fre) {
198+
// ARROW-17785: suppress exceptions caused by flaky gRPC layer
199+
if (fre.status().code().equals(FlightStatusCode.UNAVAILABLE) ||
200+
(fre.status().code().equals(FlightStatusCode.INTERNAL) &&
201+
fre.getMessage().contains("Connection closed after GOAWAY"))) {
202+
LOGGER.warn("Supressed error closing PreparedStatement", fre);
203+
return;
204+
}
205+
throw fre;
206+
}
193207
}
194208
};
195209
}

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcConnectionPoolDataSourceTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ public class ArrowFlightJdbcConnectionPoolDataSourceTest {
4444
.build();
4545

4646
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
47-
.host("localhost")
48-
.randomPort()
4947
.authentication(authentication)
5048
.producer(PRODUCER)
5149
.build();

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ public class ArrowFlightJdbcDriverTest {
5454
new UserPasswordAuthentication.Builder().user("user1", "pass1").user("user2", "pass2")
5555
.build();
5656

57-
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort()
58-
.authentication(authentication).producer(PRODUCER).build();
57+
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
58+
.authentication(authentication)
59+
.producer(PRODUCER)
60+
.build();
5961
}
6062

6163
private BufferAllocator allocator;

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactoryTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ public class ArrowFlightJdbcFactoryTest {
4949
new UserPasswordAuthentication.Builder().user("user1", "pass1").user("user2", "pass2")
5050
.build();
5151

52-
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort()
53-
.authentication(authentication).producer(PRODUCER).build();
52+
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
53+
.authentication(authentication)
54+
.producer(PRODUCER)
55+
.build();
5456
}
5557

5658
private BufferAllocator allocator;

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@ public class ConnectionTest {
5656
.user(userTest, passTest)
5757
.build();
5858

59-
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder().host("localhost").randomPort()
60-
.authentication(authentication).producer(PRODUCER).build();
59+
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
60+
.authentication(authentication)
61+
.producer(PRODUCER)
62+
.build();
6163
}
6264

6365
private BufferAllocator allocator;

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ public class ConnectionTlsTest {
6363
.build();
6464

6565
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
66-
.host("localhost")
67-
.randomPort()
6866
.authentication(authentication)
6967
.useEncryption(certKey.cert, certKey.key)
7068
.producer(PRODUCER)

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestRule.java‎

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@
5050
import org.slf4j.Logger;
5151
import org.slf4j.LoggerFactory;
5252

53-
import me.alexpanov.net.FreePortFinder;
54-
5553
/**
5654
* Utility class for unit tests that need to instantiate a {@link FlightServer}
5755
* and interact with it.
@@ -95,8 +93,6 @@ public static FlightServerTestRule createStandardTestRule(final FlightSqlProduce
9593
.build();
9694

9795
return new Builder()
98-
.host("localhost")
99-
.randomPort()
10096
.authentication(authentication)
10197
.producer(producer)
10298
.build();
@@ -106,11 +102,6 @@ ArrowFlightJdbcDataSource createDataSource() {
106102
return ArrowFlightJdbcDataSource.createNewDataSource(properties);
107103
}
108104

109-
ArrowFlightJdbcDataSource createDataSource(String token) {
110-
properties.put("token", token);
111-
return ArrowFlightJdbcDataSource.createNewDataSource(properties);
112-
}
113-
114105
public ArrowFlightJdbcConnectionPoolDataSource createConnectionPoolDataSource() {
115106
return ArrowFlightJdbcConnectionPoolDataSource.createNewDataSource(properties);
116107
}
@@ -159,9 +150,8 @@ public Statement apply(Statement base, Description description) {
159150
return new Statement() {
160151
@Override
161152
public void evaluate() throws Throwable {
162-
try (FlightServer flightServer =
163-
getStartServer(location ->
164-
initiateServer(location), 3)) {
153+
try (FlightServer flightServer = getStartServer(location -> initiateServer(location), 3)) {
154+
properties.put("port", flightServer.getPort());
165155
LOGGER.info("Started " + FlightServer.class.getName() + " as " + flightServer);
166156
base.evaluate();
167157
} finally {
@@ -174,12 +164,9 @@ public void evaluate() throws Throwable {
174164
private FlightServer getStartServer(CheckedFunction<Location, FlightServer> newServerFromLocation,
175165
int retries)
176166
throws IOException {
177-
178167
final Deque<ReflectiveOperationException> exceptions = new ArrayDeque<>();
179-
180168
for (; retries > 0; retries--) {
181-
final Location location = Location.forGrpcInsecure(config.getHost(), config.getPort());
182-
final FlightServer server = newServerFromLocation.apply(location);
169+
final FlightServer server = newServerFromLocation.apply(Location.forGrpcInsecure("localhost", 0));
183170
try {
184171
Method start = server.getClass().getMethod("start");
185172
start.setAccessible(true);
@@ -189,9 +176,7 @@ private FlightServer getStartServer(CheckedFunction<Location, FlightServer> newS
189176
exceptions.add(e);
190177
}
191178
}
192-
193-
exceptions.forEach(
194-
e -> LOGGER.error("Failed to start a new " + FlightServer.class.getName() + ".", e));
179+
exceptions.forEach(e -> LOGGER.error("Failed to start FlightServer", e));
195180
throw new IOException(exceptions.pop().getCause());
196181
}
197182

@@ -223,44 +208,14 @@ public void close() throws Exception {
223208
* Builder for {@link FlightServerTestRule}.
224209
*/
225210
public static final class Builder {
226-
private final Properties properties = new Properties();
211+
private final Properties properties;
227212
private FlightSqlProducer producer;
228213
private Authentication authentication;
229214
private CertKeyPair certKeyPair;
230215

231-
/**
232-
* Sets the host for the server rule.
233-
*
234-
* @param host the host value.
235-
* @return the Builder.
236-
*/
237-
public Builder host(final String host) {
238-
properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.HOST.camelName(),
239-
host);
240-
return this;
241-
}
242-
243-
/**
244-
* Sets a random port to be used by the server rule.
245-
*
246-
* @return the Builder.
247-
*/
248-
public Builder randomPort() {
249-
properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.PORT.camelName(),
250-
FreePortFinder.findFreeLocalPort());
251-
return this;
252-
}
253-
254-
/**
255-
* Sets a specific port to be used by the server rule.
256-
*
257-
* @param port the port value.
258-
* @return the Builder.
259-
*/
260-
public Builder port(final int port) {
261-
properties.put(ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty.PORT.camelName(),
262-
port);
263-
return this;
216+
public Builder() {
217+
this.properties = new Properties();
218+
this.properties.put("host", "localhost");
264219
}
265220

266221
/**

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ public void testShouldRunSelectQuerySettingLargeMaxRowLimit() throws Exception {
160160
try (Statement statement = connection.createStatement();
161161
ResultSet resultSet = statement.executeQuery(
162162
CoreMockedSqlProducers.LEGACY_REGULAR_SQL_CMD)) {
163-
164163
final long maxRowsLimit = 3;
165164
statement.setLargeMaxRows(maxRowsLimit);
166165

‎java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/TokenAuthenticationTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ public class TokenAuthenticationTest {
3636

3737
static {
3838
FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
39-
.host("localhost")
40-
.randomPort()
4139
.authentication(new TokenAuthentication.Builder()
4240
.token("1234")
4341
.build())

0 commit comments

Comments
 (0)