From cda26a5728c89ae9f8a4bb960a1b745e4ae830d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Tue, 3 Nov 2020 09:11:54 +0100 Subject: [PATCH 01/12] Prune in-memory blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index 18b0e89208a..d01facdf46f 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -382,6 +382,11 @@ func (rw *readerWriter) pollBlocklist() { if err != nil { metricBlocklistErrors.WithLabelValues(tenantID).Inc() level.Error(rw.logger).Log("msg", "error polling blocklist", "tenantID", tenantID, "err", err) + rw.blockListsMtx.Lock() + delete(rw.blockLists, tenantID) + delete(rw.compactedBlockLists, tenantID) + rw.blockListsMtx.Unlock() + level.Error(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) } interfaceSlice := make([]interface{}, 0, len(blockIDs)) From e5c989d427587a2fb9d763dda5b9f3e60fabafd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Tue, 3 Nov 2020 09:36:36 +0100 Subject: [PATCH 02/12] Add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2384f4dc52c..6112376b6c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,3 +7,4 @@ * [ENHANCEMENT] Add command line flags for s3 credentials. [#308](https://github.com/grafana/tempo/pull/308) * [BUGFIX] Increase Prometheus `notfound` metric on tempo-vulture. [#301](https://github.com/grafana/tempo/pull/301) * [BUGFIX] Return 404 if searching for a tenant id that does not exist in the backend. [#321](https://github.com/grafana/tempo/pull/321) +* [BUGFIX] Prune in-memory blocks from missing tenants. [#314](https://github.com/grafana/tempo/pull/314) From 4525eaf75cf5fa66e56f7956c6c78e2957a7ce8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Wed, 4 Nov 2020 18:58:49 +0100 Subject: [PATCH 03/12] Feedback from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index d01facdf46f..916d088bb33 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -382,11 +382,13 @@ func (rw *readerWriter) pollBlocklist() { if err != nil { metricBlocklistErrors.WithLabelValues(tenantID).Inc() level.Error(rw.logger).Log("msg", "error polling blocklist", "tenantID", tenantID, "err", err) + } + if len(blockIDs) == 0 { rw.blockListsMtx.Lock() delete(rw.blockLists, tenantID) delete(rw.compactedBlockLists, tenantID) rw.blockListsMtx.Unlock() - level.Error(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) + level.Info(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) } interfaceSlice := make([]interface{}, 0, len(blockIDs)) From c138335ea7c504c9ef169d403c9a9bc218e87c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Fri, 6 Nov 2020 11:26:55 +0100 Subject: [PATCH 04/12] Add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb_test.go | 52 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tempodb/tempodb_test.go b/tempodb/tempodb_test.go index b4bee56f6b3..caacd904133 100644 --- a/tempodb/tempodb_test.go +++ b/tempodb/tempodb_test.go @@ -175,6 +175,58 @@ func TestRetention(t *testing.T) { checkBlocklists(t, blockID, 0, 0, rw) } +func TestBlockCleanup(t *testing.T) { + tempDir, err := ioutil.TempDir("/tmp", "") + defer os.RemoveAll(tempDir) + assert.NoError(t, err, "unexpected error creating temp dir") + + r, w, c, err := New(&Config{ + Backend: "local", + Local: &local.Config{ + Path: path.Join(tempDir, "traces"), + }, + WAL: &wal.Config{ + Filepath: path.Join(tempDir, "wal"), + IndexDownsample: 17, + BloomFP: .01, + }, + BlocklistPoll: 0, + }, log.NewNopLogger()) + assert.NoError(t, err) + + c.EnableCompaction(&CompactorConfig{ + ChunkSizeBytes: 10, + MaxCompactionRange: time.Hour, + BlockRetention: 0, + CompactedBlockRetention: 0, + }, &mockSharder{}) + + blockID := uuid.New() + + wal := w.WAL() + assert.NoError(t, err) + + head, err := wal.NewBlock(blockID, testTenantID) + assert.NoError(t, err) + + complete, err := head.Complete(wal, &mockSharder{}) + assert.NoError(t, err) + blockID = complete.BlockMeta().BlockID + + err = w.WriteBlock(context.Background(), complete) + assert.NoError(t, err) + + rw := r.(*readerWriter) + + os.RemoveAll(tempDir + "/traces/" + testTenantID) + + // poll + rw.pollBlocklist() + + assert.Len(t, rw.blockLists[testTenantID], 0) + assert.Len(t, rw.compactedBlockLists[testTenantID], 0) +} + func checkBlocklists(t *testing.T, expectedID uuid.UUID, expectedB int, expectedCB int, rw *readerWriter) { rw.pollBlocklist() From cc1e0f5c0cd7121dcba47252f5a963393d9992ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sun, 8 Nov 2020 11:07:35 +0100 Subject: [PATCH 05/12] Improve tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tempodb/tempodb_test.go b/tempodb/tempodb_test.go index caacd904133..9bbb00270b0 100644 --- a/tempodb/tempodb_test.go +++ b/tempodb/tempodb_test.go @@ -218,13 +218,18 @@ func TestBlockCleanup(t *testing.T) { rw := r.(*readerWriter) + // poll + rw.pollBlocklist() + + assert.Len(t, rw.blockLists[testTenantID], 1) + os.RemoveAll(tempDir + "/traces/" + testTenantID) // poll rw.pollBlocklist() - assert.Len(t, rw.blockLists[testTenantID], 0) - assert.Len(t, rw.compactedBlockLists[testTenantID], 0) + _, ok := rw.blockLists[testTenantID] + assert.False(t, ok) } func checkBlocklists(t *testing.T, expectedID uuid.UUID, expectedB int, expectedCB int, rw *readerWriter) { From ddb15bcceac02ca60681948c34071ddb8d88f289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sun, 8 Nov 2020 11:08:12 +0100 Subject: [PATCH 06/12] Remove missing tenant blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index 916d088bb33..a22dfc56e82 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -377,19 +377,35 @@ func (rw *readerWriter) pollBlocklist() { level.Error(rw.logger).Log("msg", "error retrieving tenants while polling blocklist", "err", err) } + tenantSet := make(map[string]bool) for _, tenantID := range tenants { - blockIDs, err := rw.r.Blocks(ctx, tenantID) - if err != nil { - metricBlocklistErrors.WithLabelValues(tenantID).Inc() - level.Error(rw.logger).Log("msg", "error polling blocklist", "tenantID", tenantID, "err", err) - } - if len(blockIDs) == 0 { + tenantSet[tenantID] = true + } + + for tenantID := range rw.blockLists { + if _, present := tenantSet[tenantID]; !present { rw.blockListsMtx.Lock() delete(rw.blockLists, tenantID) - delete(rw.compactedBlockLists, tenantID) rw.blockListsMtx.Unlock() level.Info(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) } + } + + for tenantID := range rw.compactedBlockLists { + if _, present := tenantSet[tenantID]; !present { + rw.blockListsMtx.Lock() + delete(rw.compactedBlockLists, tenantID) + rw.blockListsMtx.Unlock() + level.Info(rw.logger).Log("msg", "deleted in-memory compacted blocklists", "tenantID", tenantID) + } + } + + for _, tenantID := range tenants { + blockIDs, err := rw.r.Blocks(ctx, tenantID) + if err != nil { + metricBlocklistErrors.WithLabelValues(tenantID).Inc() + level.Error(rw.logger).Log("msg", "error polling blocklist", "tenantID", tenantID, "err", err) + } interfaceSlice := make([]interface{}, 0, len(blockIDs)) for _, id := range blockIDs { @@ -430,6 +446,12 @@ func (rw *readerWriter) pollBlocklist() { return nil, nil }) + if len(compactedBlocklist) == 0 { + rw.blockListsMtx.Lock() + delete(rw.compactedBlockLists, tenantID) + rw.blockListsMtx.Unlock() + level.Info(rw.logger).Log("msg", "deleted in-memory compacted blocklists", "tenantID", tenantID) + } if err != nil { metricBlocklistErrors.WithLabelValues(tenantID).Inc() level.Error(rw.logger).Log("msg", "run blocklist jobs", "tenantID", tenantID, "err", err) From c63c33f65013dac60a51c1b81286a3dfe98ead84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sun, 8 Nov 2020 11:14:39 +0100 Subject: [PATCH 07/12] Remove old code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index a22dfc56e82..b534f02fdd6 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -446,12 +446,6 @@ func (rw *readerWriter) pollBlocklist() { return nil, nil }) - if len(compactedBlocklist) == 0 { - rw.blockListsMtx.Lock() - delete(rw.compactedBlockLists, tenantID) - rw.blockListsMtx.Unlock() - level.Info(rw.logger).Log("msg", "deleted in-memory compacted blocklists", "tenantID", tenantID) - } if err != nil { metricBlocklistErrors.WithLabelValues(tenantID).Inc() level.Error(rw.logger).Log("msg", "run blocklist jobs", "tenantID", tenantID, "err", err) From 1a9c5574d9a2f1aa9bd25eb0da7ad563e80c2316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sun, 8 Nov 2020 11:21:03 +0100 Subject: [PATCH 08/12] Make CI happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tempodb/tempodb_test.go b/tempodb/tempodb_test.go index 9bbb00270b0..7cfb59a4fbc 100644 --- a/tempodb/tempodb_test.go +++ b/tempodb/tempodb_test.go @@ -211,7 +211,6 @@ func TestBlockCleanup(t *testing.T) { complete, err := head.Complete(wal, &mockSharder{}) assert.NoError(t, err) - blockID = complete.BlockMeta().BlockID err = w.WriteBlock(context.Background(), complete) assert.NoError(t, err) From 230a1fd82b873ff65d2e99d1e6cd04f7734baf6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sun, 8 Nov 2020 11:28:27 +0100 Subject: [PATCH 09/12] Move code to a method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 48 +++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index b534f02fdd6..7cb8675eb4d 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -377,28 +377,7 @@ func (rw *readerWriter) pollBlocklist() { level.Error(rw.logger).Log("msg", "error retrieving tenants while polling blocklist", "err", err) } - tenantSet := make(map[string]bool) - for _, tenantID := range tenants { - tenantSet[tenantID] = true - } - - for tenantID := range rw.blockLists { - if _, present := tenantSet[tenantID]; !present { - rw.blockListsMtx.Lock() - delete(rw.blockLists, tenantID) - rw.blockListsMtx.Unlock() - level.Info(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) - } - } - - for tenantID := range rw.compactedBlockLists { - if _, present := tenantSet[tenantID]; !present { - rw.blockListsMtx.Lock() - delete(rw.compactedBlockLists, tenantID) - rw.blockListsMtx.Unlock() - level.Info(rw.logger).Log("msg", "deleted in-memory compacted blocklists", "tenantID", tenantID) - } - } + rw.cleanMissingTenants(tenants) for _, tenantID := range tenants { blockIDs, err := rw.r.Blocks(ctx, tenantID) @@ -567,3 +546,28 @@ func (rw *readerWriter) compactedBlocklist(tenantID string) []*encoding.Compacte return copiedBlocklist } + +func (rw *readerWriter) cleanMissingTenants(tenants []string) { + tenantSet := make(map[string]bool) + for _, tenantID := range tenants { + tenantSet[tenantID] = true + } + + for tenantID := range rw.blockLists { + if _, present := tenantSet[tenantID]; !present { + rw.blockListsMtx.Lock() + delete(rw.blockLists, tenantID) + rw.blockListsMtx.Unlock() + level.Info(rw.logger).Log("msg", "deleted in-memory blocklists", "tenantID", tenantID) + } + } + + for tenantID := range rw.compactedBlockLists { + if _, present := tenantSet[tenantID]; !present { + rw.blockListsMtx.Lock() + delete(rw.compactedBlockLists, tenantID) + rw.blockListsMtx.Unlock() + level.Info(rw.logger).Log("msg", "deleted in-memory compacted blocklists", "tenantID", tenantID) + } + } +} From 864f7ee184538cd5795e59cae59a987d6c0326ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Tue, 10 Nov 2020 09:15:36 +0100 Subject: [PATCH 10/12] From map[string]bool to map[string]struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tempodb/tempodb.go b/tempodb/tempodb.go index 7cb8675eb4d..912888cd674 100644 --- a/tempodb/tempodb.go +++ b/tempodb/tempodb.go @@ -548,9 +548,9 @@ func (rw *readerWriter) compactedBlocklist(tenantID string) []*encoding.Compacte } func (rw *readerWriter) cleanMissingTenants(tenants []string) { - tenantSet := make(map[string]bool) + tenantSet := make(map[string]struct{}) for _, tenantID := range tenants { - tenantSet[tenantID] = true + tenantSet[tenantID] = struct{}{} } for tenantID := range rw.blockLists { From ca15c6eb773520ca078dde72cb1306d8d978d9ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Tue, 10 Nov 2020 09:56:43 +0100 Subject: [PATCH 11/12] Add test for cleanMissingTenants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb_test.go | 51 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tempodb/tempodb_test.go b/tempodb/tempodb_test.go index 7cfb59a4fbc..5ddbc7a4fb7 100644 --- a/tempodb/tempodb_test.go +++ b/tempodb/tempodb_test.go @@ -231,6 +231,57 @@ func TestBlockCleanup(t *testing.T) { assert.False(t, ok) } +func TestCleanMissingTenants(t *testing.T) { + tests := []struct { + name string + tenants []string + blocklist map[string][]*encoding.BlockMeta + expected map[string][]*encoding.BlockMeta + }{ + { + name: "one missing tenant", + tenants: []string{"foo"}, + blocklist: map[string][]*encoding.BlockMeta{"foo": {{}}, "bar": {{}}}, + expected: map[string][]*encoding.BlockMeta{"foo": {{}}}, + }, + { + name: "no missing tenants", + tenants: []string{"foo", "bar"}, + blocklist: map[string][]*encoding.BlockMeta{"foo": {{}}, "bar": {{}}}, + expected: map[string][]*encoding.BlockMeta{"foo": {{}}, "bar": {{}}}, + }, + { + name: "all missing tenants", + tenants: []string{}, + blocklist: map[string][]*encoding.BlockMeta{"foo": {{}}, "bar": {{}}}, + expected: map[string][]*encoding.BlockMeta{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, _, _, err := New(&Config{ + Backend: "local", + Local: &local.Config{ + Path: path.Join("/tmp", "traces"), + }, + WAL: &wal.Config{ + Filepath: path.Join("/tmp", "wal"), + IndexDownsample: 17, + BloomFP: .01, + }, + BlocklistPoll: 0, + }, log.NewNopLogger()) + assert.NoError(t, err) + + rw := r.(*readerWriter) + + rw.blockLists = tt.blocklist + rw.cleanMissingTenants(tt.tenants) + assert.Equal(t, rw.blockLists, tt.expected) + }) + } +} + func checkBlocklists(t *testing.T, expectedID uuid.UUID, expectedB int, expectedCB int, rw *readerWriter) { rw.pollBlocklist() From bd82075511d2bf026d00e05edeb5c6550e9928bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Tue, 10 Nov 2020 10:00:11 +0100 Subject: [PATCH 12/12] Add enconding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- tempodb/tempodb_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tempodb/tempodb_test.go b/tempodb/tempodb_test.go index 5ddbc7a4fb7..eb771eeeac7 100644 --- a/tempodb/tempodb_test.go +++ b/tempodb/tempodb_test.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/tempo/pkg/tempopb" "github.com/grafana/tempo/pkg/util/test" "github.com/grafana/tempo/tempodb/backend/local" + "github.com/grafana/tempo/tempodb/encoding" "github.com/grafana/tempo/tempodb/wal" "github.com/stretchr/testify/assert" )