Skip to content
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

Client connection silent reset when moving to a different network #6

Open
regexb opened this issue Nov 2, 2016 · 4 comments
Open
Labels
Projects

Comments

@regexb
Copy link
Member

regexb commented Nov 2, 2016

When switching to a different network, the client thinks it's still connected, but the servers session becomes invalid so hooks don't get sent to the client.

Maybe introduce a simple ping message from the client so it can tell when the connection is bad and needs to be reset.

@regexb regexb added the bug label Nov 2, 2016
@regexb
Copy link
Member Author

regexb commented Nov 3, 2016

Did a little testing with this. It looks like we need to add a ping to the client.

  1. Do we want this to be a seperate RPC call? Ping(pingMessage) pongMessage
    This allows us to only ping from the client since the server can't invoke a call to the client

  2. Do we want to add a client stream to the Tunnel RPC call so the client can send a request and get a response over the same stream that the server uses to send hooks?

My gut is saying go with 2 since this would allow us to ping the client from the server and clean up dead connections before trying to send a hook.

@regexb
Copy link
Member Author

regexb commented Nov 3, 2016

After talking to some GRPC people from the gopher grpc slack channel, Introducing a ping doesn't actual gain us anything. The GRPC connection has a backoff strategy and will keep trying to connect to the server if the connection is lost. We can just tailer this strategy to match our needs and everything should work.

The last bug is handling the error on the server when the connection has been lost, the server tries to Send a message, and it fails with 2016/11/02 20:41:01 transport: http2Server.HandleStreams failed to read frame: read tcp 10.84.0.9:9001->10.84.0.1:61507: read: connection reset by peer

@regexb
Copy link
Member Author

regexb commented Nov 7, 2016

Still no luck figuring out what is causing that http2Server frame read failure. I'm going to leave this ticket open as a reminder to try out some things to fix it:

  1. Remove backoff reconnect strategy from grpc client and instead use FailFast dial option. Then apply a custom backoff retry strategy on the full connection. (This will only work if the FailFast option disconnects while a streaming response is being received as well)
  2. Add a streaming request so the client can send a ping message every so often to make sure the connection is alive. (We should be able to use this if the FailFast doesn't disconnect from a streaming response alone)

@regexb
Copy link
Member Author

regexb commented Nov 8, 2016

Latest Update:

Looks like the FailFast only applies to the underlying tcp connection, not the individual RPC calls. After experimenting a bit it doesn't look like this is going to help us at all. I've been toying with a few options:

  1. Forget streaming all together and move to an RPC long polling solution. This isn't the most ideal, but it is probably the least hacky of any solution. The interval could probably be pretty low since the connection would still be open. Would have to experiment to see if leaving the tcp connection open would result in the same issue. I'm guessing we would still need to have our own re-connect to handle failures, but maybe not
  2. Add a client streaming request and introduce a ping message. The goal of the ping would be to identify on the client when the server is no longer in reach. This would allow us to try and re-establish the connection ourselves with a backoff strategy knowing that the client may just be out of reach of an internet connection but that it would come back on line eventually.

Will get back to this thread after some experimenting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Gohook
Known Issues
Development

No branches or pull requests

1 participant