OCPBUGS-5523: Catalog, fatal error: concurrent map read and map write (…

…#2913) * protected subscriptionSyncCounters access to prevent concurrent map writes Signed-off-by: Daniel Franz <[email protected]> * organize map and mutex into single struct Signed-off-by: Daniel Franz <[email protected]> * initialize struct Signed-off-by: Daniel Franz <[email protected]> * use RWMutex to allow concurrent reads Signed-off-by: Daniel Franz <[email protected]> Signed-off-by: Daniel Franz <[email protected]>
operator-framework · Jan 16, 2023 · 2a49a4d · 2a49a4d
1 parent 6c564a6
commit 2a49a4d
Showing 1 changed file with 36 additions and 9 deletions.
diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
@@ -1,6 +1,7 @@
 package metrics
 
 import (
+	"sync"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -199,12 +200,37 @@ var (
 		},
 	)
 
-	// subscriptionSyncCounters keeps a record of the Prometheus counters emitted by
-	// Subscription objects. The key of a record is the Subscription name, while the value
-	//  is struct containing label values used in the counter
-	subscriptionSyncCounters = make(map[string]subscriptionSyncLabelValues)
+	subscriptionSyncCounters = newSubscriptionSyncCounter()
 )
 
+// subscriptionSyncCounter keeps a record of the Prometheus counters emitted by
+// Subscription objects. The key of a record is the Subscription name, while the value
+// is struct containing label values used in the counter. Read and Write access are
+// protected by mutex.
+type subscriptionSyncCounter struct {
+	counters     map[string]subscriptionSyncLabelValues
+	countersLock sync.RWMutex
+}
+
+func newSubscriptionSyncCounter() subscriptionSyncCounter {
+	return subscriptionSyncCounter{
+		counters: make(map[string]subscriptionSyncLabelValues),
+	}
+}
+
+func (s *subscriptionSyncCounter) setValues(key string, val subscriptionSyncLabelValues) {
+	s.countersLock.Lock()
+	defer s.countersLock.Unlock()
+	s.counters[key] = val
+}
+
+func (s *subscriptionSyncCounter) readValues(key string) (subscriptionSyncLabelValues, bool) {
+	s.countersLock.RLock()
+	defer s.countersLock.RUnlock()
+	val, ok := s.counters[key]
+	return val, ok
+}
+
 type subscriptionSyncLabelValues struct {
 	installedCSV     string
 	pkg              string
@@ -280,14 +306,15 @@ func EmitSubMetric(sub *operatorsv1alpha1.Subscription) {
 	if sub.Spec == nil {
 		return
 	}
+
 	SubscriptionSyncCount.WithLabelValues(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package, string(sub.Spec.InstallPlanApproval)).Inc()
-	if _, present := subscriptionSyncCounters[sub.GetName()]; !present {
-		subscriptionSyncCounters[sub.GetName()] = subscriptionSyncLabelValues{
+	if _, present := subscriptionSyncCounters.readValues(sub.GetName()); !present {
+		subscriptionSyncCounters.setValues(sub.GetName(), subscriptionSyncLabelValues{
 			installedCSV:     sub.Status.InstalledCSV,
 			pkg:              sub.Spec.Package,
 			channel:          sub.Spec.Channel,
 			approvalStrategy: string(sub.Spec.InstallPlanApproval),
-		}
+		})
 	}
 }
 
@@ -302,7 +329,7 @@ func UpdateSubsSyncCounterStorage(sub *operatorsv1alpha1.Subscription) {
 	if sub.Spec == nil {
 		return
 	}
-	counterValues := subscriptionSyncCounters[sub.GetName()]
+	counterValues, _ := subscriptionSyncCounters.readValues(sub.GetName())
 	approvalStrategy := string(sub.Spec.InstallPlanApproval)
 
 	if sub.Spec.Channel != counterValues.channel ||
@@ -317,7 +344,7 @@ func UpdateSubsSyncCounterStorage(sub *operatorsv1alpha1.Subscription) {
 		counterValues.channel = sub.Spec.Channel
 		counterValues.approvalStrategy = approvalStrategy
 
-		subscriptionSyncCounters[sub.GetName()] = counterValues
+		subscriptionSyncCounters.setValues(sub.GetName(), counterValues)
 	}
 }