ARROW-4871: [Java/Flight] Handle large Flight messages#3898
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3898 +/- ##
==========================================
+ Coverage 87.85% 87.85% +<.01%
==========================================
Files 726 726
Lines 89340 89337 -3
Branches 1252 1252
==========================================
Hits 78489 78489
- Misses 10733 10734 +1
+ Partials 118 114 -4
Continue to review full report at Codecov.
|
|
IIUC we aren't running the Flight integration tests in Travis CI, do you want to try to enable that? https://github.com/apache/arrow/blob/master/ci/travis_script_integration.sh |
|
I'll give it a spin. We need a way to mark integration tests as XFAIL then as Flight can't handle dictionaries yet. |
| public FlightClient(BufferAllocator incomingAllocator, Location location) { | ||
| final ManagedChannelBuilder<?> channelBuilder = ManagedChannelBuilder.forAddress(location.getHost(), | ||
| location.getPort()).maxTraceEvents(0).usePlaintext(); | ||
| location.getPort()).maxTraceEvents(0).maxInboundMessageSize(Integer.MAX_VALUE).usePlaintext(); |
There was a problem hiding this comment.
move both the constants to a common place, would be better i guess..
also do we it would be useful for the user to be able to set a custom value (through a system property?)
There was a problem hiding this comment.
I'll move the constants.
It might be useful, but so far we've wanted not to leak gRPC details to the user. And if we were to expose them somehow, I'd rather we provide a way to directly build a client on top of a gRPC Channel instead of having to wrap every property that might need changing.
|
@lihalite let's do the integration testing in a different patch? Don't want to hold this up |
|
@wesm sure, no problem. I got pulled into some other stuff today, if you want to move the constants, I'll try to get a patch out for that tonight or tomorrow. |
|
I've moved the constants. I'll follow up with the integration test changes later. |
praveenbingo
left a comment
praveenbingo
left a comment
There was a problem hiding this comment.
Test failure seems unrelated. LGTM.
Equivalent to https://issues.apache.org/jira/browse/ARROW-4421, but for Java. Author: David Li <David.M.Li@twosigma.com> Closes apache#3898 from lihalite/java-large-flight and squashes the following commits: a74e5c8 <David Li> Move Flight gRPC config values to constants 209b0a1 <David Li> Accept large messages in Java Flight client/server 2ce372b <David Li> Spawn new Flight server for each integration test 90f84b9 <David Li> Test large batch sizes in Flight integration
3 participants
Equivalent to https://issues.apache.org/jira/browse/ARROW-4421, but for Java.