diff --git a/targets/cache.go b/targets/cache.go index 28a0ec42..17789d2f 100644 --- a/targets/cache.go +++ b/targets/cache.go @@ -108,17 +108,29 @@ func (c *Cache) refresh(ctx context.Context) error { c.mtx.Lock() defer c.mtx.Unlock() - for _, t := range apiResp.Data.ActiveTargets { - key := cacheKey(t.Labels.Get("job"), t.Labels.Get("instance")) + for _, target := range apiResp.Data.ActiveTargets { + key := cacheKey(target.Labels.Get("job"), target.Labels.Get("instance")) // If the exact target already exists, reuse the same memory object. for _, prev := range c.targets[key] { - if labelsEqual(t.Labels, prev.Labels) { - t = prev + if labelsEqual(target.Labels, prev.Labels) { + target = prev break } } - repl[key] = append(repl[key], t) + repl[key] = append(repl[key], target) + } + + // If negative lookups still cannot be found in retrieved response, + // the negative lookups should be carried over to the new cache, so when + // Get is called, it won't aggressively call refresh() again. + for key, target := range c.targets { + if target != nil { + continue + } + if _, ok := repl[key]; !ok { + repl[key] = nil + } } c.targets = repl diff --git a/targets/cache_test.go b/targets/cache_test.go index 633556ba..9617420c 100644 --- a/targets/cache_test.go +++ b/targets/cache_test.go @@ -205,6 +205,73 @@ func TestTargetCache_Success(t *testing.T) { } } +func TestTargetCache_EmptyEntry(t *testing.T) { + var handler func() []*Target + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var resp apiResponse + resp.Status = "success" + resp.Data.ActiveTargets = handler() + if err := json.NewEncoder(w).Encode(resp); err != nil { + t.Fatal(err) + } + })) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + u, err := url.Parse(ts.URL) + if err != nil { + t.Fatal(err) + } + + c := NewCache(nil, nil, u) + // Initialize cache with negative-cached target. + c.targets[cacheKey("job1", "instance-not-exists")] = nil + + // No target in initial response. + handler = func() []*Target { + return []*Target{} + } + c.Get(ctx, labels.FromStrings("__name__", "metric1", "job", "job1", "instance", "instance1")) + + // Empty entry should be kept in cache. + val, ok := c.targets[cacheKey("job1", "instance-not-exists")] + if !ok { + t.Fatalf("Negative cache should be kept.") + } + if val != nil { + t.Fatalf("Unexpected value job1/instance-not-exists: %v", val) + } + + // Create a new empty entry by querying job/instance pair not available. + c.Get(ctx, labels.FromStrings("__name__", "metric2", "job", "job2", "instance", "instance-not-exists")) + val, ok = c.targets[cacheKey("job2", "instance-not-exists")] + if !ok { + t.Fatalf("Negative cache should be kept.") + } + if val != nil { + t.Fatalf("Unexpected value job2/instance-not-exists: %v", val) + } + + // Add a new instance into response, which will trigger replacing cache. + handler = func() []*Target { + return []*Target{ + {Labels: labels.FromStrings("job", "job1", "instance", "instance1")}, + } + } + c.Get(ctx, labels.FromStrings("__name__", "metric1", "job", "job1", "instance", "instance1")) + + // Empty entry created should be kept in cache. + val, ok = c.targets[cacheKey("job2", "instance-not-exists")] + if !ok { + t.Fatalf("Negative cache should be kept.") + } + if val != nil { + t.Fatalf("Unexpected value job2/instance-not-exists: %v", val) + } +} + func TestDropTargetLabels(t *testing.T) { cases := []struct { series, target, result labels.Labels