-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
unhide disable-downsampling flag in compactor #1385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM!
8317a30
to
c8661ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 suggestions and LGTM! (:
cmd/thanos/compact.go
Outdated
@@ -99,10 +99,9 @@ func registerCompact(m map[string]setupFunc, app *kingpin.Application) { | |||
generateMissingIndexCacheFiles := cmd.Flag("index.generate-missing-cache-file", "If enabled, on startup compactor runs an on-off job that scans all the blocks to find all blocks with missing index cache file. It generates those if needed and upload."). | |||
Hidden().Default("false").Bool() | |||
|
|||
// TODO(bplotka): Remove this flag once https://github.com/thanos-io/thanos/issues/297 is fixed. | |||
disableDownsampling := cmd.Flag("debug.disable-downsampling", "Disables downsampling. This is not recommended "+ | |||
disableDownsampling := cmd.Flag("disable-downsampling", "Disables downsampling. This is not recommended "+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name it downsampling.disable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially we might want to have more downsampling
options some day (:
cmd/thanos/compact.go
Outdated
@@ -99,10 +99,9 @@ func registerCompact(m map[string]setupFunc, app *kingpin.Application) { | |||
generateMissingIndexCacheFiles := cmd.Flag("index.generate-missing-cache-file", "If enabled, on startup compactor runs an on-off job that scans all the blocks to find all blocks with missing index cache file. It generates those if needed and upload."). | |||
Hidden().Default("false").Bool() | |||
|
|||
// TODO(bplotka): Remove this flag once https://github.com/thanos-io/thanos/issues/297 is fixed. | |||
disableDownsampling := cmd.Flag("debug.disable-downsampling", "Disables downsampling. This is not recommended "+ | |||
disableDownsampling := cmd.Flag("disable-downsampling", "Disables downsampling. This is not recommended "+ | |||
"as querying long time ranges without non-downsampled data is not efficient and not useful (is not possible to render all for human eye)."). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"as querying long time ranges without non-downsampled data is not efficient and not useful (is not possible to render all for human eye)."). | |
"as querying long time ranges without non-downsampled data is not efficient and useful e.g it is not possible to render all samples for a human eye anyway"). |
c8661ab
to
8d9d78d
Compare
Signed-off-by: yeya24 <yb532204897@gmail.com>
8d9d78d
to
bdbab8f
Compare
Updated 😄 PTAL @bwplotka @GiedriusS |
Thank you! LGTM. |
Signed-off-by: yeya24 yb532204897@gmail.com
fix #1377
Do I have to remove the
debug
prefix of this flag?Changes
Verification