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

Memory leak issue with massive use of disposable client #69

Open
yookoala opened this issue Feb 18, 2021 · 1 comment
Open

Memory leak issue with massive use of disposable client #69

yookoala opened this issue Feb 18, 2021 · 1 comment
Assignees
Labels

Comments

@yookoala
Copy link
Owner

yookoala commented Feb 18, 2021

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
Copy link
Owner Author

@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.

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

No branches or pull requests

1 participant