cshannon commented on PR #3134: URL: https://github.com/apache/accumulo/pull/3134#issuecomment-1374206821
@dlmarion and @ctubbsii - I looked into the RPC timeout a bit more before making the change to apply it to the server socket to fix issue #3153 and I wanted to get some feedback before I did it. As I said in my last comment it's pretty trivial to update the code to set the timeout on the server socket. However, now I'm not sure if we want to do it and what the side effects might be. Right now the value seems to be primarily intended to be used as a client side timeout (defaulting to 120 seconds) and not as a server side timeout. This does in fact work and is applied correctly to thrift clients. It is configured in the [ClientContext](https://github.com/apache/accumulo/blob/af6b4978d42e7a1f9102a7ec9605be51c67bb595/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java#L226) in the constructor and set as part of the timeoutSupplier and can be accessed using a [getter](https://github.com/apache/accumulo/blob/af6b4978d42e7a1f9102a7ec9605be51c67bb595/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java#L339). I verified with some testing that the clients time out properly when setting the value. So one potential issue I see is do we actually want to re-use this same property for a server timeout when it's been used for the client timeout and in some cases you may want to make them different. A second issue is if a timeout is applied to the server side socket as well I am a little concerned of potential consequences and changing behavior on the client side if suddenly the server has a timeout value. If the server side does timeout then the client should timeout eventually on its side as well out as long as there's an actual timeout value set (default is 120s) and in that case I think that behavior would be fine I would think and would protect the server. However, I noticed there are other cases where the client sets [no timeout](https://github.com/apache/accumulo/blob/af6b4978d42e7a1f9102a7ec9605be51c67bb595/core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java#L110) such as for manager operations. For a case like this the client would potentially hang waiting forever if the server side timed out. I actually first was alerted to this behavior when doing some testing to verify the server config worked and was seeing the client side just hang. So I just wanted to get people's thoughts on it before I proceeded. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
