Release 0.5.0 (#1963)

* Release 0.5.0-rc0

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* Fix changelog to point to 0.5.0-rc.0

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* Validate incoming labels for order and duplicate names. (#1964)

* Validate incoming labels for order and duplicate names.

Signed-off-by: Peter Štibraný <[email protected]>

* Document that cortex rejects requests with incorrectly ordered or duplicate labels.

Signed-off-by: Peter Štibraný <[email protected]>

* Ignore empty metric name.

{__name__=""} will be shown formatted as {__name__=""} instead of just empty string.

Signed-off-by: Peter Štibraný <[email protected]>

* As we rely on sorted labels, sort them before using them.

Signed-off-by: Peter Štibraný <[email protected]>

* Updated CHANGELOG.md to reflect latest changes to PR.

Signed-off-by: Peter Štibraný <[email protected]>

* Put back redundant aliases.

Signed-off-by: Peter Štibraný <[email protected]>

* wrap migration commands (#1980)

* wrap migration commands

Signed-off-by: Jacob Lisi <[email protected]>

* update changelog

Signed-off-by: Jacob Lisi <[email protected]>

* fix missing semicolon

Signed-off-by: Jacob Lisi <[email protected]>

* Fix typo to make lint pass.

Signed-off-by: Goutham Veeramachaneni <[email protected]>

Co-authored-by: Goutham Veeramachaneni <[email protected]>

* Call out breaking changes better.

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* Label this version as -rc1

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* We're abandoning the `0.5.0` release.

Signed-off-by: Goutham Veeramachaneni <[email protected]>

Co-authored-by: Peter Štibraný <[email protected]>
Co-authored-by: Jacob Lisi <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -2,8 +2,12 @@
 
 ## master / unreleased
 
+Note that the ruler flags need to be changed in this upgrade. You're moving from a single node ruler to something that might need to be sharded.
+If you are running with a high `-ruler.num-workers` and if you're not able to execute all your rules in `-ruler.evaluation-interval`, then you'll need to shard.
+Further, if you're using the configs service, we've upgraded the migration library and this requires some manual intervention. See full
+instructions below to upgrade your Postgres.
+
 * [CHANGE] The frontend component now does not cache results if it finds a `Cache-Control` header and if one of its values is `no-store`. #1974
-* [ENHANCEMENT] metric `cortex_ingester_flush_reasons` gets a new `reason` value: `Spread`, when `-ingester.spread-flushes` option is enabled.
 * [CHANGE] Flags changed with transition to upstream Prometheus rules manager:
   * `ruler.client-timeout` is now `ruler.configs.client-timeout` in order to match `ruler.configs.url`
   * `ruler.group-timeout`has been removed
@@ -15,6 +19,7 @@
 * [CHANGE] Deprecated `-distributor.limiter-reload-period` flag. #1766
 * [CHANGE] Ingesters now write only normalised tokens to the ring, although they can still read denormalised tokens used by other ingesters. `-ingester.normalise-tokens` is now deprecated, and ignored. If you want to switch back to using denormalised tokens, you need to downgrade to Cortex 0.4.0. Previous versions don't handle claiming tokens from normalised ingesters correctly. #1809
 * [CHANGE] Overrides mechanism has been renamed to "runtime config", and is now separate from limits. Runtime config is simply a file that is reloaded by Cortex every couple of seconds. Limits and now also multi KV use this mechanism.<br />New arguments were introduced: `-runtime-config.file` (defaults to empty) and `-runtime-config.reload-period` (defaults to 10 seconds), which replace previously used `-limits.per-user-override-config` and `-limits.per-user-override-period` options. Old options are still used if `-runtime-config.file` is not specified. This change is also reflected in YAML configuration, where old `limits.per_tenant_override_config` and `limits.per_tenant_override_period` fields are replaced with `runtime_config.file` and `runtime_config.period` respectively. #1749
+* [CHANGE] Cortex now rejects data with duplicate labels. Previously, such data was accepted, with duplicate labels removed with only one value left. #1964
 * [CHANGE] Changed the default value for `-distributor.ha-tracker.prefix` from `collectors/` to `ha-tracker/` in order to not clash with other keys (ie. ring) stored in the same key-value store. #1940
 * [FEATURE] The distributor can now drop labels from samples (similar to the removal of the replica label for HA ingestion) per user via the `distributor.drop-label` flag. #1726
 * [FEATURE] Added flag `debug.mutex-profile-fraction` to enable mutex profiling #1969
@@ -23,11 +28,13 @@
 * [FEATURE] Added readiness probe endpoint`/ready` to queriers. #1934
 * [FEATURE] EXPERIMENTAL: Added `/series` API endpoint support with TSDB blocks storage. #1830
 * [FEATURE] Added "multi" KV store that can interact with two other KV stores, primary one for all reads and writes, and secondary one, which only receives writes. Primary/secondary store can be modified in runtime via runtime-config mechanism (previously "overrides"). #1749
-* [ENHANCEMENT] Added `password` and `enable_tls` options to redis cache configuration. Enables usage of Microsoft Azure Cache for Redis service.
+* [ENHANCEMENT] metric `cortex_ingester_flush_reasons` gets a new `reason` value: `Spread`, when `-ingester.spread-flushes` option is enabled. #1978
+* [ENHANCEMENT] Added `password` and `enable_tls` options to redis cache configuration. Enables usage of Microsoft Azure Cache for Redis service. #1923
 * [ENHANCEMENT] Experimental TSDB: Open existing TSDB on startup to prevent ingester from becoming ready before it can accept writes. #1917
   * `--experimental.tsdb.max-tsdb-opening-concurrency-on-startup`
 * [BUGFIX] Fixed unnecessary CAS operations done by the HA tracker when the jitter is enabled. #1861
 * [BUGFIX] Fixed #1904 ingesters getting stuck in a LEAVING state after coming up from an ungraceful exit. #1921
+* [BUGFIX] Reduce memory usage when ingester Push() errors. #1922
 * [BUGFIX] TSDB: Fixed handling of out of order/bound samples in ingesters with the experimental TSDB blocks storage. #1864
 * [BUGFIX] TSDB: Fixed querying ingesters in `LEAVING` state with the experimental TSDB blocks storage. #1854
 * [BUGFIX] TSDB: Fixed error handling in the series to chunks conversion with the experimental TSDB blocks storage. #1837
@@ -36,6 +43,18 @@
 * [BUGFIX] TSDB: Fixed `cortex_ingester_queried_samples` and `cortex_ingester_queried_series` metrics when using block storage. #1981
 * [BUGFIX] TSDB: Fixed `cortex_ingester_memory_series` and `cortex_ingester_memory_users` metrics when using with the experimental TSDB blocks storage. #1982
 
+### Upgrading Postgres (if you're using configs service)
+
+Reference: https://github.com/golang-migrate/migrate/tree/master/database/postgres#upgrading-from-v1
+
+1. Install the migrate package cli tool: https://github.com/golang-migrate/migrate/tree/master/cmd/migrate#installation
+2. Drop the `schema_migrations` table: `DROP TABLE schema_migrations;`.
+2. Run the migrate command:
+
+```bash
+migrate  -path <absolute_path_to_cortex>/cmd/cortex/migrations -database postgres://localhost:5432/database force 2 
+```
+
 ## 0.4.0 / 2019-12-02
 
 * [CHANGE] The frontend component has been refactored to be easier to re-use. When upgrading the frontend, cache entries will be discarded and re-created with the new protobuf schema. #1734

RELEASE.md

@@ -12,7 +12,8 @@ Our goal is to provide a new minor release every 4 weeks. This is a new process
 | v0.2.0         | 2019-08-28                                 | Goutham Veeramachaneni (Github: @gouthamve) |
 | v0.3.0         | 2019-10-09                                 | Bryan Boreham (@bboreham)                   |
 | v0.4.0         | 2019-11-13                                 | Tom Wilkie (@tomwilkie)                     |
-| v0.5.0         | 2019-12-11                                 | **searching for volunteer**                 |
+| v0.5.0         | 2020-01-08                                 | _Abandoned_                                   |
+| v0.6.0         | 2020-01-20                                 | **searching for a volunteer**                   |
 
 ## Release shepherd responsibilities
 

cmd/cortex/migrations/002_immutable_configs.up.sql

@@ -2,6 +2,10 @@
 -- process (as exemplified in users/db/migrations/006 to 010), but currently
 -- there are no data in production and only one row in dev.
 
+-- https://github.com/mattes/migrate/tree/master/database/postgres#upgrading-from-v1
+-- Wrap all commands in BEGIN and COMMIT to accommodate upgrade
+BEGIN;
+
 -- The existing id, type columns are the id & type of the entity that owns the
 -- config.
 ALTER TABLE configs RENAME COLUMN id TO owner_id;
@@ -12,3 +16,5 @@ ALTER TABLE configs ADD COLUMN id SERIAL;
 
 ALTER TABLE configs DROP CONSTRAINT configs_pkey;
 ALTER TABLE configs ADD PRIMARY KEY (id, owner_id, owner_type, subsystem);
+
+COMMIT;

pkg/distributor/distributor.go

@@ -3,7 +3,6 @@ package distributor
 import (
 	"context"
 	"flag"
-	"fmt"
 	"net/http"
 	"sort"
 	"strings"
@@ -227,7 +226,7 @@ func (d *Distributor) Stop() {
 
 func (d *Distributor) tokenForLabels(userID string, labels []client.LabelAdapter) (uint32, error) {
 	if d.cfg.ShardByAllLabels {
-		return shardByAllLabels(userID, labels)
+		return shardByAllLabels(userID, labels), nil
 	}
 
 	metricName, err := extract.MetricNameFromLabelAdapters(labels)
@@ -244,19 +243,15 @@ func shardByMetricName(userID string, metricName string) uint32 {
 	return h
 }
 
-func shardByAllLabels(userID string, labels []client.LabelAdapter) (uint32, error) {
+// This function generates different values for different order of same labels.
+func shardByAllLabels(userID string, labels []client.LabelAdapter) uint32 {
 	h := client.HashNew32()
 	h = client.HashAdd32(h, userID)
-	var lastLabelName string
 	for _, label := range labels {
-		if strings.Compare(lastLabelName, label.Name) >= 0 {
-			return 0, fmt.Errorf("labels not sorted")
-		}
 		h = client.HashAdd32(h, label.Name)
 		h = client.HashAdd32(h, label.Value)
-		lastLabelName = label.Name
 	}
-	return h, nil
+	return h
 }
 
 // Remove the label labelname from a slice of LabelPairs if it exists.
@@ -375,6 +370,13 @@ func (d *Distributor) Push(ctx context.Context, req *client.WriteRequest) (*clie
 			continue
 		}
 
+		// We rely on sorted labels in different places:
+		// 1) When computing token for labels, and sharding by all labels. Here different order of labels returns
+		// different tokens, which is bad.
+		// 2) In validation code, when checking for duplicate label names. As duplicate label names are rejected
+		// later in the validation phase, we ignore them here.
+		sortLabelsIfNeeded(ts.Labels)
+
 		// Generate the sharding token based on the series labels without the HA replica
 		// label and dropped labels (if any)
 		key, err := d.tokenForLabels(userID, ts.Labels)
@@ -450,6 +452,28 @@ func (d *Distributor) Push(ctx context.Context, req *client.WriteRequest) (*clie
 	return &client.WriteResponse{}, lastPartialErr
 }
 
+func sortLabelsIfNeeded(labels []client.LabelAdapter) {
+	// no need to run sort.Slice, if labels are already sorted, which is most of the time.
+	// we can avoid extra memory allocations (mostly interface-related) this way.
+	sorted := true
+	last := ""
+	for _, l := range labels {
+		if strings.Compare(last, l.Name) > 0 {
+			sorted = false
+			break
+		}
+		last = l.Name
+	}
+
+	if sorted {
+		return
+	}
+
+	sort.Slice(labels, func(i, j int) bool {
+		return strings.Compare(labels[i].Name, labels[j].Name) < 0
+	})
+}
+
 func (d *Distributor) sendSamples(ctx context.Context, ingester ring.IngesterDesc, timeseries []client.PreallocTimeseries) error {
 	h, err := d.ingesterPool.GetClientFor(ingester.Addr)
 	if err != nil {

pkg/distributor/distributor_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"sort"
 	"strconv"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -905,7 +906,7 @@ func (i *mockIngester) Push(ctx context.Context, req *client.WriteRequest, opts
 
 	for j := range req.Timeseries {
 		series := req.Timeseries[j]
-		hash, _ := shardByAllLabels(orgid, series.Labels)
+		hash := shardByAllLabels(orgid, series.Labels)
 		existing, ok := i.timeseries[hash]
 		if !ok {
 			// Make a copy because the request Timeseries are reused
@@ -1183,22 +1184,46 @@ func TestRemoveReplicaLabel(t *testing.T) {
 	}
 }
 
-func TestShardByAllLabelsChecksForSortedLabelNames(t *testing.T) {
-	val, err := shardByAllLabels("test", []client.LabelAdapter{
+// This is not great, but we deal with unsorted labels when validating labels.
+func TestShardByAllLabelsReturnsWrongResultsForUnsortedLabels(t *testing.T) {
+	val1 := shardByAllLabels("test", []client.LabelAdapter{
 		{Name: "__name__", Value: "foo"},
 		{Name: "bar", Value: "baz"},
 		{Name: "sample", Value: "1"},
 	})
 
-	assert.NotZero(t, val)
-	assert.NoError(t, err)
-
-	val, err = shardByAllLabels("test", []client.LabelAdapter{
+	val2 := shardByAllLabels("test", []client.LabelAdapter{
 		{Name: "__name__", Value: "foo"},
 		{Name: "sample", Value: "1"},
 		{Name: "bar", Value: "baz"},
 	})
 
-	assert.Zero(t, val)
-	assert.Error(t, err)
+	assert.NotEqual(t, val1, val2)
+}
+
+func TestSortLabels(t *testing.T) {
+	sorted := []client.LabelAdapter{
+		{Name: "__name__", Value: "foo"},
+		{Name: "bar", Value: "baz"},
+		{Name: "cluster", Value: "cluster"},
+		{Name: "sample", Value: "1"},
+	}
+
+	// no allocations if input is already sorted
+	require.Equal(t, 0.0, testing.AllocsPerRun(100, func() {
+		sortLabelsIfNeeded(sorted)
+	}))
+
+	unsorted := []client.LabelAdapter{
+		{Name: "__name__", Value: "foo"},
+		{Name: "sample", Value: "1"},
+		{Name: "cluster", Value: "cluster"},
+		{Name: "bar", Value: "baz"},
+	}
+
+	sortLabelsIfNeeded(unsorted)
+
+	sort.SliceIsSorted(unsorted, func(i, j int) bool {
+		return strings.Compare(unsorted[i].Name, unsorted[j].Name) < 0
+	})
 }

pkg/ingester/client/compat.go

@@ -190,6 +190,9 @@ func fromLabelMatchers(matchers []*LabelMatcher) ([]*labels.Matcher, error) {
 // FromLabelAdaptersToLabels casts []LabelAdapter to labels.Labels.
 // It uses unsafe, but as LabelAdapter == labels.Label this should be safe.
 // This allows us to use labels.Labels directly in protos.
+//
+// Note: while resulting labels.Labels is supposedly sorted, this function
+// doesn't enforce that. If input is not sorted, output will be wrong.
 func FromLabelAdaptersToLabels(ls []LabelAdapter) labels.Labels {
 	return *(*labels.Labels)(unsafe.Pointer(&ls))
 }

pkg/util/validation/validate.go

@@ -1,7 +1,9 @@
 package validation
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/cortexproject/cortex/pkg/ingester/client"
@@ -14,14 +16,16 @@ import (
 const (
 	discardReasonLabel = "reason"
 
-	errMissingMetricName = "sample missing metric name"
-	errInvalidMetricName = "sample invalid metric name: %.200q"
-	errInvalidLabel      = "sample invalid label: %.200q metric %.200q"
-	errLabelNameTooLong  = "label name too long: %.200q metric %.200q"
-	errLabelValueTooLong = "label value too long: %.200q metric %.200q"
-	errTooManyLabels     = "sample for '%s' has %d label names; limit %d"
-	errTooOld            = "sample for '%s' has timestamp too old: %d"
-	errTooNew            = "sample for '%s' has timestamp too new: %d"
+	errMissingMetricName  = "sample missing metric name"
+	errInvalidMetricName  = "sample invalid metric name: %.200q"
+	errInvalidLabel       = "sample invalid label: %.200q metric %.200q"
+	errLabelNameTooLong   = "label name too long: %.200q metric %.200q"
+	errLabelValueTooLong  = "label value too long: %.200q metric %.200q"
+	errTooManyLabels      = "sample for '%s' has %d label names; limit %d"
+	errTooOld             = "sample for '%s' has timestamp too old: %d"
+	errTooNew             = "sample for '%s' has timestamp too new: %d"
+	errDuplicateLabelName = "duplicate label name: %.200q metric %.200q"
+	errLabelsNotSorted    = "labels not sorted: %.200q metric %.200q"
 
 	// ErrQueryTooLong is used in chunk store and query frontend.
 	ErrQueryTooLong = "invalid query, length > limit (%s > %s)"
@@ -31,6 +35,8 @@ const (
 	tooFarInFuture          = "too_far_in_future"
 	invalidLabel            = "label_invalid"
 	labelNameTooLong        = "label_name_too_long"
+	duplicateLabelNames     = "duplicate_label_names"
+	labelsNotSorted         = "labels_not_sorted"
 	labelValueTooLong       = "label_value_too_long"
 
 	// RateLimited is one of the values for the reason to discard samples.
@@ -102,6 +108,7 @@ func ValidateLabels(cfg LabelValidationConfig, userID string, ls []client.LabelA
 
 	maxLabelNameLength := cfg.MaxLabelNameLength(userID)
 	maxLabelValueLength := cfg.MaxLabelValueLength(userID)
+	lastLabelName := ""
 	for _, l := range ls {
 		var errTemplate string
 		var reason string
@@ -118,11 +125,48 @@ func ValidateLabels(cfg LabelValidationConfig, userID string, ls []client.LabelA
 			reason = labelValueTooLong
 			errTemplate = errLabelValueTooLong
 			cause = l.Value
+		} else if cmp := strings.Compare(lastLabelName, l.Name); cmp >= 0 {
+			if cmp == 0 {
+				reason = duplicateLabelNames
+				errTemplate = errDuplicateLabelName
+				cause = l.Name
+			} else {
+				reason = labelsNotSorted
+				errTemplate = errLabelsNotSorted
+				cause = l.Name
+			}
 		}
 		if errTemplate != "" {
 			DiscardedSamples.WithLabelValues(reason, userID).Inc()
-			return httpgrpc.Errorf(http.StatusBadRequest, errTemplate, cause, client.FromLabelAdaptersToMetric(ls).String())
+			return httpgrpc.Errorf(http.StatusBadRequest, errTemplate, cause, formatLabelSet(ls))
 		}
+		lastLabelName = l.Name
 	}
 	return nil
 }
+
+// this function formats label adapters as a metric name with labels, while preserving
+// label order, and keeping duplicates. If there are multiple "__name__" labels, only
+// first one is used as metric name, other ones will be included as regular labels.
+func formatLabelSet(ls []client.LabelAdapter) string {
+	metricName, hasMetricName := "", false
+
+	labelStrings := make([]string, 0, len(ls))
+	for _, l := range ls {
+		if l.Name == model.MetricNameLabel && !hasMetricName && l.Value != "" {
+			metricName = l.Value
+			hasMetricName = true
+		} else {
+			labelStrings = append(labelStrings, fmt.Sprintf("%s=%q", l.Name, l.Value))
+		}
+	}
+
+	if len(labelStrings) == 0 {
+		if hasMetricName {
+			return metricName
+		}
+		return "{}"
+	}
+
+	return fmt.Sprintf("%s{%s}", metricName, strings.Join(labelStrings, ", "))
+}

pkg/util/validation/validate_test.go

@@ -80,3 +80,41 @@ func TestValidateLabels(t *testing.T) {
 		assert.Equal(t, c.err, err, "wrong error")
 	}
 }
+
+func TestValidateLabelOrder(t *testing.T) {
+	var cfg validateLabelsCfg
+	cfg.maxLabelNameLength = 10
+	cfg.maxLabelNamesPerSeries = 10
+	cfg.maxLabelValueLength = 10
+
+	userID := "testUser"
+
+	err := ValidateLabels(cfg, userID, []client.LabelAdapter{
+		{Name: model.MetricNameLabel, Value: "m"},
+		{Name: "b", Value: "b"},
+		{Name: "a", Value: "a"},
+	})
+	assert.Equal(t, httpgrpc.Errorf(http.StatusBadRequest, errLabelsNotSorted, "a", `m{b="b", a="a"}`), err)
+}
+
+func TestValidateLabelDuplication(t *testing.T) {
+	var cfg validateLabelsCfg
+	cfg.maxLabelNameLength = 10
+	cfg.maxLabelNamesPerSeries = 10
+	cfg.maxLabelValueLength = 10
+
+	userID := "testUser"
+
+	err := ValidateLabels(cfg, userID, []client.LabelAdapter{
+		{Name: model.MetricNameLabel, Value: "a"},
+		{Name: model.MetricNameLabel, Value: "b"},
+	})
+	assert.Equal(t, httpgrpc.Errorf(http.StatusBadRequest, errDuplicateLabelName, "__name__", `a{__name__="b"}`), err)
+
+	err = ValidateLabels(cfg, userID, []client.LabelAdapter{
+		{Name: model.MetricNameLabel, Value: "a"},
+		{Name: "a", Value: "a"},
+		{Name: "a", Value: "a"},
+	})
+	assert.Equal(t, httpgrpc.Errorf(http.StatusBadRequest, errDuplicateLabelName, "a", `a{a="a", a="a"}`), err)
+}