From 6718a33a5671c8646a08a07cd1384060ffe12016 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Tue, 26 Sep 2023 11:22:10 +0200 Subject: [PATCH] store-gateway: reduce logging for not finding eager loading snapshot (#6135) The existing logging can be noisy when the upgrade happens. This is because the eager loading snapshot file isn't found. This emits a log line for trying to read the file and then a log line for trying to delete it. This can be unnecessarily worrying especially when there are many tenants. This PR removes logging when the file isn't found and doesn't attempt deleting it. It also removes the `tenant` log field since that's redundant with the user logger we're using. ``` ts=2023-09-25T19:57:41.797242382Z caller=reader_pool.go:104 level=warn user=123 msg="loading the list of index-headers from snapshot file failed; not eagerly loading index-headers for tenant" file=/data/tsdb/123/lazy-loaded.json err="open /data/tsdb/1054557/lazy-loaded.json: no such file or directory" tenant=123 ts=2023-09-25T19:57:41.79726468Z caller=reader_pool.go:110 level=warn user=123 msg="removing the lazy-loaded index-header snapshot failed" file=/data/tsdb/123/lazy-loaded.json err="remove /data/tsdb/1054557/lazy-loaded.json: no such file or directory" ``` Signed-off-by: Dimitar Dimitrov --- pkg/storegateway/indexheader/reader_pool.go | 41 +++++++++++++------ .../indexheader/reader_pool_test.go | 8 ++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/pkg/storegateway/indexheader/reader_pool.go b/pkg/storegateway/indexheader/reader_pool.go index b9fcb0a959..5dd632b79f 100644 --- a/pkg/storegateway/indexheader/reader_pool.go +++ b/pkg/storegateway/indexheader/reader_pool.go @@ -25,7 +25,7 @@ import ( "github.com/grafana/mimir/pkg/util/atomicfs" ) -var lazyLoadedHeadersListFileName = "lazy-loaded.json" +const lazyLoadedHeadersListFileName = "lazy-loaded.json" // ReaderPoolMetrics holds metrics tracked by ReaderPool. type ReaderPoolMetrics struct { @@ -97,18 +97,7 @@ func (l lazyLoadedHeadersSnapshot) persist(persistDir string) error { func NewReaderPool(logger log.Logger, indexHeaderConfig Config, lazyLoadingGate gate.Gate, metrics *ReaderPoolMetrics, lazyLoadedSnapshotConfig LazyLoadedHeadersSnapshotConfig) *ReaderPool { var snapshot *lazyLoadedHeadersSnapshot if indexHeaderConfig.LazyLoadingEnabled && indexHeaderConfig.EagerLoadingStartupEnabled { - lazyLoadedSnapshotFileName := filepath.Join(lazyLoadedSnapshotConfig.Path, lazyLoadedHeadersListFileName) - var err error - snapshot, err = loadLazyLoadedHeadersSnapshot(lazyLoadedSnapshotFileName) - if err != nil { - level.Warn(logger).Log("msg", "loading the list of index-headers from snapshot file failed; not eagerly loading index-headers for tenant", "file", lazyLoadedSnapshotFileName, "err", err, "tenant", lazyLoadedSnapshotConfig.UserID) - } - // We will remove the file regardless whether err is nil or not nil. - // In the case such as snapshot loading causing OOM, we will still - // remove the snapshot and lazy load after server is restarted. - if err := os.Remove(lazyLoadedSnapshotFileName); err != nil { - level.Warn(logger).Log("msg", "removing the lazy-loaded index-header snapshot failed", "file", lazyLoadedSnapshotFileName, "err", err) - } + snapshot = tryRestoreLazyLoadedHeadersSnapshot(logger, lazyLoadedSnapshotConfig) } p := newReaderPool(logger, indexHeaderConfig, lazyLoadingGate, metrics, snapshot) @@ -154,6 +143,32 @@ func NewReaderPool(logger log.Logger, indexHeaderConfig Config, lazyLoadingGate return p } +func tryRestoreLazyLoadedHeadersSnapshot(logger log.Logger, cfg LazyLoadedHeadersSnapshotConfig) *lazyLoadedHeadersSnapshot { + fileName := filepath.Join(cfg.Path, lazyLoadedHeadersListFileName) + snapshot, err := loadLazyLoadedHeadersSnapshot(fileName) + if os.IsNotExist(err) { + // We didn't find the snapshot. Could be because we crashed after restoring it last time + // or because the previous binary didn't support eager loading. + // Either way, we can continue without eagerly loading blocks. + // Since the file wasn't found, we also won't try to remove it. + return nil + } + if err != nil { + level.Warn(logger).Log( + "msg", "loading the list of index-headers from snapshot file failed; not eagerly loading index-headers for tenant", + "file", fileName, + "err", err, + ) + } + // We will remove the file regardless whether err is nil or not nil. + // In the case such as snapshot loading causing OOM, we will still + // remove the snapshot and lazy load after server is restarted. + if err := os.Remove(fileName); err != nil { + level.Warn(logger).Log("msg", "removing the lazy-loaded index-header snapshot failed", "file", fileName, "err", err) + } + return snapshot +} + // newReaderPool makes a new ReaderPool. func newReaderPool(logger log.Logger, indexHeaderConfig Config, lazyLoadingGate gate.Gate, metrics *ReaderPoolMetrics, lazyLoadedHeadersSnapshot *lazyLoadedHeadersSnapshot) *ReaderPool { return &ReaderPool{ diff --git a/pkg/storegateway/indexheader/reader_pool_test.go b/pkg/storegateway/indexheader/reader_pool_test.go index 2e16f124f3..b142f7cf61 100644 --- a/pkg/storegateway/indexheader/reader_pool_test.go +++ b/pkg/storegateway/indexheader/reader_pool_test.go @@ -96,6 +96,14 @@ func TestReaderPool_NewBinaryReader(t *testing.T) { } }, }, + "pre-shutdown loaded blocks snapshot doesn't exist and eager-loading is enabled": { + lazyReaderEnabled: true, + lazyReaderIdleTimeout: time.Minute, + eagerLoadReaderEnabled: true, + initialSync: true, + expectedLoadCountMetricBeforeLabelNamesCall: 0, // although eager loading is enabled, this test will not do eager loading because there is no snapshot file + expectedLoadCountMetricAfterLabelNamesCall: 1, + }, } ctx := context.Background()