-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
backport of "Fix memory leak in service mirror" to 2.12 branch #11345
backport of "Fix memory leak in service mirror" to 2.12 branch #11345
Commits on Sep 6, 2023
-
build(deps): NOT bump sigs.k8s.io/gateway-api from 0.5.1 to 0.6.0 (#1…
…0038) * build(deps): bump sigs.k8s.io/gateway-api from 0.5.1 to 0.6.0 ** NOTE ** This was cherry-picked from 62d6d7c in order to acquire the `AddEventHandler` changes that went there. The actualy gateway-api bump was discarded. Bumps [sigs.k8s.io/gateway-api](https://github.com/kubernetes-sigs/gateway-api) from 0.5.1 to 0.6.0. - [Release notes](https://github.com/kubernetes-sigs/gateway-api/releases) - [Changelog](https://github.com/kubernetes-sigs/gateway-api/blob/main/CHANGELOG.md) - [Commits](kubernetes-sigs/gateway-api@v0.5.1...v0.6.0) --- updated-dependencies: - dependency-name: sigs.k8s.io/gateway-api dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Account for possible errors returned from `AddEventHandler` In v0.26.0 client-go's `AddEventHandler` method for informers started returning a registration handle (that we ignore) and an error that we now surface up. * client-go v0.26.0 removed the openstack plugin * Temporary changes to trigger tests in k8s 1.21 - Adds an innocuous change to integration.yml so that all tests get triggered - Hard-code k8s version in `k3d cluster create` invocation to v1.21 * Revert "Temporary changes to trigger tests in k8s 1.21" This reverts commit 3e1fdd0. Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alejandro Pedraza <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 8de4aa7 - Browse repository at this point
Copy the full SHA 8de4aa7View commit details -
Fix memory leak in service mirror (#10833)
Fixes #10746 ## Repro Just by leaving two linked clusters by themselves and monitoring the service mirror pod's RSS (via the `container_memory_working_set_bytes` metric) and goroutines count (via the `go_goroutines` metric) one can see the small but consistent memory increase accompanied by periodic increases of goroutines in batches of 5. ![Screenshot from 2023-04-27 09-28-48](https://user-images.githubusercontent.com/554287/235007199-604dd517-47ae-4f95-a863-21ae16eca898.png) ## Diagnosis Those goroutines count increases can be correlated in the controller's log with the probe worker and cluster watcher recycling: ``` time="2023-04-27T10:06:14Z" level=info msg="Stopping probe worker" probe-key=target time="2023-04-27T10:06:14Z" level=info msg="Starting probe worker" probe-key=target time="2023-04-27T10:06:14Z" level=info msg="Received: Stop" apiAddress="https://192.168.32.4:6443" cluster=remote ``` Enabling pprof, the beginning of the goroutine stack dump look like this: ``` goroutine profile: total 94 10 @ 0x43c976 0x406d3b 0x406878 0x175ee77 0xb2aefe 0xb2adb6 0xb2aca9 0x175edeb 0x175ed95 0xb2cfda 0x46de41 # 0x175ee76 k8s.io/client-go/tools/cache.(*processorListener).run.func1+0x56 /go/pkg/mod/k8s.io/[email protected]/tools/cache/shared_informer.go:968 # 0xb2aefd k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1+0x3d /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:226 # 0xb2adb5 k8s.io/apimachinery/pkg/util/wait.BackoffUntil+0xb5 /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:227 # 0xb2aca8 k8s.io/apimachinery/pkg/util/wait.JitterUntil+0x88 /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:204 # 0x175edea k8s.io/apimachinery/pkg/util/wait.Until+0x6a /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:161 # 0x175ed94 k8s.io/client-go/tools/cache.(*processorListener).run+0x14 /go/pkg/mod/k8s.io/[email protected]/tools/cache/shared_informer.go:967 # 0xb2cfd9 k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1+0x59 /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:72 10 @ 0x43c976 0x44c81c 0x175eb99 0xb2cfda 0x46de41 # 0x175eb98 k8s.io/client-go/tools/cache.(*processorListener).pop+0x118 /go/pkg/mod/k8s.io/[email protected]/tools/cache/shared_informer.go:938 # 0xb2cfd9 k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1+0x59 /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:72 8 @ 0x43c976 0x406d3b 0x406878 0x19f35d3 0x46de41 # 0x19f35d2 k8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartEventWatcher.func1+0x72 /go/pkg/mod/k8s.io/[email protected]/tools/record/event.go:320 8 @ 0x43c976 0x406d3b 0x406878 0xa88825 0x46de41 # 0xa88824 k8s.io/apimachinery/pkg/watch.(*Broadcaster).loop+0x64 /go/pkg/mod/k8s.io/[email protected]/pkg/watch/mux.go:268 8 @ 0x43c976 0x46abb5 0x1a2a7a5 0x46de41 # 0x46abb4 time.Sleep+0x134 /usr/local/go/src/runtime/time.go:195 # 0x1a2a7a4 github.com/linkerd/linkerd2/multicluster/service-mirror.(*Ticker).loop+0x64 /linkerd-build/multicluster/service-mirror/jittered_ticker.go:55 ``` The number of goroutines shown here is almost the same as the number of goroutines count jumps we've seen in the chart when this was taken, which suggests these are indeed the leaks. The service mirror main function contains a loop that restarts the cluster watcher (and with it its probe worker) whenever the main watch following the Link resources terminates (I don't know why that happens). And some resources aren't getting cleaned up upon the restart, producing the leak. It would be interesting to see (as a followup) why that restart is required here, but not in the other controllers. ## Resolution We can map each one of those entries in the dump to these leaks: ### jitterred_ticker.go Each probe worker starts a jittered ticker, whose `Stop()` method was never called. Fixed that through a `defer` statement. ### event.go With every watcher restart a new k8s event recorder was instantiated but it wasn't cleaned. Added a `Shutdown()` call to the event's broadcaster in the cluster watcher's `Stop()` method. ### mux.go, shared_informer.go and friends The cluster watcher attached event handlers to informers for Services, Endpoints and Namespaces. Added `RemoveEventHandler()` to them on the cluster watcher's `Stop()` method. ## Result After the fix, we can see the goroutines count remains stable. ![image](https://user-images.githubusercontent.com/554287/235007727-3b3b6f31-901d-4c05-aedb-9fa3d3178ee1.png)
Configuration menu - View commit details
-
Copy full SHA for d2c2e73 - Browse repository at this point
Copy the full SHA d2c2e73View commit details -
Configuration menu - View commit details
-
Copy full SHA for 43afac1 - Browse repository at this point
Copy the full SHA 43afac1View commit details -
Revert "Update to dev:v39 with Go 1.19 (#10336)"
This reverts commit 43afac1.
Configuration menu - View commit details
-
Copy full SHA for c69acae - Browse repository at this point
Copy the full SHA c69acaeView commit details -
Configuration menu - View commit details
-
Copy full SHA for 66ddb86 - Browse repository at this point
Copy the full SHA 66ddb86View commit details -
Fixes #9616 remove kubectl version check (#9623)
* Fixes #9616 remove kubectl version check Signed-off-by: tomasz.ziolkowski <[email protected]> Signed-off-by: Alejandro Pedraza <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for bf4e7ab - Browse repository at this point
Copy the full SHA bf4e7abView commit details -
Configuration menu - View commit details
-
Copy full SHA for f1a29ca - Browse repository at this point
Copy the full SHA f1a29caView commit details -
Revert "Temporary changes to trigger tests in k8s 1.21"
This reverts commit f1a29ca.
Configuration menu - View commit details
-
Copy full SHA for 8059d96 - Browse repository at this point
Copy the full SHA 8059d96View commit details
Commits on Sep 7, 2023
-
Configuration menu - View commit details
-
Copy full SHA for fd5c258 - Browse repository at this point
Copy the full SHA fd5c258View commit details