From 8b7e5f91849194ca428d15bf72744b2ce0e40f42 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Wed, 27 May 2015 12:32:39 +0200 Subject: [PATCH] Stop holding TargetManager lock when stopping components. TargetProviders may flush some last changes to the target manager before actually stopping. To properly read those form the channel the target manager must not be locked while stopping a provider. --- retrieval/targetmanager.go | 43 ++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/retrieval/targetmanager.go b/retrieval/targetmanager.go index 7e0ecd918..9875d8851 100644 --- a/retrieval/targetmanager.go +++ b/retrieval/targetmanager.go @@ -93,6 +93,9 @@ func (tm *TargetManager) Run() { } } + tm.m.Lock() + defer tm.m.Unlock() + tm.removeTargets(func(src string) bool { if _, ok := sources[src]; ok { return false @@ -125,12 +128,12 @@ func fullSource(cfg *config.ScrapeConfig, src string) string { // Stop all background processing. func (tm *TargetManager) Stop() { - tm.m.Lock() - defer tm.m.Unlock() - + tm.m.RLock() if tm.running { - tm.stop(true) + defer tm.stop(true) } + // Return the lock before calling tm.stop(). + defer tm.m.RUnlock() } // stop background processing of the target manager. If removeTargets is true, @@ -139,11 +142,25 @@ func (tm *TargetManager) stop(removeTargets bool) { log.Info("Stopping target manager...") defer log.Info("Target manager stopped.") - for _, provs := range tm.providers { - for _, p := range provs { - p.Stop() - } + tm.m.Lock() + provs := []TargetProvider{} + for _, ps := range tm.providers { + provs = append(provs, ps...) } + tm.m.Unlock() + + var wg sync.WaitGroup + wg.Add(len(provs)) + for _, prov := range provs { + go func(p TargetProvider) { + p.Stop() + wg.Done() + }(prov) + } + wg.Wait() + + tm.m.Lock() + defer tm.m.Unlock() if removeTargets { tm.removeTargets(nil) @@ -268,10 +285,11 @@ func (tm *TargetManager) Pools() map[string][]*Target { // ApplyConfig resets the manager's target providers and job configurations as defined // by the new cfg. The state of targets that are valid in the new configuration remains unchanged. func (tm *TargetManager) ApplyConfig(cfg *config.Config) { - tm.m.Lock() - defer tm.m.Unlock() + tm.m.RLock() + running := tm.running + tm.m.RUnlock() - if tm.running { + if running { tm.stop(false) // Even if updating the config failed, we want to continue rather than stop scraping anything. defer tm.Run() @@ -282,6 +300,9 @@ func (tm *TargetManager) ApplyConfig(cfg *config.Config) { providers[scfg] = ProvidersFromConfig(scfg) } + tm.m.Lock() + defer tm.m.Unlock() + tm.globalLabels = cfg.GlobalConfig.Labels tm.providers = providers }