From 8fb0a9ab44c0ee477a18d0f9fa42d8fafa925ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Fri, 30 Oct 2020 10:53:07 +0100 Subject: [PATCH 1/3] Add warning for suspect configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- cmd/tempo/main.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cmd/tempo/main.go b/cmd/tempo/main.go index 4670e22034e..801e85c028b 100644 --- a/cmd/tempo/main.go +++ b/cmd/tempo/main.go @@ -76,6 +76,17 @@ func main() { // Allocate a block of memory to alter GC behaviour. See https://github.com/golang/go/issues/23044 ballast := make([]byte, *ballastMBs*1024*1024) + // Warn the user for suspect configurations + if config.Ingester.CompleteBlockTimeout < config.StorageConfig.Trace.MaintenanceCycle { + level.Warn(util.Logger).Log("msg", "ingester.CompleteBlockTimeout < storage.trace.maintenance-cycle", + "explan", "You may receive 404s between the time the ingesters have flushed a trace and the querier is aware of the new block") + } + + if config.Compactor.Compactor.BlockRetention < config.StorageConfig.Trace.MaintenanceCycle { + level.Warn(util.Logger).Log("msg", "compactor.CompactedBlockRetention < storage.trace.maintenance-cycle", + "explan", "Queriers and Compactors may attempt to read a block that no longer exists") + } + // Start Tempo t, err := app.New(*config) if err != nil { From 14a93a4405fb506c8f58b45b55ad9f3515d3210c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Fri, 30 Oct 2020 10:56:12 +0100 Subject: [PATCH 2/3] Add to 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf8ee984bb0..bac33d356c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ## master / unreleased * [CHANGE] Bloom filters are now sharded to reduce size and improve caching, as blocks grow. This is a **breaking change** and all data stored before this change will **not** be queryable. [192](https://github.com/grafana/tempo/pull/192) -* [ENHANCEMENT] CI checks for vendored dependencies using `make vendor-check`. Update CONTRIBUTING.md to reflect the same before checking in files in a PR. [#274](https://github.com/grafana/tempo/pull/274) \ No newline at end of file +* [ENHANCEMENT] CI checks for vendored dependencies using `make vendor-check`. Update CONTRIBUTING.md to reflect the same before checking in files in a PR. [#274](https://github.com/grafana/tempo/pull/274) +* [ENHANCEMENT] Add warnings for suspect configs. [#294](https://github.com/grafana/tempo/pull/294) From 4ff5c60f59b2a093c6b4c91b1e852c03592a748c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gonz=C3=A1lez=20Lopes?= Date: Sat, 31 Oct 2020 16:49:55 +0100 Subject: [PATCH 3/3] Move config check to its own method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel González Lopes --- cmd/tempo/app/app.go | 13 +++++++++++++ cmd/tempo/main.go | 10 +--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cmd/tempo/app/app.go b/cmd/tempo/app/app.go index 5ff5402b95d..106cc0d1314 100644 --- a/cmd/tempo/app/app.go +++ b/cmd/tempo/app/app.go @@ -83,6 +83,19 @@ func (c *Config) RegisterFlagsAndApplyDefaults(prefix string, f *flag.FlagSet) { } +// CheckConfig checks if config values are suspect. +func (c *Config) CheckConfig() { + if c.Ingester.CompleteBlockTimeout < c.StorageConfig.Trace.MaintenanceCycle { + level.Warn(util.Logger).Log("msg", "3ingester.CompleteBlockTimeout < storage.trace.maintenance-cycle", + "explan", "You may receive 404s between the time the ingesters have flushed a trace and the querier is aware of the new block") + } + + if c.Compactor.Compactor.BlockRetention < c.StorageConfig.Trace.MaintenanceCycle { + level.Warn(util.Logger).Log("msg", "compactor.CompactedBlockRetention < storage.trace.maintenance-cycle", + "explan", "Queriers and Compactors may attempt to read a block that no longer exists") + } +} + // App is the root datastructure. type App struct { cfg Config diff --git a/cmd/tempo/main.go b/cmd/tempo/main.go index 801e85c028b..277d994be65 100644 --- a/cmd/tempo/main.go +++ b/cmd/tempo/main.go @@ -77,15 +77,7 @@ func main() { ballast := make([]byte, *ballastMBs*1024*1024) // Warn the user for suspect configurations - if config.Ingester.CompleteBlockTimeout < config.StorageConfig.Trace.MaintenanceCycle { - level.Warn(util.Logger).Log("msg", "ingester.CompleteBlockTimeout < storage.trace.maintenance-cycle", - "explan", "You may receive 404s between the time the ingesters have flushed a trace and the querier is aware of the new block") - } - - if config.Compactor.Compactor.BlockRetention < config.StorageConfig.Trace.MaintenanceCycle { - level.Warn(util.Logger).Log("msg", "compactor.CompactedBlockRetention < storage.trace.maintenance-cycle", - "explan", "Queriers and Compactors may attempt to read a block that no longer exists") - } + config.CheckConfig() // Start Tempo t, err := app.New(*config)