From a0f8a36e489e4f6965ff51d2904fdd951c13fbf9 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 21 Sep 2017 17:41:35 -0400 Subject: [PATCH] Remove GC rate limiter metrics --- pkg/controller/garbagecollector/BUILD | 2 - .../garbagecollector/garbagecollector.go | 23 +++---- .../garbagecollector/graph_builder.go | 5 -- pkg/controller/garbagecollector/operations.go | 16 +++-- .../garbagecollector/rate_limiter_helper.go | 60 ------------------- 5 files changed, 21 insertions(+), 85 deletions(-) delete mode 100644 pkg/controller/garbagecollector/rate_limiter_helper.go diff --git a/pkg/controller/garbagecollector/BUILD b/pkg/controller/garbagecollector/BUILD index cee3e623b37..76702791052 100644 --- a/pkg/controller/garbagecollector/BUILD +++ b/pkg/controller/garbagecollector/BUILD @@ -15,13 +15,11 @@ go_library( "graph_builder.go", "operations.go", "patch.go", - "rate_limiter_helper.go", "uid_cache.go", ], deps = [ "//pkg/controller:go_default_library", "//pkg/controller/garbagecollector/metaonly:go_default_library", - "//pkg/util/metrics:go_default_library", "//pkg/util/reflector/prometheus:go_default_library", "//pkg/util/workqueue/prometheus:go_default_library", "//vendor/github.com/golang/glog:go_default_library", diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go index a430e5f964e..88536408713 100644 --- a/pkg/controller/garbagecollector/garbagecollector.go +++ b/pkg/controller/garbagecollector/garbagecollector.go @@ -69,9 +69,6 @@ type GarbageCollector struct { // garbage collector attempts to orphan the dependents of the items in the attemptToOrphan queue, then deletes the items. attemptToOrphan workqueue.RateLimitingInterface dependencyGraphBuilder *GraphBuilder - // used to register exactly once the rate limiter of the dynamic client - // used by the garbage collector controller. - registeredRateLimiter *RegisteredRateLimiter // GC caches the owners that do not exist according to the API server. absentOwnerCache *UIDCache sharedInformers informers.SharedInformerFactory @@ -92,19 +89,17 @@ func NewGarbageCollector( attemptToOrphan := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_orphan") absentOwnerCache := NewUIDCache(500) gc := &GarbageCollector{ - clientPool: clientPool, - restMapper: mapper, - attemptToDelete: attemptToDelete, - attemptToOrphan: attemptToOrphan, - registeredRateLimiter: NewRegisteredRateLimiter(deletableResources), - absentOwnerCache: absentOwnerCache, + clientPool: clientPool, + restMapper: mapper, + attemptToDelete: attemptToDelete, + attemptToOrphan: attemptToOrphan, + absentOwnerCache: absentOwnerCache, } gb := &GraphBuilder{ - metaOnlyClientPool: metaOnlyClientPool, - informersStarted: informersStarted, - registeredRateLimiterForControllers: NewRegisteredRateLimiter(deletableResources), - restMapper: mapper, - graphChanges: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_graph_changes"), + metaOnlyClientPool: metaOnlyClientPool, + informersStarted: informersStarted, + restMapper: mapper, + graphChanges: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_graph_changes"), uidToNode: &concurrentUIDToNode{ uidToNode: make(map[types.UID]*node), }, diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go index b4199222b41..c19305312e9 100644 --- a/pkg/controller/garbagecollector/graph_builder.go +++ b/pkg/controller/garbagecollector/graph_builder.go @@ -92,9 +92,6 @@ type GraphBuilder struct { // metaOnlyClientPool uses a special codec, which removes fields except for // apiVersion, kind, and metadata during decoding. metaOnlyClientPool dynamic.ClientPool - // used to register exactly once the rate limiters of the clients used by - // the `monitors`. - registeredRateLimiterForControllers *RegisteredRateLimiter // monitors are the producer of the graphChanges queue, graphBuilder alters // the in-memory graph according to the changes. graphChanges workqueue.RateLimitingInterface @@ -204,8 +201,6 @@ func (gb *GraphBuilder) controllerFor(resource schema.GroupVersionResource, kind if err != nil { return nil, err } - // TODO: since the gv is never unregistered, isn't this a memory leak? - gb.registeredRateLimiterForControllers.registerIfNotPresent(resource.GroupVersion(), client, "garbage_collector_monitoring") _, monitor := cache.NewInformer( listWatcher(client, resource), nil, diff --git a/pkg/controller/garbagecollector/operations.go b/pkg/controller/garbagecollector/operations.go index 2464b200fa9..36897547b97 100644 --- a/pkg/controller/garbagecollector/operations.go +++ b/pkg/controller/garbagecollector/operations.go @@ -50,7 +50,9 @@ func (gc *GarbageCollector) apiResource(apiVersion, kind string, namespaced bool func (gc *GarbageCollector) deleteObject(item objectReference, policy *metav1.DeletionPropagation) error { fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind) client, err := gc.clientPool.ClientForGroupVersionKind(fqKind) - gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation") + if err != nil { + return err + } resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0) if err != nil { return err @@ -64,7 +66,9 @@ func (gc *GarbageCollector) deleteObject(item objectReference, policy *metav1.De func (gc *GarbageCollector) getObject(item objectReference) (*unstructured.Unstructured, error) { fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind) client, err := gc.clientPool.ClientForGroupVersionKind(fqKind) - gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation") + if err != nil { + return nil, err + } resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0) if err != nil { return nil, err @@ -75,7 +79,9 @@ func (gc *GarbageCollector) getObject(item objectReference) (*unstructured.Unstr func (gc *GarbageCollector) updateObject(item objectReference, obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind) client, err := gc.clientPool.ClientForGroupVersionKind(fqKind) - gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation") + if err != nil { + return nil, err + } resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0) if err != nil { return nil, err @@ -86,7 +92,9 @@ func (gc *GarbageCollector) updateObject(item objectReference, obj *unstructured func (gc *GarbageCollector) patchObject(item objectReference, patch []byte) (*unstructured.Unstructured, error) { fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind) client, err := gc.clientPool.ClientForGroupVersionKind(fqKind) - gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation") + if err != nil { + return nil, err + } resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0) if err != nil { return nil, err diff --git a/pkg/controller/garbagecollector/rate_limiter_helper.go b/pkg/controller/garbagecollector/rate_limiter_helper.go deleted file mode 100644 index 26d89389154..00000000000 --- a/pkg/controller/garbagecollector/rate_limiter_helper.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package garbagecollector - -import ( - "fmt" - "strings" - "sync" - - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" - "k8s.io/kubernetes/pkg/util/metrics" -) - -// RegisteredRateLimiter records the registered RateLimters to avoid -// duplication. -type RegisteredRateLimiter struct { - rateLimiters map[schema.GroupVersion]*sync.Once -} - -// NewRegisteredRateLimiter returns a new RegisteredRateLimiter. -// TODO: NewRegisteredRateLimiter is not dynamic. We need to find a better way -// when GC dynamically change the resources it monitors. -func NewRegisteredRateLimiter(resources map[schema.GroupVersionResource]struct{}) *RegisteredRateLimiter { - rateLimiters := make(map[schema.GroupVersion]*sync.Once) - for resource := range resources { - gv := resource.GroupVersion() - if _, found := rateLimiters[gv]; !found { - rateLimiters[gv] = &sync.Once{} - } - } - return &RegisteredRateLimiter{rateLimiters: rateLimiters} -} - -func (r *RegisteredRateLimiter) registerIfNotPresent(gv schema.GroupVersion, client dynamic.Interface, prefix string) { - once, found := r.rateLimiters[gv] - if !found { - return - } - once.Do(func() { - if rateLimiter := client.GetRateLimiter(); rateLimiter != nil { - group := strings.Replace(gv.Group, ".", ":", -1) - metrics.RegisterMetricAndTrackRateLimiterUsage(fmt.Sprintf("%s_%s_%s", prefix, group, gv.Version), rateLimiter) - } - }) -}