-
Notifications
You must be signed in to change notification settings - Fork 158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose Netty Connection Read Timeout #477
base: develop
Are you sure you want to change the base?
Conversation
SO_TIMEOUT would not affect anything here. We're using NIO (non-blocking) connections; there's nothing to time out on because the only time a socket is read from is after it signals it's ready for reading and then the read is non-blocking. See: http://stackoverflow.com/questions/22892575/so-timeout-in-non-blocking-channel-in-netty Early on I had written a handler as the Netty guys describe. I can't remember if it was when I was prototyping or if I just didn't include it for some reason . What is causing you to have "hanging sockets" ? |
We are seeing blocking sockets when the start tls call is issued, execution blocks waiting for the decoder promise, which will never returns. io.netty.util.concurrent.DefaultPromise.await(DefaultPromise.java:260) |
I'm putting a read timeout handler in place right now, to leverage the NIO sockets. |
The line you reference (RiakNode.java:730) is waiting on the SSL handler's promise (well, erm, or something? In master it's a blank line but L729 does that). Are you trying to use SSL? Or have you just modified that file enough to push things down so that it's the normal connection wait? Either way, you're in |
It's not an issue obtaining the connection, it's once it get's into start tls land. It's not related to the handshake failing, as that throws errors as expected. |
Updated to do read timeout w/async sockets. I'm debugging against it locally to see if it exposes more info about the TLS issues. |
With the added logging, I'm seeing what's below. It looks like a connection is being closed somewhere, or riak isn't responding to the start tls request. Probably worthwhile leaving the timeout in place, if only to catch issues like this. It does appear that there would need to be some sort of handler for the timeouts, but I wasn't sure how the handling would work with the current flow. Would expose itself as a failed operation, or just throw an exception? APP | 2014-10-07 06:45:51,084 [main] DEBUG com.basho.riak.client.core.RiakNode - Waiting for new connection from channel future to :8087 |
@exell-christopher Is this still an issue for you? |
FWIW, I added a TLS handshake timeout in basho/riak_api#119 in this code |
Thanks for adding that! |
@exell-christopher - we may still wish to add the code in this PR. @alexmoore ??? |
Unfortunately I don't currently have a project that is using Riak, so I can't comment on whether this is still needed, but I've found that having timeout settings exposed always seems to be useful. |
We were debugging an issue with network connectivity, and noticed that having a setting for the SO timeout would be useful, so that unresponsive sockets timeout instead of hanging forever.
Still wracking my brain as to how best to test this, so for now all I have is the validation tests that verify that get getters and setters work.