ennuite commented on PR #1027: URL: https://github.com/apache/arrow-java/pull/1027#issuecomment-5444065231
I tracked this down all the way to grpc-java and IMO the root cause is there, my change in this PR surfaced the problem. The situation looks a bit involved. I can create some PRs in Arrow Java to gain some robustness over these lower level errors in time for this release, or we can leave this PR for a later release. I have no strong opinion either way: - Port validation in the JDBC driver in the path of [DriverManager.getConnection()](https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/java/sql/DriverManager.html#getConnection(java.lang.String)): we could validate that the port is numeric and in the range [1, 65535]. This would fix the problem in the test and allow this PR to be merged. - Liveness hardening on JDBC connection establishment: could honor the limits of https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/java/sql/DriverManager.html#setLoginTimeout(int) and properly implement https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/javax/sql/DataSource.html#setLoginTimeout(int) and https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/javax/sql/DataSource.html#getLoginTimeout(). This would not fix the bug in this test, but would allow users of the driver to have a way to prevent being blocked in the future by bugs that cause connection establishment to hang indefinitely. Details about the root cause (no need to read this unless you're interested in the gRPC internals): 1- I changed the NettyChannelBuilder call from [forAddress(SocketAddress)](https://grpc.github.io/grpc-java/javadoc/io/grpc/netty/NettyChannelBuilder.html#forAddress(java.net.SocketAddress)) to [forAddress(String, int)](https://grpc.github.io/grpc-java/javadoc/io/grpc/netty/NettyChannelBuilder.html#forAddress(java.lang.String,int)). This allows us to respect JVM proxy settings and is the overload gRPC recommends for normal TCP channels. 2-An unintended side effect is that port validation moved from synchronous `SocketAddress` construction to asynchronous DNS resolution in a separate thread owned by gRPC. 3-If that asynchronous validation fails because of an invalid port, gRPC does not propagate the exception through the resolver listener. This causes testUnencryptedConnectionProvidingInvalidPort to hang indefinitely and CI to time out. I will take this up with the gRPC folks and see if they agree that we need to propagate the exception there. In any case that will be too slow for this release. -- 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]
