-
Notifications
You must be signed in to change notification settings - Fork 2.2k
compact: Add compactor time validation #7293
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
base: main
Are you sure you want to change the base?
Changes from all commits
85111cc
c63b550
68f4399
334c761
3757a57
c43ea37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -228,6 +228,10 @@ func runCompact( | |||||
| } | ||||||
| }() | ||||||
|
|
||||||
| if err := validateFilterConf(conf.filterConf); err != nil { | ||||||
| return fmt.Errorf("invalid time filters: %s", err.Error()) | ||||||
| } | ||||||
|
|
||||||
| // While fetching blocks, we filter out blocks that were marked for deletion by using IgnoreDeletionMarkFilter. | ||||||
| // The delay of deleteDelay/2 is added to ensure we fetch blocks that are meant to be deleted but do not have a replacement yet. | ||||||
| // This is to make sure compactor will not accidentally perform compactions with gap instead. | ||||||
|
|
@@ -689,6 +693,27 @@ func runCompact( | |||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| func validateFilterConf(filterConf *store.FilterConfig) error { | ||||||
| if filterConf.MinTime.PrometheusTimestamp() > filterConf.MaxTime.PrometheusTimestamp() { | ||||||
| return errors.New("max time must be a time or duration after min time") | ||||||
| } | ||||||
|
|
||||||
| if err := checkIfDurInFuture(filterConf.MinTime.Dur); err != nil { | ||||||
| return fmt.Errorf("min time: %s", err.Error()) | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| func checkIfDurInFuture(dur *model.Duration) error { | ||||||
| if dur == nil { | ||||||
| return nil | ||||||
| } | ||||||
| if !strings.HasPrefix("-", dur.String()) { | ||||||
| return errors.New("duration cannot be in the future") | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| type compactConfig struct { | ||||||
| haltOnError bool | ||||||
| acceptMalformedIndex bool | ||||||
|
|
@@ -823,9 +848,9 @@ func (cc *compactConfig) registerFlag(cmd extkingpin.FlagClause) { | |||||
| Default("").EnumVar(&cc.hashFunc, "SHA256", "") | ||||||
|
|
||||||
| cc.filterConf = &store.FilterConfig{} | ||||||
| cmd.Flag("min-time", "Start of time range limit to compact. Thanos Compactor will compact only blocks, which happened later than this value. Option can be a constant time in RFC3339 format or time duration relative to current time, such as -1d or 2h45m. Valid duration units are ms, s, m, h, d, w, y."). | ||||||
| cmd.Flag("min-time", "Start of time range limit to compact. Thanos Compactor will only compact blocks whose max time is at a date more recent than this value. Option can be a constant time in RFC3339 format or time duration relative to current time, such as -1d or -2h45m. Durations must be negative, and valid units are ms, s, m, h, d, w, y."). | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Whose is an unnatural possessive for an inanimate object (a block). |
||||||
| Default("0000-01-01T00:00:00Z").SetValue(&cc.filterConf.MinTime) | ||||||
| cmd.Flag("max-time", "End of time range limit to compact. Thanos Compactor will compact only blocks, which happened earlier than this value. Option can be a constant time in RFC3339 format or time duration relative to current time, such as -1d or 2h45m. Valid duration units are ms, s, m, h, d, w, y."). | ||||||
| cmd.Flag("max-time", "End of time range limit to compact. Thanos Compactor will only compact blocks whose min time is at a date less recent this value. Option can be a constant time in RFC3339 format or time duration relative to current time, such as -1d or -2h45m. Valid units for duration are ms, s, m, h, d, w, y."). | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Same as above, and I also believe it has to be older than max time, if it's older than min-time, block might be beyond this value and thus you'd actually be compacting max-time + block size (which could be up to multiple days/weeks) |
||||||
| Default("9999-12-31T23:59:59Z").SetValue(&cc.filterConf.MaxTime) | ||||||
|
|
||||||
| cmd.Flag("web.disable", "Disable Block Viewer UI.").Default("false").BoolVar(&cc.disableWeb) | ||||||
|
|
||||||
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.
Is it valid to have them equal? If the compactor
MinTimeandMaxTimeare equal it will just be non-functional iirc.