Unimplement method in Okhttp transport
Hi guys,
I'm trying to use Okhttp as a transport for grpc. I'm using it with this library Link.
The problem is why OkHttpReadableBuffer.readBytes(ByteBuffer dest) function is not implemented like in Netty.
Is there a specific reason for this?
- Okhttp, this function was not implemented.
- Netty, this function was implemented properly.
Hi, @NguyenHoangSon96 Can you describe the issue you are facing because of it ?
Hi @AgraVator Thank you for jumping on this issue. I used a library called Arrow Flight. This library is using Netty as an underlying transport, but I need to use OkHttp instead. So basically, Arrow Flight will call OkHttpReadableBuffer.readBytes(ByteBuffer dest) function, but as you can see in my screenshots above, Okhttp did not implement this function, they just throw UnsupportedOperationException. The question is why the team did not implement that function as they did in Netty.
@NguyenHoangSon96 Can you provide the call stack as well to help us understand better ?
@NguyenHoangSon96 is there a real problem or just a theoretical discussion?
ReadableBuffer is an internal type, it should not be used from other projects.
Hi @AgraVator , this is the stacktrace.
at io.grpc.okhttp.OkHttpReadableBuffer.readBytes(OkHttpReadableBuffer.java:77)
at io.grpc.internal.CompositeReadableBuffer$4.read(CompositeReadableBuffer.java:133)
at io.grpc.internal.CompositeReadableBuffer$4.read(CompositeReadableBuffer.java:126)
at io.grpc.internal.CompositeReadableBuffer.execute(CompositeReadableBuffer.java:320)
at io.grpc.internal.CompositeReadableBuffer.executeNoThrow(CompositeReadableBuffer.java:336)
at io.grpc.internal.CompositeReadableBuffer.readBytes(CompositeReadableBuffer.java:156)
at org.apache.arrow.flight.grpc.GetReadableBuffer.readIntoBuffer(GetReadableBuffer.java:91)
at org.apache.arrow.flight.ArrowMessage.frame(ArrowMessage.java:323)
at org.apache.arrow.flight.ArrowMessage$ArrowMessageHolderMarshaller.parse(ArrowMessage.java:575)
at org.apache.arrow.flight.ArrowMessage$ArrowMessageHolderMarshaller.parse(ArrowMessage.java:560)
at io.grpc.MethodDescriptor.parseResponse(MethodDescriptor.java:284)
at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessagesAvailable.runInternal(ClientCallImpl.java:661)
at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessagesAvailable.runInContext(ClientCallImpl.java:648)
at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.lang.Thread.run(Thread.java:840)
Hi @panchenko This is the real problem I'm facing now. Please check the stacktrace I sent above.
@NguyenHoangSon96 As a workaround you can try setting the properties/environment variables mentioned in https://github.com/apache/arrow-java/blob/96156ccc2bf933c75c852ca7c04418a61f87defd/flight/flight-core/src/main/java/org/apache/arrow/flight/ArrowMessage.java#L70
They access internal classes (with reflection) in https://github.com/apache/arrow-java/blob/main/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java - not sure if that's an expected use-case.
Hi @panchenko
- Thank you for your suggestions, it works. But I don't think we should do this in production.
- What I don't understand is why the grpc team not implemented
readBytes(ByteBuffer dest)properly as they do with Netty. I have checked out the grpc-okhttp source code and modified it slightly; then the code works. I fix it like this:
@Override
public void readBytes(ByteBuffer dest) {
buffer.read(dest); // Just add this line
// We are not using it.
// throw new UnsupportedOperationException();
}
- Any reason that prevents the team from implementing it like above?
ReadableBuffer was introduce long ago, probably there were some plans for such a method back then. As of now, it's not used in grpc-java.
We can agree the current state is inconsistent.
Another way to make things consistent would be removing unused code.
@panchenko Thank you, I will close this issue