From f804cff51a3924c78fee440e6d6288187668aa60 Mon Sep 17 00:00:00 2001 From: javiermolinar Date: Thu, 25 Jul 2024 16:24:33 +0200 Subject: [PATCH 1/4] remove unused properties from wal config --- docs/sources/tempo/configuration/_index.md | 10 ------ docs/sources/tempo/configuration/manifest.md | 4 --- tempodb/wal/wal.go | 32 +++++--------------- tempodb/wal/wal_test.go | 27 +---------------- 4 files changed, 9 insertions(+), 64 deletions(-) diff --git a/docs/sources/tempo/configuration/_index.md b/docs/sources/tempo/configuration/_index.md index 148bb5294c9..88db3964cef 100644 --- a/docs/sources/tempo/configuration/_index.md +++ b/docs/sources/tempo/configuration/_index.md @@ -1433,16 +1433,6 @@ The storage WAL configuration block. # Example: "/var/tempo/wal [path: | default = ""] -# Where to store the completed wal files -# If not set (""), will join the `path` with "completed" to generate the effective path -# Example: "/var/tempo/wal/completed" -[completedfilepath: | default = join(.path, "/completed")] - -# Where to store the intermediate blocks while they are being appended to. -# Will always join the `path` with "blocks" to generate the effective path -# Example: "/var/tempo/wal/blocks" (ignored) -[blocksfilepath: | = join(.path, "/blocks")] - # WAL encoding/compression. # options: none, gzip, lz4-64k, lz4-256k, lz4-1M, lz4, snappy, zstd, s2 [v2_encoding: | default = "zstd" ] diff --git a/docs/sources/tempo/configuration/manifest.md b/docs/sources/tempo/configuration/manifest.md index 493eed2dae2..076346e27d9 100644 --- a/docs/sources/tempo/configuration/manifest.md +++ b/docs/sources/tempo/configuration/manifest.md @@ -581,8 +581,6 @@ metrics_generator: remote_write_add_org_id_header: true traces_storage: path: "" - completedfilepath: "" - blocksfilepath: "" v2_encoding: none search_encoding: none ingestion_time_range_slack: 0s @@ -597,8 +595,6 @@ storage: queue_depth: 20000 wal: path: /var/tempo/wal - completedfilepath: /var/tempo/wal/completed - blocksfilepath: /var/tempo/wal/blocks v2_encoding: snappy search_encoding: none ingestion_time_range_slack: 2m0s diff --git a/tempodb/wal/wal.go b/tempodb/wal/wal.go index bba29a82f1d..d18a535a2d3 100644 --- a/tempodb/wal/wal.go +++ b/tempodb/wal/wal.go @@ -25,13 +25,11 @@ type WAL struct { } type Config struct { - Filepath string `yaml:"path"` - CompletedFilepath string - BlocksFilepath string - Encoding backend.Encoding `yaml:"v2_encoding"` - SearchEncoding backend.Encoding `yaml:"search_encoding"` - IngestionSlack time.Duration `yaml:"ingestion_time_range_slack"` - Version string `yaml:"version,omitempty"` + Filepath string `yaml:"path"` + Encoding backend.Encoding `yaml:"v2_encoding"` + SearchEncoding backend.Encoding `yaml:"search_encoding"` + IngestionSlack time.Duration `yaml:"ingestion_time_range_slack"` + Version string `yaml:"version,omitempty"` } func ValidateConfig(c *Config) error { @@ -53,29 +51,15 @@ func New(c *Config) (*WAL, error) { return nil, err } - // The /completed/ folder is now obsolete and no new data is written, - // but it needs to be cleared out one last time for any files left - // from a previous version. - if c.CompletedFilepath == "" { - completedFilepath := filepath.Join(c.Filepath, completedDir) - err = os.RemoveAll(completedFilepath) - if err != nil { - return nil, err - } - - c.CompletedFilepath = completedFilepath - } - // Setup local backend in /blocks/ - p := filepath.Join(c.Filepath, blocksDir) - err = os.MkdirAll(p, os.ModePerm) + blocksFolderPath := filepath.Join(c.Filepath, blocksDir) + err = os.MkdirAll(blocksFolderPath, os.ModePerm) if err != nil { return nil, err } - c.BlocksFilepath = p l, err := local.NewBackend(&local.Config{ - Path: p, + Path: blocksFolderPath, }) if err != nil { return nil, err diff --git a/tempodb/wal/wal_test.go b/tempodb/wal/wal_test.go index b43afbdff77..480803f4af1 100644 --- a/tempodb/wal/wal_test.go +++ b/tempodb/wal/wal_test.go @@ -9,7 +9,6 @@ import ( "io" "math/rand" "os" - "path" "path/filepath" "testing" "time" @@ -29,30 +28,6 @@ import ( "github.com/grafana/tempo/tempodb/encoding/common" ) -const ( - testTenantID = "fake" -) - -func TestCompletedDirIsRemoved(t *testing.T) { - // Create /completed/testfile and verify it is removed. - tempDir := t.TempDir() - - err := os.MkdirAll(path.Join(tempDir, completedDir), os.ModePerm) - require.NoError(t, err, "unexpected error creating completedDir") - - _, err = os.Create(path.Join(tempDir, completedDir, "testfile")) - require.NoError(t, err, "unexpected error creating testfile") - - _, err = New(&Config{ - Filepath: tempDir, - Version: encoding.DefaultEncoding().Version(), - }) - require.NoError(t, err, "unexpected error creating temp wal") - - _, err = os.Stat(path.Join(tempDir, completedDir)) - require.Error(t, err, "completedDir should not exist") -} - func TestAppendBlockStartEnd(t *testing.T) { for _, e := range encoding.AllEncodings() { t.Run(e.Version(), func(t *testing.T) { @@ -488,7 +463,7 @@ func BenchmarkFindUnknownTraceID(b *testing.B) { for _, enc := range encoding.AllEncodings() { version := enc.Version() b.Run(version, func(b *testing.B) { - runWALBenchmark(b, version, 1, func(ids [][]byte, objs []*tempopb.Trace, block common.WALBlock) { + runWALBenchmark(b, version, 1, func(_ [][]byte, _ []*tempopb.Trace, block common.WALBlock) { for i := 0; i < b.N; i++ { _, err := block.FindTraceByID(context.Background(), common.ID{}, common.DefaultSearchOptions()) require.NoError(b, err) From ddf6b724aa6e7754c13406e2b20b823720c340e5 Mon Sep 17 00:00:00 2001 From: javiermolinar Date: Thu, 25 Jul 2024 16:25:52 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15c564f32bd..7f872e47521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ * [BUGFIX] Improved handling of complete blocks in localblocks processor after enabling flusing [#3805](https://github.com/grafana/tempo/pull/3805) (@mapno) * [BUGFIX] Handle out of boundaries spans kinds [#3861](https://github.com/grafana/tempo/pull/3861) (@javiermolinar) * [BUGFIX] Maintain previous tenant blocklist on tenant errors [#3860](https://github.com/grafana/tempo/pull/3860) (@zalegrala) - +* [BUGFIX] Remove unused properties from the WAL configuration [#3860](https://github.com/grafana/tempo/pull/3860) (@javiermolianr) ## v2.5.0 From 6b2106b2bc6ab572e214b0eea9ca0cef8b970d75 Mon Sep 17 00:00:00 2001 From: javiermolinar Date: Thu, 25 Jul 2024 16:29:38 +0200 Subject: [PATCH 3/4] added PR number to the changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f872e47521..ba2630f8334 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ * [BUGFIX] Improved handling of complete blocks in localblocks processor after enabling flusing [#3805](https://github.com/grafana/tempo/pull/3805) (@mapno) * [BUGFIX] Handle out of boundaries spans kinds [#3861](https://github.com/grafana/tempo/pull/3861) (@javiermolinar) * [BUGFIX] Maintain previous tenant blocklist on tenant errors [#3860](https://github.com/grafana/tempo/pull/3860) (@zalegrala) -* [BUGFIX] Remove unused properties from the WAL configuration [#3860](https://github.com/grafana/tempo/pull/3860) (@javiermolianr) +* [BUGFIX] Remove unused properties from the WAL configuration [#3911](https://github.com/grafana/tempo/pull/3911) (@javiermolianr) ## v2.5.0 From ee5098d85d1e69066d97180ec101fca80dcd3f97 Mon Sep 17 00:00:00 2001 From: javiermolinar Date: Fri, 26 Jul 2024 12:26:20 +0200 Subject: [PATCH 4/4] update changelog with breaking change --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba2630f8334..5fb9a4e55fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ * [BUGFIX] Improved handling of complete blocks in localblocks processor after enabling flusing [#3805](https://github.com/grafana/tempo/pull/3805) (@mapno) * [BUGFIX] Handle out of boundaries spans kinds [#3861](https://github.com/grafana/tempo/pull/3861) (@javiermolinar) * [BUGFIX] Maintain previous tenant blocklist on tenant errors [#3860](https://github.com/grafana/tempo/pull/3860) (@zalegrala) -* [BUGFIX] Remove unused properties from the WAL configuration [#3911](https://github.com/grafana/tempo/pull/3911) (@javiermolianr) +* [BUGFIX] **BREAKING CHANGE** Remove unused properties from the WAL configuration [#3911](https://github.com/grafana/tempo/pull/3911) (@javiermolianr) ## v2.5.0