From 767c0709b18c17179ad6f77e843db3b6bbd88999 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Fri, 6 Jan 2017 18:43:41 +0100 Subject: [PATCH] Retrieval: Avoid copying Target retreival.Target contains a mutex. It was copied in the Targets() call. This potentially can wreak a lot of havoc. It might even have caused the issues reported as #2266 and #2262 . --- retrieval/targetmanager.go | 6 +++--- web/api/v1/api.go | 2 +- web/api/v1/api_test.go | 10 +++++----- web/web.go | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/retrieval/targetmanager.go b/retrieval/targetmanager.go index 48d641185..28e387a96 100644 --- a/retrieval/targetmanager.go +++ b/retrieval/targetmanager.go @@ -132,16 +132,16 @@ func (tm *TargetManager) reload() { } // Targets returns the targets currently being scraped bucketed by their job name. -func (tm *TargetManager) Targets() []Target { +func (tm *TargetManager) Targets() []*Target { tm.mtx.RLock() defer tm.mtx.RUnlock() - targets := []Target{} + targets := []*Target{} for _, ps := range tm.targetSets { ps.sp.mtx.RLock() for _, t := range ps.sp.targets { - targets = append(targets, *t) + targets = append(targets, t) } ps.sp.mtx.RUnlock() diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 340a6c58f..152091aab 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -68,7 +68,7 @@ func (e *apiError) Error() string { } type targetRetriever interface { - Targets() []retrieval.Target + Targets() []*retrieval.Target } type response struct { diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index daf8a6819..27343edb6 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -33,9 +33,9 @@ import ( "github.com/prometheus/prometheus/retrieval" ) -type targetRetrieverFunc func() []retrieval.Target +type targetRetrieverFunc func() []*retrieval.Target -func (f targetRetrieverFunc) Targets() []retrieval.Target { +func (f targetRetrieverFunc) Targets() []*retrieval.Target { return f() } @@ -57,9 +57,9 @@ func TestEndpoints(t *testing.T) { now := model.Now() - tr := targetRetrieverFunc(func() []retrieval.Target { - return []retrieval.Target{ - *retrieval.NewTarget( + tr := targetRetrieverFunc(func() []*retrieval.Target { + return []*retrieval.Target{ + retrieval.NewTarget( model.LabelSet{ model.SchemeLabel: "http", model.AddressLabel: "example.com:8080", diff --git a/web/web.go b/web/web.go index 3d16e09d0..2469c27c7 100644 --- a/web/web.go +++ b/web/web.go @@ -373,14 +373,14 @@ func (h *Handler) rules(w http.ResponseWriter, r *http.Request) { func (h *Handler) targets(w http.ResponseWriter, r *http.Request) { // Bucket targets by job label - tps := map[string][]retrieval.Target{} + tps := map[string][]*retrieval.Target{} for _, t := range h.targetManager.Targets() { job := string(t.Labels()[model.JobLabel]) tps[job] = append(tps[job], t) } h.executeTemplate(w, "targets.html", struct { - TargetPools map[string][]retrieval.Target + TargetPools map[string][]*retrieval.Target }{ TargetPools: tps, })