ARROW-10441: [Java] Prevent closure of shared channels for FlightClient #8581
Conversation
channels when specified for FlightClient.
kylepbit
changed the title
|
@lidavidm here's a PR for the shared channel issue: https://issues.apache.org/jira/browse/ARROW-10441 |
| @@ -49,10 +143,10 @@ public static BindableService createFlightService(BufferAllocator allocator, Fli | |||
| /** | |||
| * Creates a Flight client. | |||
| * @param incomingAllocator Memory allocator | |||
| * @param channel provides a connection to a gRPC server | |||
| * @param channel provides a connection to a gRPC server. Will not be closed on closure of the returned FlightClient. | |||
There was a problem hiding this comment.
This would be a breaking change - can we do this in a separate method?
There was a problem hiding this comment.
To clarify, introduce a new method for creating flight clients which uses the proxy? To me that persists the issue that this was intended to solve, which is that closing a client closes the channel that was passed to it when that channel may be used to create multiple clients. Otherwise, I would suggest this simply be doc-fixed by noting that the client takes ownership and will close the channel upon closure.
There was a problem hiding this comment.
Yes - how does that not solve the problem? The caller can choose whether the FlightClient takes ownership of the channel or not by calling different methods. But if the current method is modified, then any existing code will suddenly silently leak channels.
java/flight/flight-grpc/src/main/java/org/apache/arrow/flight/FlightGrpcUtils.java
Outdated
Show resolved
Hide resolved
Update naming of proxy channel to better reflect behaviour.
|
@lidavidm I believe the JNI issue is not related and is a common problem - how can we get this merged? |
|
No problem, I was just waiting for other builds to pass. Thanks! |
Add a proxy for ManagedChannel to prevent closure of shared channels when specified for FlightClient in FlightGrpcUtils. Closes apache#8581 from kylepbit/ARROW-10441 Authored-by: Kyle Porter <kporter@dremio.com> Signed-off-by: David Li <li.davidm96@gmail.com>
Add a proxy for ManagedChannel to prevent closure of shared
channels when specified for FlightClient in FlightGrpcUtils.