DRILL-3317: when ProtobufLengthDecoder couldn't allocate a new DrillB… #446
Conversation
…uf, this error is just logged and nothing else is done
| outOfMemoryHandler.handle(); | ||
| return; | ||
| } | ||
| ByteBuf outBuf = allocator.buffer(length); |
There was a problem hiding this comment.
Can you confirm that this has a consistent failure behavior by adding a test and create a very small rpc allocator (maybe add a config option to set the max for allocator (not just reservation)).
There was a problem hiding this comment.
I will look into that. So far, when I was trying to reproduce the issue through Sqlline I would get an OutOfMemory error in a different place (earlier ?) of the RPC layer.
There was a problem hiding this comment.
So I was looking at how we are using ProtobufLengthDecoder in our code, and we use in the control, server and user tunnels both on the client and server side.
If I am not mistaken, the only derived class from ProtobufLengthDecoder that actually uses an rpc allocator is UserProtobufLengthDecoder and only on the server side of the user tunnel. All other implementations use the root allocator.
This may actually explain why I wasn't able to reproduce this issue no matter how much I reduced the "rpc:bit-data" max allocation.
am I missing something ?
There was a problem hiding this comment.
@jacques-n can you confirm it's indeed the case ? thanks
|
So I spent more time looking at ways to create a unit test for this, and I don't think it's possible: |
|
Thanks for trying to put together a test. Can you include the fix to correct the issue about the root allocator as well. I haven't had a chance to look at that but it would explain why those numbers have been as low as they have been. |
|
Done. |
|
+1 on this. Can you just confirm the behavior you're expecting in the out of memory situation? |
|
I was not able to reproduce an out of memory from the ProtobufLengthDecoder. But I whenever I got an out of memory from ByteToMessageDecoder the query would fail properly, thanks to the fix for DRILL-3714 |
|
To confirm, the expectation is the exception is caught by the rpc handler and propagated back to the sender via the coordination id, right? |
|
This is indeed what happens |
|
Great, thanks! |
|
Does this get propated as a connection level failure or an RpcRemoteException. (E.g. do we break connection once this happens?) Seems like it would be better to generate a RpcRemoteException on the sending side to ensure that the connection is maintained. |
* MD-5265: Introduction of 4th digit in pom.xml files
Switching to ${revision}
https://maven.apache.org/maven-ci-friendly.html#Multi_Module_Setup
[Apache Issue]
DRILL-6956: Maintain a single entry for Drill Version in the pom file
Note: client/pom.xml and contrib/auth-mechanism-maprsasl/pom.xml are the only additional files in the MapR distro and not present in Apache
* Reverting to using explicit version information for all POMs until DevOps does upgrade to `Maven 3.5.0`+
|
Abandoned. |
2 participants
…uf, this error is just logged and nothing else is done