Merge pull request #2510 from rarruda/fix/reduce_locking

perf: use concurrent map when storing metrics

pkg/metrics_store/metrics_store.go

@@ -20,7 +20,6 @@ import (
 	"sync"
 
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/types"
 
 	"k8s.io/kube-state-metrics/v2/pkg/metric"
 )
@@ -33,7 +32,7 @@ type MetricsStore struct {
 	// metric families, containing a slice of metrics. We need to keep metrics
 	// grouped by metric families in order to zip families with their help text in
 	// MetricsStore.WriteAll().
-	metrics map[types.UID][][]byte
+	metrics sync.Map
 
 	// generateMetricsFunc generates metrics based on a given Kubernetes object
 	// and returns them grouped by metric family.
@@ -42,17 +41,14 @@ type MetricsStore struct {
 	// later on zipped with with their corresponding metric families in
 	// MetricStore.WriteAll().
 	headers []string
-
-	// Protects metrics
-	mutex sync.RWMutex
 }
 
 // NewMetricsStore returns a new MetricsStore
 func NewMetricsStore(headers []string, generateFunc func(interface{}) []metric.FamilyInterface) *MetricsStore {
 	return &MetricsStore{
 		generateMetricsFunc: generateFunc,
 		headers:             headers,
-		metrics:             map[types.UID][][]byte{},
+		metrics:             sync.Map{},
 	}
 }
 
@@ -66,17 +62,14 @@ func (s *MetricsStore) Add(obj interface{}) error {
 		return err
 	}
 
-	s.mutex.Lock()
-	defer s.mutex.Unlock()
-
 	families := s.generateMetricsFunc(obj)
 	familyStrings := make([][]byte, len(families))
 
 	for i, f := range families {
 		familyStrings[i] = f.ByteSlice()
 	}
 
-	s.metrics[o.GetUID()] = familyStrings
+	s.metrics.Store(o.GetUID(), familyStrings)
 
 	return nil
 }
@@ -95,10 +88,7 @@ func (s *MetricsStore) Delete(obj interface{}) error {
 		return err
 	}
 
-	s.mutex.Lock()
-	defer s.mutex.Unlock()
-
-	delete(s.metrics, o.GetUID())
+	s.metrics.Delete(o.GetUID())
 
 	return nil
 }
@@ -126,9 +116,7 @@ func (s *MetricsStore) GetByKey(_ string) (item interface{}, exists bool, err er
 // Replace will delete the contents of the store, using instead the
 // given list.
 func (s *MetricsStore) Replace(list []interface{}, _ string) error {
-	s.mutex.Lock()
-	s.metrics = map[types.UID][][]byte{}
-	s.mutex.Unlock()
+	s.metrics.Clear()
 
 	for _, o := range list {
 		err := s.Add(o)

pkg/metrics_store/metrics_writer.go

@@ -56,31 +56,35 @@ func (m MetricsWriter) WriteAll(w io.Writer) error {
 		return nil
 	}
 
-	for _, s := range m.stores {
-		s.mutex.RLock()
-		defer func(s *MetricsStore) {
-			s.mutex.RUnlock()
-		}(s)
-	}
-
 	for i, help := range m.stores[0].headers {
 		if help != "" && help != "\n" {
 			help += "\n"
 		}
 
-		if len(m.stores[0].metrics) > 0 {
-			_, err := w.Write([]byte(help))
+		var err error
+		m.stores[0].metrics.Range(func(_ interface{}, _ interface{}) bool {
+			_, err = w.Write([]byte(help))
 			if err != nil {
-				return fmt.Errorf("failed to write help text: %v", err)
+				err = fmt.Errorf("failed to write help text: %v", err)
 			}
+			return false
+		})
+		if err != nil {
+			return err
 		}
 
 		for _, s := range m.stores {
-			for _, metricFamilies := range s.metrics {
-				_, err := w.Write(metricFamilies[i])
+			s.metrics.Range(func(_ interface{}, value interface{}) bool {
+				metricFamilies := value.([][]byte)
+				_, err = w.Write(metricFamilies[i])
 				if err != nil {
-					return fmt.Errorf("failed to write metrics family: %v", err)
+					err = fmt.Errorf("failed to write metrics family: %v", err)
+					return false
 				}
+				return true
+			})
+			if err != nil {
+				return err
 			}
 		}
 	}