Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a flag to enable TSDB memorry snapshot on shutdown #5011

Conversation

wgliang
Copy link
Contributor

@wgliang wgliang commented Dec 2, 2022

Signed-off-by: wangguoliang iamwgliang@gmail.com

What this PR does:
Add a flag to enable TSDB memorry snapshot on shutdown.

FYI:https://ganeshvernekar.com/blog/prometheus-tsdb-snapshot-on-shutdown/#faster-restarts

By skipping the WAL replay entirely during graceful restart, we have seen anywhere between 50-80% reduction in restart time.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch 2 times, most recently from 9a9d626 to 978215a Compare December 2, 2022 03:49
pkg/storage/tsdb/config.go Outdated Show resolved Hide resolved
pkg/storage/tsdb/config.go Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch 2 times, most recently from dc073d5 to 2b28bd9 Compare December 2, 2022 08:26
CHANGELOG.md Outdated Show resolved Hide resolved
@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch from 2b28bd9 to 46136f4 Compare December 3, 2022 08:22
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
pkg/storage/tsdb/config.go Show resolved Hide resolved
@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch from 46136f4 to df1fb7c Compare December 4, 2022 00:40
@yeya24
Copy link
Collaborator

yeya24 commented Dec 4, 2022

@alanprot @friedrichg Please share your thoughts about this change. Should we make it a global configuration or a per tenant configuration?
My initial thought is that a per tenant level flag is easier for testing as an experimental feature.
But it is also okay for me to make it a global flag.

The criteria of when should we make a config per tenant is somewhat not very clear to me. But since some other TSDB configurations are also global so this one is probably fine to be global, too.

@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch from df1fb7c to 6203538 Compare December 4, 2022 08:24
Signed-off-by: wangguoliang <iamwgliang@gmail.com>
@wgliang wgliang force-pushed the feature/enable-tsdb-memory-snapshot-on-shutdown branch from 6203538 to da52a1f Compare December 4, 2022 08:26
@wgliang
Copy link
Contributor Author

wgliang commented Dec 5, 2022

The pr has been updated, are there any other comments?

Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and addressing the feedback!

Copy link
Collaborator

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@yeya24 yeya24 merged commit 061b348 into cortexproject:master Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants