From fc462857bbe7c3bab0fd723764a3e4bd4de3f726 Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Wed, 26 May 2021 18:12:07 +0800 Subject: [PATCH] Add controllerUID index to improve ReplicaSetController performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of listing all ReplicaSets in the namespace and checking their controller UID, this patch adds a controllerUID index to the ReplicaSet store and use it to get ReplicaSets with same controller, which reduces the cost from O(#ReplicaSets) to O(1). Benchmark results: ``` name old time/op new time/op delta GetReplicaSetsWithSameController-48 18.2µs ± 9% 0.4µs ± 5% -97.64% (p=0.008 n=5+5) name old alloc/op new alloc/op delta GetReplicaSetsWithSameController-48 4.18kB ± 0% 0.05kB ± 0% -98.85% (p=0.008 n=5+5) name old allocs/op new allocs/op delta GetReplicaSetsWithSameController-48 15.0 ± 0% 2.0 ± 0% -86.67% (p=0.008 n=5+5) ``` --- pkg/controller/replicaset/replica_set.go | 30 +++++++++++++----- pkg/controller/replicaset/replica_set_test.go | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/pkg/controller/replicaset/replica_set.go b/pkg/controller/replicaset/replica_set.go index cef41bf9c90..932512941b2 100644 --- a/pkg/controller/replicaset/replica_set.go +++ b/pkg/controller/replicaset/replica_set.go @@ -69,6 +69,10 @@ const ( // The number of times we retry updating a ReplicaSet's status. statusUpdateRetries = 1 + + // controllerUIDIndex is the name for the ReplicaSet store's index function, + // which is to index by ReplicaSet's controllerUID. + controllerUIDIndex = "controllerUID" ) // ReplicaSetController is responsible for synchronizing ReplicaSet objects stored @@ -96,6 +100,7 @@ type ReplicaSetController struct { // rsListerSynced returns true if the pod store has been synced at least once. // Added as a member to the struct to allow injection for testing. rsListerSynced cache.InformerSynced + rsIndexer cache.Indexer // A store of pods, populated by the shared informer passed to NewReplicaSetController podLister corelisters.PodLister @@ -145,6 +150,20 @@ func NewBaseController(rsInformer appsinformers.ReplicaSetInformer, podInformer UpdateFunc: rsc.updateRS, DeleteFunc: rsc.deleteRS, }) + rsInformer.Informer().AddIndexers(cache.Indexers{ + controllerUIDIndex: func(obj interface{}) ([]string, error) { + rs, ok := obj.(*apps.ReplicaSet) + if !ok { + return []string{}, nil + } + controllerRef := metav1.GetControllerOf(rs) + if controllerRef == nil { + return []string{}, nil + } + return []string{string(controllerRef.UID)}, nil + }, + }) + rsc.rsIndexer = rsInformer.Informer().GetIndexer() rsc.rsLister = rsInformer.Lister() rsc.rsListerSynced = rsInformer.Informer().HasSynced @@ -201,17 +220,14 @@ func (rsc *ReplicaSetController) getReplicaSetsWithSameController(rs *apps.Repli return nil } - allRSs, err := rsc.rsLister.ReplicaSets(rs.Namespace).List(labels.Everything()) + objects, err := rsc.rsIndexer.ByIndex(controllerUIDIndex, string(controllerRef.UID)) if err != nil { utilruntime.HandleError(err) return nil } - - var relatedRSs []*apps.ReplicaSet - for _, r := range allRSs { - if ref := metav1.GetControllerOf(r); ref != nil && ref.UID == controllerRef.UID { - relatedRSs = append(relatedRSs, r) - } + relatedRSs := make([]*apps.ReplicaSet, 0, len(objects)) + for _, obj := range objects { + relatedRSs = append(relatedRSs, obj.(*apps.ReplicaSet)) } if klog.V(2).Enabled() { diff --git a/pkg/controller/replicaset/replica_set_test.go b/pkg/controller/replicaset/replica_set_test.go index 3c09d2816cc..437a0a7d961 100644 --- a/pkg/controller/replicaset/replica_set_test.go +++ b/pkg/controller/replicaset/replica_set_test.go @@ -36,6 +36,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" @@ -438,6 +439,36 @@ func TestGetReplicaSetsWithSameController(t *testing.T) { } } +func BenchmarkGetReplicaSetsWithSameController(b *testing.B) { + stopCh := make(chan struct{}) + defer close(stopCh) + controller, informers := testNewReplicaSetControllerFromClient(clientset.NewForConfigOrDie(&restclient.Config{Host: "", ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}), stopCh, BurstReplicas) + + targetRS := newReplicaSet(1, map[string]string{"foo": "bar"}) + targetRS.Name = "rs1" + targetRS.ObjectMeta.OwnerReferences[0].UID = "123456" + informers.Apps().V1().ReplicaSets().Informer().GetIndexer().Add(targetRS) + relatedRS := newReplicaSet(1, map[string]string{"foo": "bar"}) + relatedRS.Name = "rs2" + relatedRS.ObjectMeta.OwnerReferences[0].UID = "123456" + informers.Apps().V1().ReplicaSets().Informer().GetIndexer().Add(relatedRS) + for i := 0; i < 100; i++ { + unrelatedRS := newReplicaSet(1, map[string]string{"foo": fmt.Sprintf("baz-%d", i)}) + unrelatedRS.Name = fmt.Sprintf("rs-%d", i) + unrelatedRS.ObjectMeta.OwnerReferences[0].UID = types.UID(fmt.Sprintf("%d", i)) + informers.Apps().V1().ReplicaSets().Informer().GetIndexer().Add(unrelatedRS) + } + + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + gotRSs := controller.getReplicaSetsWithSameController(targetRS) + if len(gotRSs) != 2 { + b.Errorf("Incorrect ReplicaSets number, expected 2, got: %d", len(gotRSs)) + } + } +} + func TestPodControllerLookup(t *testing.T) { stopCh := make(chan struct{}) defer close(stopCh)