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

* Allow 1w in the period duration.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Add support for 1w, 1y to table manager retention

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Update docs and changelog

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Added model.Duration support to doc generator

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Fix infinite recursion in UnmarshalYAML.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Walk back integration config changes to support old versions

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Final nits

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

Co-authored-by: Marco Pracucci <marco@pracucci.com>

Author: gouthamve

CHANGELOG.md

@@ -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
 

docs/configuration/config-file-reference.md

@@ -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)

docs/configuration/config-file-reference.template

@@ -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)

docs/configuration/schema-config-reference.md

@@ -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>
@@ -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
@@ -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
@@ -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
@@ -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

docs/configuration/single-process-config.md

@@ -64,7 +64,7 @@ schema:
     schema: v10
     index:
       prefix: index_
-      period: 168h
+      period: 1w 
 
 storage:
   boltdb:

docs/configuration/single-process-config.yaml

@@ -61,7 +61,7 @@ schema:
     schema: v10
     index:
       prefix: index_
-      period: 168h
+      period: 1w 
 
 storage:
   boltdb:

integration/configs.go

@@ -27,10 +27,10 @@ const (
   schema: v9
   index:
     prefix: cortex_
-    period: 168h0m0s
+    period: 168h 
   chunks:
     prefix: cortex_chunks_
-    period: 168h0m0s
+    period: 168h 
 `
 
 	cortexAlertmanagerUserConfigYaml = `route:
@@ -109,7 +109,7 @@ storage:
 
 table_manager:
   dynamodb_poll_interval: 1m
-  retention_period:       168h
+  retention_period:       168h 
 
 schema:
 {{.SchemaConfig}}

pkg/chunk/schema_config.go

@@ -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.

pkg/chunk/schema_config_test.go

@@ -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) {
@@ -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))
+}

pkg/chunk/table_manager.go

@@ -90,7 +90,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"`
@@ -102,6 +104,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"`
@@ -123,7 +160,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).")