Skip to content

Commit

Permalink
store-gateway: reduce logging for not finding eager loading snapshot (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
dimitarvdimitrov authored Sep 26, 2023
1 parent 3e9168f commit 6718a33
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 13 deletions.
41 changes: 28 additions & 13 deletions pkg/storegateway/indexheader/reader_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand Down
8 changes: 8 additions & 0 deletions pkg/storegateway/indexheader/reader_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 6718a33

Please sign in to comment.