yookoala / gofast

gofast is a FastCGI "client" library written purely in go
BSD 3-Clause "New" or "Revised" License
224 stars 49 forks source link

Memory leak issue with massive use of disposable client #69

Open yookoala opened 3 years ago

yookoala commented 3 years ago

Potential problem here is that the idpool channel in these "disposable" clients never got proper GC and recovery.

As reported by @adri in this comment about this usage of the library:

I'm not working often in Golang, please forgive me if what I say makes no sense. I've been looking for potential memory that is not cleaned up.

  1. Maybe c.ids.Release(reqID) is not called in certain error cases The ID is allocated and later released. What if there is an error like this, will the ID be released back in the pool?

  2. Looking at one Skipper pod I see the typical saw-tooth graph of a memory leak. I'm not sure if this has anything to do with the gofast usage or something in Skipper. If you have a tip how I can find out more please let me know.

    Screen Shot 2020-09-15 at 09 14 19

    I can see goroutines increasing:

    Screen Shot 2020-09-15 at 09 07 56

    And live objects:

    Screen Shot 2020-09-15 at 09 15 02

And in this comment:

The design of gofast is to have the ClientFactory reused, not the Client.

@yookoala mentioned that the ClientFactory should be reused in the comment.

However, if I read it right, the ClientFactory is created on every request.

@yookoala @ruudk Should this not be changed in Skipper to only create the ClientFactory once? See this line in skipper.


I think the problem is with creating new request IDs. When the ClientFactory is not reused, I guess that there are new ID pools created on every request?

Explanation When getting a diff between two goroutine dumps I see many goroutines in newIDs.func1:

goroutine 75451 [chan send, 10 minutes]:
github.com/yookoala/gofast.newIDs.func1(0xc001323020, 0xffff)
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:102 +0x43
created by github.com/yookoala/gofast.newIDs
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:100 +0x78

goroutine 68511 [chan send, 11 minutes]:
github.com/yookoala/gofast.newIDs.func1(0xc002043500, 0xffff)
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:102 +0x43
created by github.com/yookoala/gofast.newIDs
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:100 +0x78

goroutine 53003 [chan send, 12 minutes]:
github.com/yookoala/gofast.newIDs.func1(0xc001f52f60, 0xffff)
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:102 +0x43
created by github.com/yookoala/gofast.newIDs
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:100 +0x78

goroutine 120002 [chan send, 6 minutes]:
github.com/yookoala/gofast.newIDs.func1(0xc001a0cba0, 0xffff)
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:102 +0x43
created by github.com/yookoala/gofast.newIDs
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:100 +0x78

goroutine 169080 [chan send, 2 minutes]:
github.com/yookoala/gofast.newIDs.func1(0xc000039bc0, 0xffff)
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:102 +0x43
created by github.com/yookoala/gofast.newIDs
  /go/pkg/mod/github.com/yookoala/gofast@v0.4.0/client.go:100 +0x78

How to dump

  1. Configure -enable-profile for Skipper
  2. Proxy to a Skipper pod kubectl port-forward skipper-ingress-pzx2b 9911:9911
  3. Go to http://127.0.0.1:9911/debug/pprof/ and download a dump full goroutine stack dump, wait a few minutes, and download another dump
  4. Install goroutine-inspect and run it $GOPATH/bin/goroutine-inspect
  5. Make a diff, (l = only in original, c = in both, r = in original 2)
    original = load("goroutine.dump")
    original2 = load("goroutine2.dump")
    l, c, r = original.diff(original2)
    c.show()
yookoala commented 3 years ago

@adri: Please try to use the new v0.6.0 release and see if #70 fixed the memory leak issue.

Note: SimpleClientFactory has been updated to remove the obsoleted limit parameter for the new idPool implementation. Nothing else has been changed otherwise.