Fix/gorpc waitgroup panic #5
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gorpc server use clientID (which by default client IP), to dispatch client messages. In case of Tyk it was causing issues, since when hybrid traffic goes though the proxy, behind MDCB, original IP gets lost, and all clients looks the same. It was causing issues when clients did not received messages in some cases. We solved it by introducing additional protocol handshake, on which Gateway sends its UUID, which will be used as clientID instead of IP. We had to implement handshake functionality inside Accept() function, because of Gorpc design. Accept() in networking applications should be non blocking, but in our case we did a few connection reads inside it, which in most cases was fine (since they are fast), but in some cases it was blocking for a long period of time, because of network timeout and etc (for example in case of TCP health-checks). This change moves clientID logic out of Accept() function to OnConnect() hook, and MDCB does its handshake logic inside it. Additionally, all connection variables set to type net.Conn.
It was not using UUID, and always falling back to use IP
Requires calling `gob.Register` manually
Updated some of the code of server.go with the latest available in [https://github.com/valyala/gorpc](https://github.com/valyala/gorpc) Some of the changes are: - Suppress more 'EOF' log messages when clien disconnects from the server or Server.Stop() has been called - Do not log 'closed conn'-like error messages on server side after Server.Stop() - Increase the probability of message batching on both client and server by calling `runtime.Gosched() `after requests/responses channels have been drained. This should reduce the number of write() syscalls (especially on go1.5)
…-waitgroup-for-connections-established added waitgroup to know when the connections are stablished
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See explanation here https://chatgpt.com/share/670fa163-0404-8005-ae68-9d7fdb8fda38
But basically if you have more then 1 failure in that loop you can get issue like that.