From e2b3626e0cde417ffd9c3d384d281bad4ff44e25 Mon Sep 17 00:00:00 2001
From: beorn7 <beorn@soundcloud.com>
Date: Mon, 22 Aug 2016 19:25:33 +0200
Subject: [PATCH] retrieval: Clean up target group map on config reload

Also, remove unused `providers` field in targetSet.

If the config file changes, we recreate all providers (by calling
`providersFromConfig`) and retrieve all targets anew from the newly
created providers. From that perspective, it cannot harm to clean up
the target group map in the targetSet. Not doing so (as it was the
case so far) keeps stale targets around. This mattered if an existing
key in the target group map was not overwritten in the initial fetch
of all targets from the providers. Examples where that mattered:

```
scrape_configs:
- job_name: "foo"
  static_configs:
  - targets: ["foo:9090"]
  - targets: ["bar:9090"]
```
updated to:
```
scrape_configs:
- job_name: "foo"
  static_configs:
  - targets: ["foo:9090"]
```

`bar:9090` would still be monitored. (The static provider just
enumerates the target groups. If the number of target groups
decreases, the old ones stay around.

```
scrape_configs:
- job_name: "foo"
  dns_sd_configs:
  - names:
    - "srv.name.one.example.org"
```
updated to:
```
scrape_configs:
- job_name: "foo"
  dns_sd_configs:
  - names:
    - "srv.name.two.example.org"
```

Now both SRV records are still monitored. The SRV name is part of the
key in the target group map, thus the new one is just added and the
old ane stays around.

Obviously, this should have tests, and should have tests before, not
only for this case. This is the quick fix. I have created
https://github.com/prometheus/prometheus/issues/1906 to track test
creation.

Fixes https://github.com/prometheus/prometheus/issues/1610 .
---
 retrieval/targetmanager.go | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/retrieval/targetmanager.go b/retrieval/targetmanager.go
index 4788f27dc..b7e7e915c 100644
--- a/retrieval/targetmanager.go
+++ b/retrieval/targetmanager.go
@@ -180,8 +180,7 @@ type targetSet struct {
 	mtx sync.RWMutex
 
 	// Sets of targets by a source string that is unique across target providers.
-	tgroups   map[string][]*Target
-	providers map[string]TargetProvider
+	tgroups map[string][]*Target
 
 	scrapePool *scrapePool
 	config     *config.ScrapeConfig
@@ -193,7 +192,6 @@ type targetSet struct {
 
 func newTargetSet(cfg *config.ScrapeConfig, app storage.SampleAppender) *targetSet {
 	ts := &targetSet{
-		tgroups:    map[string][]*Target{},
 		scrapePool: newScrapePool(cfg, app),
 		syncCh:     make(chan struct{}, 1),
 		config:     cfg,
@@ -272,6 +270,11 @@ func (ts *targetSet) runProviders(ctx context.Context, providers map[string]Targ
 	}
 	ctx, ts.cancelProviders = context.WithCancel(ctx)
 
+	// (Re-)create a fresh tgroups map to not keep stale targets around. We
+	// will retrieve all targets below anyway, so cleaning up everything is
+	// safe and doesn't inflict any additional cost.
+	ts.tgroups = map[string][]*Target{}
+
 	for name, prov := range providers {
 		wg.Add(1)