From 466a499fcb5cd89042a2b8764cd3aa8699b67ea8 Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Thu, 18 Jan 2018 16:05:55 -0800 Subject: [PATCH] Move equivalence class hash code. This moves the equivalence hashing code from algorithm/predicates/utils.go to core/equivalence_cache.go. In the process, making the hashing function and hashing function factory both injectable dependencies is removed. --- pkg/scheduler/algorithm/predicates/BUILD | 1 - pkg/scheduler/algorithm/predicates/utils.go | 45 ------------- pkg/scheduler/algorithm/types.go | 2 - .../algorithmprovider/defaults/defaults.go | 7 -- pkg/scheduler/core/equivalence_cache.go | 62 ++++++++++++----- pkg/scheduler/core/equivalence_cache_test.go | 66 +++---------------- pkg/scheduler/core/generic_scheduler.go | 2 +- pkg/scheduler/factory/factory.go | 12 ++-- pkg/scheduler/factory/plugins.go | 11 ---- 9 files changed, 59 insertions(+), 149 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD index a028dac6259..85b38d86817 100644 --- a/pkg/scheduler/algorithm/predicates/BUILD +++ b/pkg/scheduler/algorithm/predicates/BUILD @@ -34,7 +34,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", "//vendor/k8s.io/client-go/listers/storage/v1:go_default_library", diff --git a/pkg/scheduler/algorithm/predicates/utils.go b/pkg/scheduler/algorithm/predicates/utils.go index 612f85c60db..2b9c94f9dc8 100644 --- a/pkg/scheduler/algorithm/predicates/utils.go +++ b/pkg/scheduler/algorithm/predicates/utils.go @@ -19,7 +19,6 @@ package predicates import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/kubernetes/pkg/scheduler/algorithm" schedutil "k8s.io/kubernetes/pkg/scheduler/util" ) @@ -67,50 +66,6 @@ func CreateSelectorFromLabels(aL map[string]string) labels.Selector { return labels.Set(aL).AsSelector() } -// EquivalencePodGenerator is a generator of equivalence class for pod with consideration of PVC info. -type EquivalencePodGenerator struct { - pvcInfo PersistentVolumeClaimInfo -} - -// NewEquivalencePodGenerator returns a getEquivalencePod method with consideration of PVC info. -func NewEquivalencePodGenerator(pvcInfo PersistentVolumeClaimInfo) algorithm.GetEquivalencePodFunc { - g := &EquivalencePodGenerator{ - pvcInfo: pvcInfo, - } - return g.getEquivalencePod -} - -// GetEquivalencePod returns a equivalencePod which contains a group of pod attributes which can be reused. -func (e *EquivalencePodGenerator) getEquivalencePod(pod *v1.Pod) interface{} { - // For equivalence hash to be formally correct, lists and maps - // in the equivalencePod should be normalized. (e.g. by sorting - // them) However, the vast majority of equivalent pod classes - // are expected to be created from a single pod template, so - // they will all have the same ordering. - return &equivalencePod{ - Labels: pod.Labels, - Affinity: pod.Spec.Affinity, - Containers: pod.Spec.Containers, - InitContainers: pod.Spec.InitContainers, - NodeName: &pod.Spec.NodeName, - NodeSelector: pod.Spec.NodeSelector, - Tolerations: pod.Spec.Tolerations, - Volumes: pod.Spec.Volumes, - } -} - -// equivalencePod is a group of pod attributes which can be reused as equivalence to schedule other pods. -type equivalencePod struct { - Labels map[string]string - Affinity *v1.Affinity - Containers []v1.Container // note about ordering - InitContainers []v1.Container // note about ordering - NodeName *string - NodeSelector map[string]string - Tolerations []v1.Toleration - Volumes []v1.Volume // note about ordering -} - // portsConflict check whether existingPorts and wantPorts conflict with each other // return true if we have a conflict func portsConflict(existingPorts schedutil.HostPortInfo, wantPorts []*v1.ContainerPort) bool { diff --git a/pkg/scheduler/algorithm/types.go b/pkg/scheduler/algorithm/types.go index f6ff3b49427..8a40bfd6928 100644 --- a/pkg/scheduler/algorithm/types.go +++ b/pkg/scheduler/algorithm/types.go @@ -75,8 +75,6 @@ type PredicateFailureReason interface { GetReason() string } -type GetEquivalencePodFunc func(pod *v1.Pod) interface{} - // NodeLister interface represents anything that can list nodes for a scheduler. type NodeLister interface { // We explicitly return []*v1.Node, instead of v1.NodeList, to avoid diff --git a/pkg/scheduler/algorithmprovider/defaults/defaults.go b/pkg/scheduler/algorithmprovider/defaults/defaults.go index 11b0a54042a..016a4fcb050 100644 --- a/pkg/scheduler/algorithmprovider/defaults/defaults.go +++ b/pkg/scheduler/algorithmprovider/defaults/defaults.go @@ -77,13 +77,6 @@ func init() { // Fit is determined by node selector query. factory.RegisterFitPredicate(predicates.MatchNodeSelectorPred, predicates.PodMatchNodeSelector) - // Use equivalence class to speed up heavy predicates phase. - factory.RegisterGetEquivalencePodFunction( - func(args factory.PluginFactoryArgs) algorithm.GetEquivalencePodFunc { - return predicates.NewEquivalencePodGenerator(args.PVCInfo) - }, - ) - // ServiceSpreadingPriority is a priority config factory that spreads pods by minimizing // the number of pods (belonging to the same service) on the same node. // Register the factory so that it's available, but do not include it as part of the default priorities diff --git a/pkg/scheduler/core/equivalence_cache.go b/pkg/scheduler/core/equivalence_cache.go index 5d9bda7eafe..d6bb34633ba 100644 --- a/pkg/scheduler/core/equivalence_cache.go +++ b/pkg/scheduler/core/equivalence_cache.go @@ -37,8 +37,7 @@ const maxCacheEntries = 100 // 2. function to get equivalence pod type EquivalenceCache struct { sync.RWMutex - getEquivalencePod algorithm.GetEquivalencePodFunc - algorithmCache map[string]AlgorithmCache + algorithmCache map[string]AlgorithmCache } // The AlgorithmCache stores PredicateMap with predicate name as key @@ -62,10 +61,9 @@ func newAlgorithmCache() AlgorithmCache { } } -func NewEquivalenceCache(getEquivalencePodFunc algorithm.GetEquivalencePodFunc) *EquivalenceCache { +func NewEquivalenceCache() *EquivalenceCache { return &EquivalenceCache{ - getEquivalencePod: getEquivalencePodFunc, - algorithmCache: make(map[string]AlgorithmCache), + algorithmCache: make(map[string]AlgorithmCache), } } @@ -208,15 +206,47 @@ func (ec *EquivalenceCache) InvalidateCachedPredicateItemForPodAdd(pod *v1.Pod, ec.InvalidateCachedPredicateItem(nodeName, invalidPredicates) } -// getHashEquivalencePod returns the hash of equivalence pod. -// 1. equivalenceHash -// 2. if equivalence hash is valid -func (ec *EquivalenceCache) getHashEquivalencePod(pod *v1.Pod) (uint64, bool) { - equivalencePod := ec.getEquivalencePod(pod) - if equivalencePod != nil { - hash := fnv.New32a() - hashutil.DeepHashObject(hash, equivalencePod) - return uint64(hash.Sum32()), true - } - return 0, false +// getEquivalenceHash computes a hash of the given pod. +// The hashing function returns the same value for any two pods that are +// equivalent from the perspective of scheduling. +func (ec *EquivalenceCache) getEquivalenceHash(pod *v1.Pod) (uint64, bool) { + equivalencePod := getEquivalencePod(pod) + hash := fnv.New32a() + hashutil.DeepHashObject(hash, equivalencePod) + return uint64(hash.Sum32()), true +} + +// equivalencePod is the set of pod attributes which must match for two pods to +// be considered equivalent for scheduling purposes. For correctness, this must +// include any Pod field which is used by a FitPredicate. +// +// NOTE: For equivalence hash to be formally correct, lists and maps in the +// equivalencePod should be normalized. (e.g. by sorting them) However, the +// vast majority of equivalent pod classes are expected to be created from a +// single pod template, so they will all have the same ordering. +type equivalencePod struct { + Namespace *string + Labels map[string]string + Affinity *v1.Affinity + Containers []v1.Container // See note about ordering + InitContainers []v1.Container // See note about ordering + NodeName *string + NodeSelector map[string]string + Tolerations []v1.Toleration + Volumes []v1.Volume // See note about ordering +} + +// getEquivalencePod returns the equivalencePod for a Pod. +func getEquivalencePod(pod *v1.Pod) *equivalencePod { + return &equivalencePod{ + Namespace: &pod.Namespace, + Labels: pod.Labels, + Affinity: pod.Spec.Affinity, + Containers: pod.Spec.Containers, + InitContainers: pod.Spec.InitContainers, + NodeName: &pod.Spec.NodeName, + NodeSelector: pod.Spec.NodeSelector, + Tolerations: pod.Spec.Tolerations, + Volumes: pod.Spec.Volumes, + } } diff --git a/pkg/scheduler/core/equivalence_cache_test.go b/pkg/scheduler/core/equivalence_cache_test.go index fb2096295f3..1ae821edc72 100644 --- a/pkg/scheduler/core/equivalence_cache_test.go +++ b/pkg/scheduler/core/equivalence_cache_test.go @@ -190,9 +190,7 @@ func TestUpdateCachedPredicateItem(t *testing.T) { }, } for _, test := range tests { - // this case does not need to calculate equivalence hash, just pass an empty function - fakeGetEquivalencePodFunc := func(pod *v1.Pod) interface{} { return nil } - ecache := NewEquivalenceCache(fakeGetEquivalencePodFunc) + ecache := NewEquivalenceCache() if test.expectPredicateMap { ecache.algorithmCache[test.nodeName] = newAlgorithmCache() predicateItem := HostPredicate{ @@ -310,9 +308,7 @@ func TestPredicateWithECache(t *testing.T) { } for _, test := range tests { - // this case does not need to calculate equivalence hash, just pass an empty function - fakeGetEquivalencePodFunc := func(pod *v1.Pod) interface{} { return nil } - ecache := NewEquivalenceCache(fakeGetEquivalencePodFunc) + ecache := NewEquivalenceCache() // set cached item to equivalence cache ecache.UpdateCachedPredicateItem( test.podName, @@ -361,27 +357,7 @@ func TestGetHashEquivalencePod(t *testing.T) { testNamespace := "test" - pvcInfo := predicates.FakePersistentVolumeClaimInfo{ - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol1", Name: "someEBSVol1", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "someEBSVol1"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol2", Name: "someEBSVol2", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "someNonEBSVol"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol3-0", Name: "someEBSVol3-0", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "pvcWithDeletedPV"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol3-1", Name: "someEBSVol3-1", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "anotherPVCWithDeletedPV"}, - }, - } - - // use default equivalence class generator - ecache := NewEquivalenceCache(predicates.NewEquivalencePodGenerator(pvcInfo)) + ecache := NewEquivalenceCache() isController := true @@ -607,7 +583,7 @@ func TestGetHashEquivalencePod(t *testing.T) { for _, test := range tests { for i, podInfo := range test.podInfoList { testPod := podInfo.pod - hash, isValid := ecache.getHashEquivalencePod(testPod) + hash, isValid := ecache.getEquivalenceHash(testPod) if isValid != podInfo.hashIsValid { t.Errorf("Failed: pod %v is expected to have valid hash", testPod) } @@ -668,9 +644,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) { }, }, } - // this case does not need to calculate equivalence hash, just pass an empty function - fakeGetEquivalencePodFunc := func(pod *v1.Pod) interface{} { return nil } - ecache := NewEquivalenceCache(fakeGetEquivalencePodFunc) + ecache := NewEquivalenceCache() for _, test := range tests { // set cached item to equivalence cache @@ -736,9 +710,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) { }, }, } - // this case does not need to calculate equivalence hash, just pass an empty function - fakeGetEquivalencePodFunc := func(pod *v1.Pod) interface{} { return nil } - ecache := NewEquivalenceCache(fakeGetEquivalencePodFunc) + ecache := NewEquivalenceCache() for _, test := range tests { // set cached item to equivalence cache @@ -763,31 +735,9 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) { } func BenchmarkEquivalenceHash(b *testing.B) { - testNamespace := "test" - - pvcInfo := predicates.FakePersistentVolumeClaimInfo{ - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol1", Name: "someEBSVol1", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "someEBSVol1"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol2", Name: "someEBSVol2", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "someNonEBSVol"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol3-0", Name: "someEBSVol3-0", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "pvcWithDeletedPV"}, - }, - { - ObjectMeta: metav1.ObjectMeta{UID: "someEBSVol3-1", Name: "someEBSVol3-1", Namespace: testNamespace}, - Spec: v1.PersistentVolumeClaimSpec{VolumeName: "anotherPVCWithDeletedPV"}, - }, - } - - // use default equivalence class generator - cache := NewEquivalenceCache(predicates.NewEquivalencePodGenerator(pvcInfo)) + cache := NewEquivalenceCache() pod := makeBasicPod() for i := 0; i < b.N; i++ { - cache.getHashEquivalencePod(pod) + cache.getEquivalenceHash(pod) } } diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index da04ff45ad6..e3f47a387f0 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -418,7 +418,7 @@ func podFitsOnNode( if ecache != nil { // getHashEquivalencePod will return immediately if no equivalence pod found - equivalenceHash, eCacheAvailable = ecache.getHashEquivalencePod(pod) + equivalenceHash, eCacheAvailable = ecache.getEquivalenceHash(pod) } podsAdded := false // We run predicates twice in some cases. If the node has greater or equal priority diff --git a/pkg/scheduler/factory/factory.go b/pkg/scheduler/factory/factory.go index dfc5d6c3db8..3f6e6e815e0 100644 --- a/pkg/scheduler/factory/factory.go +++ b/pkg/scheduler/factory/factory.go @@ -135,6 +135,8 @@ type configFactory struct { alwaysCheckAllPredicates bool } +var _ scheduler.Configurator = &configFactory{} + // NewConfigFactory initializes the default implementation of a Configurator To encourage eventual privatization of the struct type, we only // return the interface. func NewConfigFactory( @@ -967,14 +969,8 @@ func (f *configFactory) CreateFromKeys(predicateKeys, priorityKeys sets.String, } // Init equivalence class cache - if f.enableEquivalenceClassCache && getEquivalencePodFuncFactory != nil { - pluginArgs, err := f.getPluginArgs() - if err != nil { - return nil, err - } - f.equivalencePodCache = core.NewEquivalenceCache( - getEquivalencePodFuncFactory(*pluginArgs), - ) + if f.enableEquivalenceClassCache { + f.equivalencePodCache = core.NewEquivalenceCache() glog.Info("Created equivalence class cache") } diff --git a/pkg/scheduler/factory/plugins.go b/pkg/scheduler/factory/plugins.go index 1447d9487c2..9ec28df06fc 100644 --- a/pkg/scheduler/factory/plugins.go +++ b/pkg/scheduler/factory/plugins.go @@ -75,9 +75,6 @@ type PriorityConfigFactory struct { Weight int } -// EquivalencePodFuncFactory produces a function to get equivalence class for given pod. -type EquivalencePodFuncFactory func(PluginFactoryArgs) algorithm.GetEquivalencePodFunc - var ( schedulerFactoryMutex sync.Mutex @@ -90,9 +87,6 @@ var ( // Registered metadata producers priorityMetadataProducer PriorityMetadataProducerFactory predicateMetadataProducer PredicateMetadataProducerFactory - - // get equivalence pod function - getEquivalencePodFuncFactory EquivalencePodFuncFactory ) const ( @@ -341,11 +335,6 @@ func RegisterCustomPriorityFunction(policy schedulerapi.PriorityPolicy) string { return RegisterPriorityConfigFactory(policy.Name, *pcf) } -// RegisterGetEquivalencePodFunction registers equivalenceFuncFactory to produce equivalence class for given pod. -func RegisterGetEquivalencePodFunction(equivalenceFuncFactory EquivalencePodFuncFactory) { - getEquivalencePodFuncFactory = equivalenceFuncFactory -} - // IsPriorityFunctionRegistered is useful for testing providers. func IsPriorityFunctionRegistered(name string) bool { schedulerFactoryMutex.Lock()