From 92b8c8e4b0776b38f70a2f49eee83e42faeb791d Mon Sep 17 00:00:00 2001 From: Jennie Pham <94262131+ie-pham@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:56:53 -0600 Subject: [PATCH] deprecating vparquet v1 (#3377) * deprecating vparquet v1 * update docs and tests * meow * remove line --- CHANGELOG.md | 1 + docs/sources/tempo/configuration/_index.md | 2 +- docs/sources/tempo/configuration/parquet.md | 4 +- tempodb/config_test.go | 85 ++++++++++++++++++++- tempodb/encoding/common/config.go | 4 + tempodb/encoding/versioned.go | 1 - tempodb/wal/wal.go | 5 ++ 7 files changed, 96 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51fdad81ca6..e9dc5d2c93a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ * [BUGFIX] Fix TLS when GRPC is enabled on HTTP [#3300](https://github.com/grafana/tempo/pull/3300) (@joe-elliott) * [BUGFIX] Correctly return 400 when max limit is requested on search. [#3340](https://github.com/grafana/tempo/pull/3340) (@joe-elliott) * [BUGFIX] Fix autocomplete filters sometimes returning erroneous results. [#3339](https://github.com/grafana/tempo/pull/3339) (@joe-elliott) +* [CHANGE] **Breaking Change** Deprecating vParquet v1 [#3377](https://github.com/grafana/tempo/pull/3377) (@ie-pham) ## v2.3.1 / 2023-11-28 diff --git a/docs/sources/tempo/configuration/_index.md b/docs/sources/tempo/configuration/_index.md index 19756b2e82c..29b169bbe52 100644 --- a/docs/sources/tempo/configuration/_index.md +++ b/docs/sources/tempo/configuration/_index.md @@ -1037,7 +1037,7 @@ storage: # block configuration block: - # block format version. options: v2, vParquet, vParquet2, vParquet3 + # block format version. options: v2, vParquet2, vParquet3 [version: | default = vParquet3] # bloom filter false positive rate. lower values create larger filters but fewer false positives diff --git a/docs/sources/tempo/configuration/parquet.md b/docs/sources/tempo/configuration/parquet.md index c0bb34ccf8b..db492bd7d07 100644 --- a/docs/sources/tempo/configuration/parquet.md +++ b/docs/sources/tempo/configuration/parquet.md @@ -32,14 +32,14 @@ You can still use the previous format `vParquet2`. To enable it, set the block version option to `vParquet2` in the Storage section of the configuration file. ```yaml -# block format version. options: v2, vParquet, vParquet2, vParquet3 +# block format version. options: v2, vParquet2, vParquet3 [version: vParquet2] ``` In some cases, you may choose to disable Parquet and use the old `v2` block format. Using the `v2` block format disables all forms of search, but also reduces resource consumption, and may be desired for a high-throughput cluster that does not need these capabilities. To make this change, set the block version option to `v2` in the Storage section of the configuration file. ```yaml -# block format version. options: v2, vParquet, vParquet2, vParquet3 +# block format version. options: v2, vParquet2, vParquet3 [version: v2] ``` diff --git a/tempodb/config_test.go b/tempodb/config_test.go index 042ca578712..bf064aefc58 100644 --- a/tempodb/config_test.go +++ b/tempodb/config_test.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/tempo/tempodb/encoding/common" "github.com/grafana/tempo/tempodb/wal" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -92,7 +93,7 @@ func TestValidateConfig(t *testing.T) { { cfg: &Config{ WAL: &wal.Config{ - Version: "vParquet", + Version: "vParquet2", }, Block: &common.BlockConfig{ IndexDownsampleBytes: 1, @@ -104,7 +105,7 @@ func TestValidateConfig(t *testing.T) { }, expectedConfig: &Config{ WAL: &wal.Config{ - Version: "vParquet", + Version: "vParquet2", }, Block: &common.BlockConfig{ IndexDownsampleBytes: 1, @@ -127,6 +128,86 @@ func TestValidateConfig(t *testing.T) { } } +func TestDeprecatedVersions(t *testing.T) { + errorMessage := "this version of vParquet has been deprecated, please use vParquet2 or higher" + tests := []struct { + cfg *Config + expectedConfig *Config + err string + }{ + // block version not copied to wal if populated + { + cfg: &Config{ + WAL: &wal.Config{ + Version: "vParquet2", + }, + Block: &common.BlockConfig{ + IndexDownsampleBytes: 1, + IndexPageSizeBytes: 1, + BloomFP: 0.01, + BloomShardSizeBytes: 1, + Version: "vParquet3", + }, + }, + expectedConfig: &Config{ + WAL: &wal.Config{ + Version: "vParquet2", + }, + Block: &common.BlockConfig{ + IndexDownsampleBytes: 1, + IndexPageSizeBytes: 1, + BloomFP: 0.01, + BloomShardSizeBytes: 1, + Version: "vParquet3", + }, + }, + }, + { + cfg: &Config{ + WAL: &wal.Config{ + Version: "vParquet", + }, + Block: &common.BlockConfig{ + IndexDownsampleBytes: 1, + IndexPageSizeBytes: 1, + BloomFP: 0.01, + BloomShardSizeBytes: 1, + Version: "v2", + }, + }, + err: errorMessage, + }, + { + cfg: &Config{ + WAL: &wal.Config{ + Version: "vParquet2", + }, + Block: &common.BlockConfig{ + IndexDownsampleBytes: 1, + IndexPageSizeBytes: 1, + BloomFP: 0.01, + BloomShardSizeBytes: 1, + Version: "vParquet", + }, + }, + err: errorMessage, + }, + } + + for _, test := range tests { + err := validateConfig(test.cfg) + if test.err == "" { + require.Equal(t, nil, err) + } else { + assert.Contains(t, err.Error(), test.err) + } + + if test.expectedConfig != nil { + require.Equal(t, test.expectedConfig, test.cfg) + } + } +} + func TestValidateCompactorConfig(t *testing.T) { compactorConfig := CompactorConfig{ MaxCompactionRange: 0, diff --git a/tempodb/encoding/common/config.go b/tempodb/encoding/common/config.go index 85a3ef9efe4..37639d9d3ce 100644 --- a/tempodb/encoding/common/config.go +++ b/tempodb/encoding/common/config.go @@ -65,5 +65,9 @@ func ValidateConfig(b *BlockConfig) error { return fmt.Errorf("positive value required for bloom-filter shard size") } + if b.Version == "vParquet" { + return fmt.Errorf("this version of vParquet has been deprecated, please use vParquet2 or higher") + } + return b.DedicatedColumns.Validate() } diff --git a/tempodb/encoding/versioned.go b/tempodb/encoding/versioned.go index 40236f07109..1ec583aea51 100644 --- a/tempodb/encoding/versioned.go +++ b/tempodb/encoding/versioned.go @@ -85,7 +85,6 @@ func LatestEncoding() VersionedEncoding { func AllEncodings() []VersionedEncoding { return []VersionedEncoding{ v2.Encoding{}, - vparquet.Encoding{}, vparquet2.Encoding{}, vparquet3.Encoding{}, } diff --git a/tempodb/wal/wal.go b/tempodb/wal/wal.go index 9421dbafcb6..36c2c8c767a 100644 --- a/tempodb/wal/wal.go +++ b/tempodb/wal/wal.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/tempo/tempodb/backend/local" "github.com/grafana/tempo/tempodb/encoding" "github.com/grafana/tempo/tempodb/encoding/common" + "github.com/grafana/tempo/tempodb/encoding/vparquet" ) const ( @@ -36,6 +37,10 @@ type Config struct { } func ValidateConfig(c *Config) error { + if c.Version == vparquet.VersionString { + return fmt.Errorf("this version of vParquet has been deprecated, please use vParquet2 or higher") + } + if _, err := encoding.FromVersion(c.Version); err != nil { return fmt.Errorf("failed to validate block version %s: %w", c.Version, err) }