Skip to content

Support 1w, 1y for retention and table period take 2 #2252

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

Merged
merged 7 commits into from
Mar 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* `-flusher.concurrent-flushes` for number of concurrent flushes.
* `-flusher.flush-op-timeout` is duration after which a flush should timeout.
* [ENHANCEMENT] Experimental TSDB: Add support for local `filesystem` backend. #2245
* [ENHANCEMENT] Allow 1w (where w denotes week) and 1y (where y denotes year) when setting table period and retention. #2252

## 0.7.0-rc.0 / 2020-03-09

Expand Down
2 changes: 1 addition & 1 deletion docs/configuration/config-file-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ To specify which configuration file to load, pass the `-config.file` flag at the

* `<boolean>`: a boolean that can take the values `true` or `false`
* `<int>`: any integer matching the regular expression `[1-9]+[0-9]*`
* `<duration>`: a duration matching the regular expression `[0-9]+(ns|us|µs|ms|[smh])`
* `<duration>`: a duration matching the regular expression `[0-9]+(ns|us|µs|ms|s|m|h|d|w|y)` where y = 365 days.
* `<string>`: a regular string
* `<url>`: an URL
* `<prefix>`: a CLI flag prefix based on the context (look at the parent configuration block to see which CLI flags prefix should be used)
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration/config-file-reference.template
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ To specify which configuration file to load, pass the `-config.file` flag at the

* `<boolean>`: a boolean that can take the values `true` or `false`
* `<int>`: any integer matching the regular expression `[1-9]+[0-9]*`
* `<duration>`: a duration matching the regular expression `[0-9]+(ns|us|µs|ms|[smh])`
* `<duration>`: a duration matching the regular expression `[0-9]+(ns|us|µs|ms|s|m|h|d|w|y)` where y = 365 days.
* `<string>`: a regular string
* `<url>`: an URL
* `<prefix>`: a CLI flag prefix based on the context (look at the parent configuration block to see which CLI flags prefix should be used)
Expand Down
22 changes: 11 additions & 11 deletions docs/configuration/schema-config-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Where `periodic_table_config` is
```
# The prefix to use for the tables.
prefix: <string>
# We typically run Cortex with new tables every week to keep the index size low and to make retention easier. This sets the period at which new tables are created and used. Typically 168h (1week).
# We typically run Cortex with new tables every week to keep the index size low and to make retention easier. This sets the period at which new tables are created and used. Typically 1w (1week).
period: <duration>
# The tags that can be set on the dynamo table.
tags: <map[string]string>
Expand All @@ -45,12 +45,12 @@ configs:
- from: "2020-03-01" # Or typically a week before the Cortex cluster was created.
schema: v9
index:
period: 168h
period: 1w
prefix: cortex_index_
# Chunks section is optional and required only if you're not using a
# separate object store.
chunks:
period: 168h
period: 1w
prefix: cortex_chunks
store: aws-dynamo/bigtable-hashed/cassandra/boltdb
object_store: <above options>/s3/gcs/azure/filesystem
Expand All @@ -66,21 +66,21 @@ configs:
- from: "2018-08-23"
schema: v9
chunks:
period: 168h0m0s
period: 1w
prefix: dev_chunks_
index:
period: 168h0m0s
period: 1w
prefix: dev_index_
store: gcp-columnkey

# Starting 2018-02-13 we moved from BigTable to GCS for storing the chunks.
- from: "2019-02-13"
schema: v9
chunks:
period: 168h
period: 1w
prefix: dev_chunks_
index:
period: 168h
period: 1w
prefix: dev_index_
object_store: gcs
store: gcp-columnkey
Expand All @@ -90,10 +90,10 @@ configs:
- from: "2019-02-24"
schema: v9
chunks:
period: 168h
period: 1w
prefix: dev_chunks_
index:
period: 168h
period: 1w
prefix: dev_index_
object_store: gcs
store: bigtable-hashed
Expand All @@ -102,10 +102,10 @@ configs:
- from: "2019-03-05"
schema: v10
chunks:
period: 168h
period: 1w
prefix: dev_chunks_
index:
period: 168h
period: 1w
prefix: dev_index_
object_store: gcs
store: bigtable-hashed
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration/single-process-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ schema:
schema: v10
index:
prefix: index_
period: 168h
period: 1w

storage:
boltdb:
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration/single-process-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ schema:
schema: v10
index:
prefix: index_
period: 168h
period: 1w

storage:
boltdb:
Expand Down
6 changes: 3 additions & 3 deletions integration/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ const (
schema: v9
index:
prefix: cortex_
period: 168h0m0s
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed this when I changed 1w to 168h when I wanted older versions to run in integration tests: d5f8f81

PR to update changelog here: #2265

period: 168h
chunks:
prefix: cortex_chunks_
period: 168h0m0s
period: 168h
`

