[GitHub] [accumulo] cshannon commented on a diff in pull request #3134: Make sure maxMessageSize is correctly set when Thrift is configured to use a non blocking server

[GitBox <gi...@apache.org>]

cshannon commented on code in PR #3134:
URL: https://github.com/apache/accumulo/pull/3134#discussion_r1051612111


##########
server/base/src/main/java/org/apache/accumulo/server/rpc/TServerUtils.java:
##########
@@ -221,7 +223,8 @@ private static ServerAddress createThreadedSelectorServer(HostAndPort address,
       long timeBetweenThreadChecks, long maxMessageSize) throws TTransportException {
 
     final TNonblockingServerSocket transport =
-        new TNonblockingServerSocket(new InetSocketAddress(address.getHost(), address.getPort()));
+        new TNonblockingServerSocket(new InetSocketAddress(address.getHost(), address.getPort()), 0,
+            Ints.saturatedCast(maxMessageSize));

Review Comment:
   Yeah I've been using that for a couple years, quite handy to avoid possible buffer overflows.
   
   I'm setting it to 0 because that is the default value and what we were doing before with the previous constructor as shown here https://github.com/apache/thrift/blob/v0.17.0/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingServerSocket.java#L75
   
   The value eventually makes it's way to being set as the soTimeout() value on the socket which mean it's an infinite timeout, so basically no timeout. https://github.com/apache/thrift/blob/v0.17.0/lib/java/src/main/java/org/apache/thrift/transport/TNonblockingSocket.java#L114
   
   We could make this configurable but for now i was just keeping the existing behavior as that would be a new config option and should be a new Issue/PR i would think.



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org