From ca6b312c97789451ff924559f59350715a60a8d9 Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Mon, 23 Apr 2018 15:02:39 -0700 Subject: [PATCH 1/5] Add RunPredicate to EquivalenceCache. This method combines "lookup" and "update" into one operation. The benefit is that this method call is very similar to running an ordinary predicate, so callers can simplify their code. --- pkg/scheduler/core/equivalence_cache.go | 31 ++++ pkg/scheduler/core/equivalence_cache_test.go | 144 +++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/pkg/scheduler/core/equivalence_cache.go b/pkg/scheduler/core/equivalence_cache.go index 2976a3c654b..acc912cedd8 100644 --- a/pkg/scheduler/core/equivalence_cache.go +++ b/pkg/scheduler/core/equivalence_cache.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/scheduler/algorithm" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" + "k8s.io/kubernetes/pkg/scheduler/schedulercache" hashutil "k8s.io/kubernetes/pkg/util/hash" "github.com/golang/glog" @@ -70,6 +71,36 @@ func NewEquivalenceCache() *EquivalenceCache { } } +// RunPredicate will return a cached predicate result. In case of a cache miss, the predicate will +// be run and its results cached for the next call. +// +// NOTE: RunPredicate will not update the equivalence cache if the given NodeInfo is stale. +func (ec *EquivalenceCache) RunPredicate( + pred algorithm.FitPredicate, + predicateKey string, + pod *v1.Pod, + meta algorithm.PredicateMetadata, + nodeInfo *schedulercache.NodeInfo, + equivClassInfo *equivalenceClassInfo, + cache schedulercache.Cache, +) (bool, []algorithm.PredicateFailureReason, error) { + ec.Lock() + defer ec.Unlock() + fit, reasons, invalid := ec.PredicateWithECache(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, equivClassInfo.hash, false) + if !invalid { + return fit, reasons, nil + } + fit, reasons, err := pred(pod, meta, nodeInfo) + if err != nil { + return fit, reasons, err + } + // Skip update if NodeInfo is stale. + if cache != nil && cache.IsUpToDate(nodeInfo) { + ec.UpdateCachedPredicateItem(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, fit, reasons, equivClassInfo.hash, false) + } + return fit, reasons, nil +} + // UpdateCachedPredicateItem updates pod predicate for equivalence class func (ec *EquivalenceCache) UpdateCachedPredicateItem( podName, nodeName, predicateKey string, diff --git a/pkg/scheduler/core/equivalence_cache_test.go b/pkg/scheduler/core/equivalence_cache_test.go index 63e4fd780bf..42623e3a7d7 100644 --- a/pkg/scheduler/core/equivalence_cache_test.go +++ b/pkg/scheduler/core/equivalence_cache_test.go @@ -17,6 +17,7 @@ limitations under the License. package core import ( + "errors" "reflect" "sync" "testing" @@ -157,6 +158,149 @@ type predicateItemType struct { reasons []algorithm.PredicateFailureReason } +// upToDateCache is a fake Cache where IsUpToDate always returns true. +type upToDateCache = schedulertesting.FakeCache + +// staleNodeCache is a fake Cache where IsUpToDate always returns false. +type staleNodeCache struct { + schedulertesting.FakeCache +} + +func (c *staleNodeCache) IsUpToDate(*schedulercache.NodeInfo) bool { return false } + +// mockPredicate provides an algorithm.FitPredicate with pre-set return values. +type mockPredicate struct { + fit bool + reasons []algorithm.PredicateFailureReason + err error + callCount int +} + +func (p *mockPredicate) predicate(*v1.Pod, algorithm.PredicateMetadata, *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) { + p.callCount++ + return p.fit, p.reasons, p.err +} + +func TestRunPredicate(t *testing.T) { + tests := []struct { + name string + pred mockPredicate + cache schedulercache.Cache + expectFit, expectCacheHit, expectCacheWrite bool + expectedReasons []algorithm.PredicateFailureReason + expectedError string + }{ + { + name: "pod fits/cache hit", + pred: mockPredicate{}, + cache: &upToDateCache{}, + expectFit: true, + expectCacheHit: true, + expectCacheWrite: false, + }, + { + name: "pod fits/cache miss", + pred: mockPredicate{fit: true}, + cache: &upToDateCache{}, + expectFit: true, + expectCacheHit: false, + expectCacheWrite: true, + }, + { + name: "pod fits/cache miss/no write", + pred: mockPredicate{fit: true}, + cache: &staleNodeCache{}, + expectFit: true, + expectCacheHit: false, + expectCacheWrite: false, + }, + { + name: "pod doesn't fit/cache miss", + pred: mockPredicate{reasons: []algorithm.PredicateFailureReason{predicates.ErrFakePredicate}}, + cache: &upToDateCache{}, + expectFit: false, + expectCacheHit: false, + expectCacheWrite: true, + expectedReasons: []algorithm.PredicateFailureReason{predicates.ErrFakePredicate}, + }, + { + name: "pod doesn't fit/cache hit", + pred: mockPredicate{}, + cache: &upToDateCache{}, + expectFit: false, + expectCacheHit: true, + expectCacheWrite: false, + expectedReasons: []algorithm.PredicateFailureReason{predicates.ErrFakePredicate}, + }, + { + name: "predicate error", + pred: mockPredicate{err: errors.New("This is expected")}, + cache: &upToDateCache{}, + expectFit: false, + expectCacheHit: false, + expectCacheWrite: false, + expectedError: "This is expected", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + node := schedulercache.NewNodeInfo() + node.SetNode(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n1"}}) + pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1"}} + meta := algorithm.EmptyPredicateMetadataProducer(nil, nil) + + ecache := NewEquivalenceCache() + equivClass := ecache.getEquivalenceClassInfo(pod) + if test.expectCacheHit { + ecache.UpdateCachedPredicateItem(pod.Name, node.Node().Name, "testPredicate", test.expectFit, test.expectedReasons, equivClass.hash) + } + + fit, reasons, err := ecache.RunPredicate(test.pred.predicate, "testPredicate", pod, meta, node, equivClass, test.cache) + + if err != nil { + if err.Error() != test.expectedError { + t.Errorf("Expected error %v but got %v", test.expectedError, err) + } + } else if len(test.expectedError) > 0 { + t.Errorf("Expected error %v but got nil", test.expectedError) + } + if fit && !test.expectFit { + t.Errorf("pod should not fit") + } + if !fit && test.expectFit { + t.Errorf("pod should fit") + } + if len(reasons) != len(test.expectedReasons) { + t.Errorf("Expected failures: %v but got %v", test.expectedReasons, reasons) + } else { + for i, reason := range reasons { + if reason != test.expectedReasons[i] { + t.Errorf("Expected failures: %v but got %v", test.expectedReasons, reasons) + break + } + } + } + if test.expectCacheHit && test.pred.callCount != 0 { + t.Errorf("Predicate should not be called") + } + if !test.expectCacheHit && test.pred.callCount == 0 { + t.Errorf("Predicate should be called") + } + _, _, invalid := ecache.PredicateWithECache(pod.Name, node.Node().Name, "testPredicate", equivClass.hash) + if invalid && test.expectCacheWrite { + t.Errorf("Cache write should happen") + } + if !test.expectCacheHit && test.expectCacheWrite && invalid { + t.Errorf("Cache write should happen") + } + if !test.expectCacheHit && !test.expectCacheWrite && !invalid { + t.Errorf("Cache write should not happen") + } + }) + } +} + func TestUpdateCachedPredicateItem(t *testing.T) { tests := []struct { name string From e67b3225a48fca0ee163a1a38a53a0cb5856f01d Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Wed, 25 Apr 2018 10:54:17 -0700 Subject: [PATCH 2/5] Remove predicateResults map from podFitsOnNode. The purpose of this map is to combine two predicate results before writing to the equivalence cache. However, the branch that combines results is unreachable. 1. Combining results happens in the second iteration of the outer loop. 2. There is only a second iteration when podsAdded is true. 3. We skip equiv. cache when podsAdded is true. --- pkg/scheduler/core/generic_scheduler.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index c222b80d314..2edb785ba1f 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -468,7 +468,6 @@ func podFitsOnNode( eCacheAvailable bool failedPredicates []algorithm.PredicateFailureReason ) - predicateResults := make(map[string]HostPredicate) podsAdded := false // We run predicates twice in some cases. If the node has greater or equal priority @@ -532,20 +531,11 @@ func podFitsOnNode( } if eCacheAvailable { - // Store data to update equivClassCacheafter this loop. - if res, exists := predicateResults[predicateKey]; exists { - res.Fit = res.Fit && fit - res.FailReasons = append(res.FailReasons, reasons...) - predicateResults[predicateKey] = res - } else { - predicateResults[predicateKey] = HostPredicate{Fit: fit, FailReasons: reasons} - } // Skip update if NodeInfo is stale. if cache != nil && cache.IsUpToDate(info) { - result := predicateResults[predicateKey] ecache.UpdateCachedPredicateItem( pod.GetName(), info.Node().GetName(), - predicateKey, result.Fit, result.FailReasons, equivCacheInfo.hash, false) + predicateKey, fit, reasons, equivCacheInfo.hash, false) } } } From 55662f26f19207c812fbb566204c5975bc55d4ff Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Wed, 25 Apr 2018 11:08:03 -0700 Subject: [PATCH 3/5] Simplify logic in podFitsOnNode. Use new (*EquivalenceCache).RunPredicate to simplify how we read and update the equivalence cache items. --- pkg/scheduler/core/generic_scheduler.go | 38 ++++--------------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 2edb785ba1f..960054b0d14 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -508,39 +508,11 @@ func podFitsOnNode( ) //TODO (yastij) : compute average predicate restrictiveness to export it as Prometheus metric if predicate, exist := predicateFuncs[predicateKey]; exist { - // Use an in-line function to guarantee invocation of ecache.Unlock() - // when the in-line function returns. - func() { - var invalid bool - if eCacheAvailable { - // Lock ecache here to avoid a race condition against cache invalidation invoked - // in event handlers. This race has existed despite locks in equivClassCacheimplementation. - ecache.Lock() - defer ecache.Unlock() - // PredicateWithECache will return its cached predicate results. - fit, reasons, invalid = ecache.PredicateWithECache( - pod.GetName(), info.Node().GetName(), - predicateKey, equivCacheInfo.hash, false) - } - - if !eCacheAvailable || invalid { - // we need to execute predicate functions since equivalence cache does not work - fit, reasons, err = predicate(pod, metaToUse, nodeInfoToUse) - if err != nil { - return - } - - if eCacheAvailable { - // Skip update if NodeInfo is stale. - if cache != nil && cache.IsUpToDate(info) { - ecache.UpdateCachedPredicateItem( - pod.GetName(), info.Node().GetName(), - predicateKey, fit, reasons, equivCacheInfo.hash, false) - } - } - } - }() - + if eCacheAvailable { + fit, reasons, err = ecache.RunPredicate(predicate, predicateKey, pod, metaToUse, nodeInfoToUse, equivCacheInfo, cache) + } else { + fit, reasons, err = predicate(pod, metaToUse, nodeInfoToUse) + } if err != nil { return false, []algorithm.PredicateFailureReason{}, err } From b85184227dda7b2f1c12d0d96245c3a85abf0e2b Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Wed, 25 Apr 2018 11:32:51 -0700 Subject: [PATCH 4/5] Rename exported methods on EquivalenceCache. This changes two methods in EquivalenceCache to be unexported, because they should no longer be called by users of this type. (Even users in the same package!) --- pkg/scheduler/core/equivalence_cache.go | 19 +++++++++---------- pkg/scheduler/core/equivalence_cache_test.go | 18 +++++++++--------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/pkg/scheduler/core/equivalence_cache.go b/pkg/scheduler/core/equivalence_cache.go index acc912cedd8..1eb7df685d7 100644 --- a/pkg/scheduler/core/equivalence_cache.go +++ b/pkg/scheduler/core/equivalence_cache.go @@ -86,7 +86,7 @@ func (ec *EquivalenceCache) RunPredicate( ) (bool, []algorithm.PredicateFailureReason, error) { ec.Lock() defer ec.Unlock() - fit, reasons, invalid := ec.PredicateWithECache(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, equivClassInfo.hash, false) + fit, reasons, invalid := ec.lookupResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, equivClassInfo.hash, false) if !invalid { return fit, reasons, nil } @@ -96,13 +96,13 @@ func (ec *EquivalenceCache) RunPredicate( } // Skip update if NodeInfo is stale. if cache != nil && cache.IsUpToDate(nodeInfo) { - ec.UpdateCachedPredicateItem(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, fit, reasons, equivClassInfo.hash, false) + ec.updateResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, fit, reasons, equivClassInfo.hash, false) } return fit, reasons, nil } -// UpdateCachedPredicateItem updates pod predicate for equivalence class -func (ec *EquivalenceCache) UpdateCachedPredicateItem( +// updateResult updates the cached result of a predicate. +func (ec *EquivalenceCache) updateResult( podName, nodeName, predicateKey string, fit bool, reasons []algorithm.PredicateFailureReason, @@ -134,12 +134,11 @@ func (ec *EquivalenceCache) UpdateCachedPredicateItem( glog.V(5).Infof("Updated cached predicate: %v for pod: %v on node: %s, with item %v", predicateKey, podName, nodeName, predicateItem) } -// PredicateWithECache returns: -// 1. if fit -// 2. reasons if not fit -// 3. if this cache is invalid -// based on cached predicate results -func (ec *EquivalenceCache) PredicateWithECache( +// lookupResult returns cached predicate results: +// 1. if pod fit +// 2. reasons if pod did not fit +// 3. if cache item is not found +func (ec *EquivalenceCache) lookupResult( podName, nodeName, predicateKey string, equivalenceHash uint64, needLock bool, ) (bool, []algorithm.PredicateFailureReason, bool) { diff --git a/pkg/scheduler/core/equivalence_cache_test.go b/pkg/scheduler/core/equivalence_cache_test.go index 42623e3a7d7..512fbf2afb0 100644 --- a/pkg/scheduler/core/equivalence_cache_test.go +++ b/pkg/scheduler/core/equivalence_cache_test.go @@ -253,7 +253,7 @@ func TestRunPredicate(t *testing.T) { ecache := NewEquivalenceCache() equivClass := ecache.getEquivalenceClassInfo(pod) if test.expectCacheHit { - ecache.UpdateCachedPredicateItem(pod.Name, node.Node().Name, "testPredicate", test.expectFit, test.expectedReasons, equivClass.hash) + ecache.updateResult(pod.Name, node.Node().Name, "testPredicate", test.expectFit, test.expectedReasons, equivClass.hash) } fit, reasons, err := ecache.RunPredicate(test.pred.predicate, "testPredicate", pod, meta, node, equivClass, test.cache) @@ -287,7 +287,7 @@ func TestRunPredicate(t *testing.T) { if !test.expectCacheHit && test.pred.callCount == 0 { t.Errorf("Predicate should be called") } - _, _, invalid := ecache.PredicateWithECache(pod.Name, node.Node().Name, "testPredicate", equivClass.hash) + _, _, invalid := ecache.lookupResult(pod.Name, node.Node().Name, "testPredicate", equivClass.hash) if invalid && test.expectCacheWrite { t.Errorf("Cache write should happen") } @@ -301,7 +301,7 @@ func TestRunPredicate(t *testing.T) { } } -func TestUpdateCachedPredicateItem(t *testing.T) { +func TestUpdateResult(t *testing.T) { tests := []struct { name string pod string @@ -350,7 +350,7 @@ func TestUpdateCachedPredicateItem(t *testing.T) { test.equivalenceHash: predicateItem, }) } - ecache.UpdateCachedPredicateItem( + ecache.updateResult( test.pod, test.nodeName, test.predicateKey, @@ -374,7 +374,7 @@ func TestUpdateCachedPredicateItem(t *testing.T) { } } -func TestPredicateWithECache(t *testing.T) { +func TestLookupResult(t *testing.T) { tests := []struct { name string podName string @@ -460,7 +460,7 @@ func TestPredicateWithECache(t *testing.T) { for _, test := range tests { ecache := NewEquivalenceCache() // set cached item to equivalence cache - ecache.UpdateCachedPredicateItem( + ecache.updateResult( test.podName, test.nodeName, test.predicateKey, @@ -476,7 +476,7 @@ func TestPredicateWithECache(t *testing.T) { ecache.InvalidateCachedPredicateItem(test.nodeName, predicateKeys) } // calculate predicate with equivalence cache - fit, reasons, invalid := ecache.PredicateWithECache(test.podName, + fit, reasons, invalid := ecache.lookupResult(test.podName, test.nodeName, test.predicateKey, test.equivalenceHashForCalPredicate, @@ -668,7 +668,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) { for _, test := range tests { // set cached item to equivalence cache - ecache.UpdateCachedPredicateItem( + ecache.updateResult( test.podName, test.nodeName, testPredicate, @@ -735,7 +735,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) { for _, test := range tests { // set cached item to equivalence cache - ecache.UpdateCachedPredicateItem( + ecache.updateResult( test.podName, test.nodeName, testPredicate, From 79d30b1ad623c4e71b8d9ed33f4b18cc353908c7 Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Wed, 25 Apr 2018 11:40:35 -0700 Subject: [PATCH 5/5] Hide EquivalenceCache mutex from users. Since the equiv. cache lock no longer needs to be held across multiple method calls, move the locking inside and don't expose it to users. --- pkg/scheduler/core/equivalence_cache.go | 33 +++++++------------- pkg/scheduler/core/equivalence_cache_test.go | 19 ++++++++--- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/pkg/scheduler/core/equivalence_cache.go b/pkg/scheduler/core/equivalence_cache.go index 1eb7df685d7..1b6183ba99f 100644 --- a/pkg/scheduler/core/equivalence_cache.go +++ b/pkg/scheduler/core/equivalence_cache.go @@ -38,7 +38,7 @@ const maxCacheEntries = 100 // 1. a map of AlgorithmCache with node name as key // 2. function to get equivalence pod type EquivalenceCache struct { - sync.RWMutex + mu sync.Mutex algorithmCache map[string]AlgorithmCache } @@ -84,9 +84,9 @@ func (ec *EquivalenceCache) RunPredicate( equivClassInfo *equivalenceClassInfo, cache schedulercache.Cache, ) (bool, []algorithm.PredicateFailureReason, error) { - ec.Lock() - defer ec.Unlock() - fit, reasons, invalid := ec.lookupResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, equivClassInfo.hash, false) + ec.mu.Lock() + defer ec.mu.Unlock() + fit, reasons, invalid := ec.lookupResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, equivClassInfo.hash) if !invalid { return fit, reasons, nil } @@ -96,7 +96,7 @@ func (ec *EquivalenceCache) RunPredicate( } // Skip update if NodeInfo is stale. if cache != nil && cache.IsUpToDate(nodeInfo) { - ec.updateResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, fit, reasons, equivClassInfo.hash, false) + ec.updateResult(pod.GetName(), nodeInfo.Node().GetName(), predicateKey, fit, reasons, equivClassInfo.hash) } return fit, reasons, nil } @@ -107,12 +107,7 @@ func (ec *EquivalenceCache) updateResult( fit bool, reasons []algorithm.PredicateFailureReason, equivalenceHash uint64, - needLock bool, ) { - if needLock { - ec.Lock() - defer ec.Unlock() - } if _, exist := ec.algorithmCache[nodeName]; !exist { ec.algorithmCache[nodeName] = newAlgorithmCache() } @@ -140,12 +135,8 @@ func (ec *EquivalenceCache) updateResult( // 3. if cache item is not found func (ec *EquivalenceCache) lookupResult( podName, nodeName, predicateKey string, - equivalenceHash uint64, needLock bool, + equivalenceHash uint64, ) (bool, []algorithm.PredicateFailureReason, bool) { - if needLock { - ec.RLock() - defer ec.RUnlock() - } glog.V(5).Infof("Begin to calculate predicate: %v for pod: %s on node: %s based on equivalence cache", predicateKey, podName, nodeName) if algorithmCache, exist := ec.algorithmCache[nodeName]; exist { @@ -170,8 +161,8 @@ func (ec *EquivalenceCache) InvalidateCachedPredicateItem(nodeName string, predi if len(predicateKeys) == 0 { return } - ec.Lock() - defer ec.Unlock() + ec.mu.Lock() + defer ec.mu.Unlock() if algorithmCache, exist := ec.algorithmCache[nodeName]; exist { for predicateKey := range predicateKeys { algorithmCache.predicatesCache.Remove(predicateKey) @@ -185,8 +176,8 @@ func (ec *EquivalenceCache) InvalidateCachedPredicateItemOfAllNodes(predicateKey if len(predicateKeys) == 0 { return } - ec.Lock() - defer ec.Unlock() + ec.mu.Lock() + defer ec.mu.Unlock() // algorithmCache uses nodeName as key, so we just iterate it and invalid given predicates for _, algorithmCache := range ec.algorithmCache { for predicateKey := range predicateKeys { @@ -199,8 +190,8 @@ func (ec *EquivalenceCache) InvalidateCachedPredicateItemOfAllNodes(predicateKey // InvalidateAllCachedPredicateItemOfNode marks all cached items on given node as invalid func (ec *EquivalenceCache) InvalidateAllCachedPredicateItemOfNode(nodeName string) { - ec.Lock() - defer ec.Unlock() + ec.mu.Lock() + defer ec.mu.Unlock() delete(ec.algorithmCache, nodeName) glog.V(5).Infof("Done invalidating all cached predicates on node: %s", nodeName) } diff --git a/pkg/scheduler/core/equivalence_cache_test.go b/pkg/scheduler/core/equivalence_cache_test.go index 512fbf2afb0..0129fd2db2a 100644 --- a/pkg/scheduler/core/equivalence_cache_test.go +++ b/pkg/scheduler/core/equivalence_cache_test.go @@ -253,7 +253,9 @@ func TestRunPredicate(t *testing.T) { ecache := NewEquivalenceCache() equivClass := ecache.getEquivalenceClassInfo(pod) if test.expectCacheHit { + ecache.mu.Lock() ecache.updateResult(pod.Name, node.Node().Name, "testPredicate", test.expectFit, test.expectedReasons, equivClass.hash) + ecache.mu.Unlock() } fit, reasons, err := ecache.RunPredicate(test.pred.predicate, "testPredicate", pod, meta, node, equivClass, test.cache) @@ -287,7 +289,9 @@ func TestRunPredicate(t *testing.T) { if !test.expectCacheHit && test.pred.callCount == 0 { t.Errorf("Predicate should be called") } + ecache.mu.Lock() _, _, invalid := ecache.lookupResult(pod.Name, node.Node().Name, "testPredicate", equivClass.hash) + ecache.mu.Unlock() if invalid && test.expectCacheWrite { t.Errorf("Cache write should happen") } @@ -350,6 +354,7 @@ func TestUpdateResult(t *testing.T) { test.equivalenceHash: predicateItem, }) } + ecache.mu.Lock() ecache.updateResult( test.pod, test.nodeName, @@ -357,8 +362,8 @@ func TestUpdateResult(t *testing.T) { test.fit, test.reasons, test.equivalenceHash, - true, ) + ecache.mu.Unlock() value, ok := ecache.algorithmCache[test.nodeName].predicatesCache.Get(test.predicateKey) if !ok { @@ -460,6 +465,7 @@ func TestLookupResult(t *testing.T) { for _, test := range tests { ecache := NewEquivalenceCache() // set cached item to equivalence cache + ecache.mu.Lock() ecache.updateResult( test.podName, test.nodeName, @@ -467,8 +473,8 @@ func TestLookupResult(t *testing.T) { test.cachedItem.fit, test.cachedItem.reasons, test.equivalenceHashForUpdatePredicate, - true, ) + ecache.mu.Unlock() // if we want to do invalid, invalid the cached item if test.expectedInvalidPredicateKey { predicateKeys := sets.NewString() @@ -476,12 +482,13 @@ func TestLookupResult(t *testing.T) { ecache.InvalidateCachedPredicateItem(test.nodeName, predicateKeys) } // calculate predicate with equivalence cache + ecache.mu.Lock() fit, reasons, invalid := ecache.lookupResult(test.podName, test.nodeName, test.predicateKey, test.equivalenceHashForCalPredicate, - true, ) + ecache.mu.Unlock() // returned invalid should match expectedInvalidPredicateKey or expectedInvalidEquivalenceHash if test.equivalenceHashForUpdatePredicate != test.equivalenceHashForCalPredicate { if invalid != test.expectedInvalidEquivalenceHash { @@ -668,6 +675,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) { for _, test := range tests { // set cached item to equivalence cache + ecache.mu.Lock() ecache.updateResult( test.podName, test.nodeName, @@ -675,8 +683,8 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) { test.cachedItem.fit, test.cachedItem.reasons, test.equivalenceHashForUpdatePredicate, - true, ) + ecache.mu.Unlock() } // invalidate cached predicate for all nodes @@ -735,6 +743,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) { for _, test := range tests { // set cached item to equivalence cache + ecache.mu.Lock() ecache.updateResult( test.podName, test.nodeName, @@ -742,8 +751,8 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) { test.cachedItem.fit, test.cachedItem.reasons, test.equivalenceHashForUpdatePredicate, - true, ) + ecache.mu.Unlock() } for _, test := range tests {