grpc-proxy icon indicating copy to clipboard operation
grpc-proxy copied to clipboard

Connection leakage

Open vgough opened this issue 7 years ago • 12 comments

Using grpc-proxy with a simple test server, I see every connection from the proxy to the endpoint leaking a file descriptor.

I've narrowed it down to 2 cases in handler.handler, both involve not closing backendConn.

  1. if the client goes away unexpectedly, grpc.NewClientStream can fail and the backendConn leaks. Need to close the backendConn in that case.

  2. seems that backendConn needs to be closed at some point. Experimentally, I found that closing backendConn after serverStream.SetTrailer eliminated the leaks I was seeing.

Without those two fixes, I see connections pile up within the proxy until it runs out of file descriptors and fails subsequent requests.

vgough avatar Jun 15 '17 19:06 vgough

Did you manage to work around this issue?

qustavo avatar May 20 '18 10:05 qustavo

See linked issue (#16). I believe that connection management remains necessary. Those haven't been incorporated, so I have my own fork that contains those along with a number of other updates. You're welcome to use my repo or extract the improvements at github.com/vgough/grpc-proxy.

vgough avatar May 21 '18 15:05 vgough

Thank you so much!

qustavo avatar May 22 '18 12:05 qustavo

Hi @vgough your fork is unusable due to your repo is vendored which leads to two problems.

First: Now the method proxy.TransparentHandler(director) returns a type from your vendor dir and thus can not be passed as a grpc server configuration: image

Second: The proxy.StreamDirector interface can not be implemented for the same reason (the interface wants github.com/vgough/grpc-proxy/vendor/google.golang.org/grpc instead of google.golang.org/grpc as paramter to the function). If I add this import: "github.com/vgough/grpc-proxy/vendor/google.golang.org/grpc" I can implement the interface, but this type of imports are forbidden and can not be compiled.

tsony-tsonev avatar Dec 06 '19 13:12 tsony-tsonev

@tsony-tsonev Thanks. I've updated my repo to use Go modules rather than the dep vendoring.

vgough avatar Dec 07 '19 20:12 vgough

@vgough thank for the fast reaction. I'll definitely try it out again.

PS: super @vgough it works and no memory leaks.

tsony-tsonev avatar Dec 09 '19 07:12 tsony-tsonev

See linked issue (#16). I believe that connection management remains necessary. Those haven't been incorporated, so I have my own fork that contains those along with a number of other updates. You're welcome to use my repo or extract the improvements at github.com/vgough/grpc-proxy.

@vgough I have read your fork about memory leakage,but i have no idea when should i release those connections. (https://github.com/vgough/grpc-proxy/blob/master/proxy/examples_test.go).Can you provide me some examples about [ Release(ctx context.Context, conn *grpc.ClientConn)] ,thank you very much

gakkiyomi avatar Jul 14 '20 10:07 gakkiyomi

@gakkiyomi There is more documentation in the readme: https://github.com/vgough/grpc-proxy/blob/master/proxy/README.md#type-streamdirector

I think that normally you would be implementing a director, not calling one. The handler calls Release on the director when it is done proxying a call: https://github.com/vgough/grpc-proxy/blob/master/proxy/handler.go#L71

When your code receives a Release call, it has the opportunity to release resources used by the director.

vgough avatar Jul 14 '20 16:07 vgough

@gakkiyomi There is more documentation in the readme: https://github.com/vgough/grpc-proxy/blob/master/proxy/README.md#type-streamdirector

I think that normally you would be implementing a director, not calling one. The handler calls Release on the director when it is done proxying a call: https://github.com/vgough/grpc-proxy/blob/master/proxy/handler.go#L71

When your code receives a Release call, it has the opportunity to release resources used by the director.

@vgough I truly appreciate your timely help.

gakkiyomi avatar Jul 15 '20 07:07 gakkiyomi

Guys just want to inform you for another bug if you are using this on prod. Memory leakage is working good, in my case I'm just closing the connections in the release function of the Director. But couple of months ago I had problems with bidirectional streaming and had to fork the repo and fix it. The proxy is forwarding messages on the bidi stream successfully from the client to the proxy than to the backend service and on the other direction without problems. The bug occurs when the backend service is restarted then the client continues to be connected to the proxy and is thinking that it has connection with the backend. This can be seen if we set 1 sec keep alive for the client connection.

tsony-tsonev avatar Jul 15 '20 07:07 tsony-tsonev

@tsony-tsonev How you managed to fix this bug? Closing client connection or redialing the backend?

SergeyNarozhny avatar Apr 01 '24 19:04 SergeyNarozhny

@SergeyNarozhny i don't remember exactly how, but you can check out our fork https://github.com/taxime-hq/grpc-proxy

tsony-tsonev avatar Apr 05 '24 08:04 tsony-tsonev