cortexAlertmanagerUserConfigYaml = `route:
Expand Down Expand Up @@ -102,7 +102,7 @@ storage:

table_manager:
dynamodb_poll_interval: 1m
retention_period: 168h
retention_period: 168h

schema:
{{.SchemaConfig}}
Expand Down
39 changes: 36 additions & 3 deletions pkg/chunk/schema_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,42 @@ func (cfg *PeriodConfig) dailyBuckets(from, through model.Time, userID string) [

// PeriodicTableConfig is configuration for a set of time-sharded tables.
type PeriodicTableConfig struct {
Prefix string `yaml:"prefix"`
Period time.Duration `yaml:"period,omitempty"`
Tags Tags `yaml:"tags,omitempty"`
Prefix string
Period time.Duration
Tags Tags
}

// UnmarshalYAML implements the yaml.Unmarshaler interface.
func (cfg *PeriodicTableConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
g := struct {
Prefix string `yaml:"prefix"`
Period model.Duration `yaml:"period"`
Tags Tags `yaml:"tags"`
}{}
if err := unmarshal(&g); err != nil {
return err
}

cfg.Prefix = g.Prefix
cfg.Period = time.Duration(g.Period)
cfg.Tags = g.Tags

return nil
}

// MarshalYAML implements the yaml.Marshaler interface.
func (cfg PeriodicTableConfig) MarshalYAML() (interface{}, error) {
g := &struct {
Prefix string `yaml:"prefix"`
Period model.Duration `yaml:"period"`
Tags Tags `yaml:"tags"`
}{
Prefix: cfg.Prefix,
Period: model.Duration(cfg.Period),
Tags: cfg.Tags,
}

return g, nil
}

// AutoScalingConfig for DynamoDB tables.
Expand Down
28 changes: 28 additions & 0 deletions pkg/chunk/schema_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
yaml "gopkg.in/yaml.v2"
)

func TestHourlyBuckets(t *testing.T) {
Expand Down Expand Up @@ -422,3 +423,30 @@ func MustParseDayTime(s string) DayTime {
}
return DayTime{model.TimeFromUnix(t.Unix())}
}

func TestPeriodicTableConfigCustomUnmarshalling(t *testing.T) {
yamlFile := `prefix: cortex_
period: 1w
tags:
foo: bar
`

cfg := PeriodicTableConfig{}
err := yaml.Unmarshal([]byte(yamlFile), &cfg)
require.NoError(t, err)

expectedCfg := PeriodicTableConfig{
Prefix: "cortex_",
Period: 7 * 24 * time.Hour,
Tags: map[string]string{
"foo": "bar",
},
}

require.Equal(t, expectedCfg, cfg)

yamlGenerated, err := yaml.Marshal(&cfg)
require.NoError(t, err)

require.Equal(t, yamlFile, string(yamlGenerated))
}
41 changes: 39 additions & 2 deletions pkg/chunk/table_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ type TableManagerConfig struct {
RetentionDeletesEnabled bool `yaml:"retention_deletes_enabled"`

// How far back tables will be kept before they are deleted
RetentionPeriod time.Duration `yaml:"retention_period"`
RetentionPeriod time.Duration `yaml:"-"`
// This is so that we can accept 1w, 1y in the YAML.
RetentionPeriodModel model.Duration `yaml:"retention_period"`

// Period with which the table manager will poll for tables.
DynamoDBPollInterval time.Duration `yaml:"dynamodb_poll_interval"`
Expand All @@ -94,6 +96,41 @@ type TableManagerConfig struct {
ChunkTables ProvisionConfig `yaml:"chunk_tables_provisioning"`
}

// UnmarshalYAML implements the yaml.Unmarshaler interface. To support RetentionPeriod.
func (cfg *TableManagerConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {

// If we call unmarshal on TableManagerConfig, it will call UnmarshalYAML leading to infinite recursion.
// To make unmarshal fill the plain data struct rather than calling UnmarshalYAML
// again, we have to hide it using a type indirection.
type plain TableManagerConfig
if err := unmarshal((*plain)(cfg)); err != nil {
return err
}

if cfg.RetentionPeriodModel > 0 {
cfg.RetentionPeriod = time.Duration(cfg.RetentionPeriodModel)
}

return nil
}

// MarshalYAML implements the yaml.Marshaler interface. To support RetentionPeriod.
func (cfg *TableManagerConfig) MarshalYAML() (interface{}, error) {
cfg.RetentionPeriodModel = model.Duration(cfg.RetentionPeriod)
return cfg, nil
}

// Validate validates the config.
func (cfg *TableManagerConfig) Validate() error {
// We're setting this field because when using flags, you set the RetentionPeriodModel but not RetentionPeriod.
// TODO(gouthamve): Its a hack, but I can't think of any other way :/
if cfg.RetentionPeriodModel > 0 {
cfg.RetentionPeriod = time.Duration(cfg.RetentionPeriodModel)
}

return nil
}

// ProvisionConfig holds config for provisioning capacity (on DynamoDB)
type ProvisionConfig struct {
ProvisionedThroughputOnDemandMode bool `yaml:"provisioned_throughput_on_demand_mode"`
Expand All @@ -115,7 +152,7 @@ type ProvisionConfig struct {
func (cfg *TableManagerConfig) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.ThroughputUpdatesDisabled, "table-manager.throughput-updates-disabled", false, "If true, disable all changes to DB capacity")
f.BoolVar(&cfg.RetentionDeletesEnabled, "table-manager.retention-deletes-enabled", false, "If true, enables retention deletes of DB tables")
f.DurationVar(&cfg.RetentionPeriod, "table-manager.retention-period", 0, "Tables older than this retention period are deleted. Note: This setting is destructive to data!(default: 0, which disables deletion)")
f.Var(&cfg.RetentionPeriodModel, "table-manager.retention-period", "Tables older than this retention period are deleted. Note: This setting is destructive to data!(default: 0, which disables deletion)")
f.DurationVar(&cfg.DynamoDBPollInterval, "dynamodb.poll-interval", 2*time.Minute, "How frequently to poll DynamoDB to learn our capacity.")
f.DurationVar(&cfg.CreationGracePeriod, "dynamodb.periodic-table.grace-period", 10*time.Minute, "DynamoDB periodic tables grace period (duration which table will be created/deleted before/after it's needed).")

Expand Down
3 changes: 3 additions & 0 deletions pkg/cortex/cortex.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ func (c *Config) Validate(log log.Logger) error {
if err := c.QueryRange.Validate(log); err != nil {
return errors.Wrap(err, "invalid queryrange config")
}
if err := c.TableManager.Validate(); err != nil {
return errors.Wrap(err, "invalid tablemanager config")
}
return nil
}

Expand Down
17 changes: 17 additions & 0 deletions tools/doc-generator/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"unicode"

"github.com/pkg/errors"
"github.com/prometheus/common/model"
"github.com/weaveworks/common/logging"

"github.com/cortexproject/cortex/pkg/util/flagext"
Expand Down Expand Up @@ -348,6 +349,22 @@ func getCustomFieldEntry(field reflect.StructField, fieldValue reflect.Value, fl
fieldDefault: fieldFlag.DefValue,
}, nil
}
if field.Type == reflect.TypeOf(model.Duration(0)) {
fieldFlag, err := getFieldFlag(field, fieldValue, flags)
if err != nil {
return nil, err
}

return &configEntry{
kind: "field",
name: getFieldName(field),
required: isFieldRequired(field),
fieldFlag: fieldFlag.Name,
fieldDesc: fieldFlag.Usage,
fieldType: "duration",
fieldDefault: fieldFlag.DefValue,
}, nil
}

return nil, nil
}
Expand Down