diff --git a/tools/cache/thread_safe_store.go b/tools/cache/thread_safe_store.go index 33e6239a..e7232514 100644 --- a/tools/cache/thread_safe_store.go +++ b/tools/cache/thread_safe_store.go @@ -292,6 +292,13 @@ func (c *threadSafeMap) deleteFromIndices(obj interface{}, key string) { set := index[indexValue] if set != nil { set.Delete(key) + + // If we don't delete the set when zero, indices with high cardinality + // short lived resources can cause memory to increase over time from + // unused empty sets. See `kubernetes/kubernetes/issues/84959`. + if len(set) == 0 { + delete(index, indexValue) + } } } } diff --git a/tools/cache/thread_safe_store_test.go b/tools/cache/thread_safe_store_test.go new file mode 100644 index 00000000..267ebcbb --- /dev/null +++ b/tools/cache/thread_safe_store_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2019 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 cache + +import ( + "testing" +) + +func TestThreadSafeStoreDeleteRemovesEmptySetsFromIndex(t *testing.T) { + testIndexer := "testIndexer" + + indexers := Indexers{ + testIndexer: func(obj interface{}) (strings []string, e error) { + indexes := []string{obj.(string)} + return indexes, nil + }, + } + + indices := Indices{} + store := NewThreadSafeStore(indexers, indices).(*threadSafeMap) + + testKey := "testKey" + + store.Add(testKey, testKey) + + // Assumption check, there should be a set for the `testKey` with one element in the added index + set := store.indices[testIndexer][testKey] + + if len(set) != 1 { + t.Errorf("Initial assumption of index backing string set having 1 element failed. Actual elements: %d", len(set)) + return + } + + store.Delete(testKey) + set, present := store.indices[testIndexer][testKey] + + if present { + t.Errorf("Index backing string set not deleted from index. Set length: %d", len(set)) + } +} + +func TestThreadSafeStoreAddKeepsNonEmptySetPostDeleteFromIndex(t *testing.T) { + testIndexer := "testIndexer" + testIndex := "testIndex" + + indexers := Indexers{ + testIndexer: func(obj interface{}) (strings []string, e error) { + indexes := []string{testIndex} + return indexes, nil + }, + } + + indices := Indices{} + store := NewThreadSafeStore(indexers, indices).(*threadSafeMap) + + store.Add("retain", "retain") + store.Add("delete", "delete") + + // Assumption check, there should be a set for the `testIndex` with two elements + set := store.indices[testIndexer][testIndex] + + if len(set) != 2 { + t.Errorf("Initial assumption of index backing string set having 2 elements failed. Actual elements: %d", len(set)) + return + } + + store.Delete("delete") + set, present := store.indices[testIndexer][testIndex] + + if !present { + t.Errorf("Index backing string set erroneously deleted from index.") + return + } + + if len(set) != 1 { + t.Errorf("Index backing string set has incorrect length, expect 1. Set length: %d", len(set)) + } +}