diff --git a/discovery/manager.go b/discovery/manager.go index ad1e5ad06..e10cfc7bd 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 2caae0080..e06fefc70 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 +}