From 9621c2c0ccdb35d3678969ba8e32e69607e3ba73 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 5 Nov 2021 01:13:04 +0100 Subject: [PATCH 1/2] Fix race with targets update during ApplyConfig (#9656) I ended up extending the lock so refTargets remains valid for the duration of the update. Signed-off-by: Julien Pivotto --- discovery/manager.go | 6 ++- discovery/manager_test.go | 88 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/discovery/manager.go b/discovery/manager.go index ad1e5ad064..e10cfc7bd3 100644 --- a/discovery/manager.go +++ b/discovery/manager.go @@ -200,14 +200,14 @@ func (m *Manager) ApplyConfig(cfg map[string]Configs) error { // refTargets keeps reference targets used to populate new subs' targets var refTargets map[string]*targetgroup.Group prov.mu.Lock() + + m.targetsMtx.Lock() for s := range prov.subs { keep = true refTargets = m.targets[poolKey{s, prov.name}] // Remove obsolete subs' targets. if _, ok := prov.newSubs[s]; !ok { - m.targetsMtx.Lock() delete(m.targets, poolKey{s, prov.name}) - m.targetsMtx.Unlock() discoveredTargets.DeleteLabelValues(m.name, s) } } @@ -223,6 +223,8 @@ func (m *Manager) ApplyConfig(cfg map[string]Configs) error { } } } + m.targetsMtx.Unlock() + prov.subs = prov.newSubs prov.newSubs = map[string]struct{}{} prov.mu.Unlock() diff --git a/discovery/manager_test.go b/discovery/manager_test.go index 2caae0080c..e06fefc700 100644 --- a/discovery/manager_test.go +++ b/discovery/manager_test.go @@ -1394,3 +1394,91 @@ func (o onceProvider) Run(_ context.Context, ch chan<- []*targetgroup.Group) { } close(ch) } + +// TestTargetSetTargetGroupsUpdateDuringApplyConfig is used to detect races when +// ApplyConfig happens at the same time as targets update. +func TestTargetSetTargetGroupsUpdateDuringApplyConfig(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + discoveryManager := NewManager(ctx, log.NewNopLogger()) + discoveryManager.updatert = 100 * time.Millisecond + go discoveryManager.Run() + + td := newTestDiscoverer() + + c := map[string]Configs{ + "prometheus": { + td, + }, + } + discoveryManager.ApplyConfig(c) + + var wg sync.WaitGroup + wg.Add(2000) + + start := make(chan struct{}) + for i := 0; i < 1000; i++ { + go func() { + <-start + td.update([]*targetgroup.Group{ + { + Targets: []model.LabelSet{ + {model.AddressLabel: model.LabelValue("127.0.0.1:9090")}, + }, + }, + }) + wg.Done() + }() + } + + for i := 0; i < 1000; i++ { + go func(i int) { + <-start + c := map[string]Configs{ + fmt.Sprintf("prometheus-%d", i): { + td, + }, + } + discoveryManager.ApplyConfig(c) + wg.Done() + }(i) + } + + close(start) + wg.Wait() +} + +// testDiscoverer is a config and a discoverer that can adjust targets with a +// simple function. +type testDiscoverer struct { + up chan<- []*targetgroup.Group + ready chan struct{} +} + +func newTestDiscoverer() *testDiscoverer { + return &testDiscoverer{ + ready: make(chan struct{}), + } +} + +// Name implements Config. +func (t *testDiscoverer) Name() string { + return "test" +} + +// NewDiscoverer implements Config. +func (t *testDiscoverer) NewDiscoverer(DiscovererOptions) (Discoverer, error) { + return t, nil +} + +// Run implements Discoverer. +func (t *testDiscoverer) Run(ctx context.Context, up chan<- []*targetgroup.Group) { + t.up = up + close(t.ready) + <-ctx.Done() +} + +func (t *testDiscoverer) update(tgs []*targetgroup.Group) { + <-t.ready + t.up <- tgs +} From 411021ada9ab41095923b8d2df9365b632fd40c3 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 5 Nov 2021 21:13:21 +0100 Subject: [PATCH 2/2] Release 2.31.1 (#9669) Signed-off-by: Julien Pivotto --- CHANGELOG.md | 5 +++++ VERSION | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80dd066119..a6d3caa9fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.31.1 / 2021-11-05 + +* [BUGFIX] SD: Fix a panic when the experimental discovery manager receives + targets during a reload. #9656 + ## 2.31.0 / 2021-11-02 * [CHANGE] UI: Remove standard PromQL editor in favour of the codemirror-based editor. #9452 diff --git a/VERSION b/VERSION index bafceb320e..70ff1993b1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.31.0 +2.31.1