From 436e9cec10bbb71678acb93b019a3d5823ed676c Mon Sep 17 00:00:00 2001 From: OpenShift Merge Robot Date: Thu, 19 Jan 2023 20:16:40 -0500 Subject: [PATCH] Merge pull request #429 from dtfranz/metrics-map-mutex OCPBUGS-5523: Catalog, fatal error: concurrent map read and map write Signed-off-by: dtfranz Upstream-repository: operator-lifecycle-manager Upstream-commit: 2a49a4dddeb3e0fc38b44925bf9bd0d3931d4ff4 --- .../pkg/metrics/metrics.go | 45 +++++++++++++++---- .../pkg/metrics/metrics.go | 45 +++++++++++++++---- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/metrics/metrics.go b/staging/operator-lifecycle-manager/pkg/metrics/metrics.go index 6efeb7cd67..ab3d3cb3cb 100644 --- a/staging/operator-lifecycle-manager/pkg/metrics/metrics.go +++ b/staging/operator-lifecycle-manager/pkg/metrics/metrics.go @@ -1,6 +1,7 @@ package metrics import ( + "sync" "time" "github.com/prometheus/client_golang/prometheus" @@ -200,12 +201,37 @@ var ( }, ) - // subscriptionSyncCounters keeps a record of the promethues 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 @@ -281,14 +307,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), - } + }) } } @@ -303,7 +330,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 || @@ -319,7 +346,7 @@ func UpdateSubsSyncCounterStorage(sub *operatorsv1alpha1.Subscription) { counterValues.channel = sub.Spec.Channel counterValues.approvalStrategy = approvalStrategy - subscriptionSyncCounters[sub.GetName()] = counterValues + subscriptionSyncCounters.setValues(sub.GetName(), counterValues) } } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/metrics/metrics.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/metrics/metrics.go index 6efeb7cd67..ab3d3cb3cb 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/metrics/metrics.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/metrics/metrics.go @@ -1,6 +1,7 @@ package metrics import ( + "sync" "time" "github.com/prometheus/client_golang/prometheus" @@ -200,12 +201,37 @@ var ( }, ) - // subscriptionSyncCounters keeps a record of the promethues 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 @@ -281,14 +307,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), - } + }) } } @@ -303,7 +330,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 || @@ -319,7 +346,7 @@ func UpdateSubsSyncCounterStorage(sub *operatorsv1alpha1.Subscription) { counterValues.channel = sub.Spec.Channel counterValues.approvalStrategy = approvalStrategy - subscriptionSyncCounters[sub.GetName()] = counterValues + subscriptionSyncCounters.setValues(sub.GetName(), counterValues) } }