Resolving comments

Signed-off-by: Paurush Garg <[email protected]>

Author: PaurushGarg

CHANGELOG.md

@@ -52,6 +52,7 @@
 * [ENHANCEMENT] Metadata Cache: Support inmemory and multi level cache backend. #6829
 * [ENHANCEMENT] Store Gateway: Allow to ignore syncing blocks older than certain time using `ignore_blocks_before`. #6830
 * [ENHANCEMENT] Distributor: Add native histograms max sample size bytes limit validation. #6834
+* [ENHANCEMENT] Ingester: Add activeSeries limit specifically for Native Histogram. #6796
 * [BUGFIX] Ingester: Avoid error or early throttling when READONLY ingesters are present in the ring #6517
 * [BUGFIX] Ingester: Fix labelset data race condition. #6573
 * [BUGFIX] Compactor: Cleaner should not put deletion marker for blocks with no-compact marker. #6576

docs/configuration/config-file-reference.md

@@ -3606,10 +3606,10 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s
 # CLI flag: -ingester.max-series-per-metric
 [max_series_per_metric: <int> | default = 50000]
 
-# The maximum number of active nativeHistograms series per user, per ingester. 0
+# The maximum number of active native histogram series per user, per ingester. 0
 # to disable.
-# CLI flag: -ingester.max-native-histograms-series-per-user
-[max_native_histograms_series_per_user: <int> | default = 5000000]
+# CLI flag: -ingester.max-native-histogram-series-per-user
+[max_native_histogram_series_per_user: <int> | default = 5000000]
 
 # The maximum number of active series per user, across the cluster before
 # replication. 0 to disable. Supported only if -distributor.shard-by-all-labels
@@ -3622,11 +3622,11 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s
 # CLI flag: -ingester.max-global-series-per-metric
 [max_global_series_per_metric: <int> | default = 0]
 
-# The maximum number of active nativeHistograms series per user, across the
+# The maximum number of active native histogram series per user, across the
 # cluster before replication. 0 to disable. Supported only if
 # -distributor.shard-by-all-labels is true.
-# CLI flag: -ingester.max-global-native-histograms-series-per-user
-[max_global_native_histograms_series_per_user: <int> | default = 0]
+# CLI flag: -ingester.max-global-native-histogram-series-per-user
+[max_global_native_histogram_series_per_user: <int> | default = 0]
 
 # [Experimental] Enable limits per LabelSet. Supported limits per labelSet:
 # [max_series]

pkg/ingester/ingester.go

@@ -449,7 +449,7 @@ func (u *userTSDB) PreCreation(metric labels.Labels) error {
 	}
 
 	// Total nativeHistograms series limit.
-	if err := u.limiter.AssertMaxNativeHistogramsSeriesPerUser(u.userID, u.activeSeries.ActiveNativeHistogram()); err != nil {
+	if err := u.limiter.AssertMaxNativeHistogramSeriesPerUser(u.userID, u.activeSeries.ActiveNativeHistogram()); err != nil {
 		return err
 	}
 
@@ -1224,22 +1224,22 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte
 	// Keep track of some stats which are tracked only if the samples will be
 	// successfully committed
 	var (
-		succeededSamplesCount                   = 0
-		failedSamplesCount                      = 0
-		succeededHistogramsCount                = 0
-		failedHistogramsCount                   = 0
-		succeededExemplarsCount                 = 0
-		failedExemplarsCount                    = 0
-		startAppend                             = time.Now()
-		sampleOutOfBoundsCount                  = 0
-		sampleOutOfOrderCount                   = 0
-		sampleTooOldCount                       = 0
-		newValueForTimestampCount               = 0
-		perUserSeriesLimitCount                 = 0
-		perUserNativeHistogramsSeriesLimitCount = 0
-		perLabelSetSeriesLimitCount             = 0
-		perMetricSeriesLimitCount               = 0
-		discardedNativeHistogramCount           = 0
+		succeededSamplesCount                  = 0
+		failedSamplesCount                     = 0
+		succeededHistogramsCount               = 0
+		failedHistogramsCount                  = 0
+		succeededExemplarsCount                = 0
+		failedExemplarsCount                   = 0
+		startAppend                            = time.Now()
+		sampleOutOfBoundsCount                 = 0
+		sampleOutOfOrderCount                  = 0
+		sampleTooOldCount                      = 0
+		newValueForTimestampCount              = 0
+		perUserSeriesLimitCount                = 0
+		perUserNativeHistogramSeriesLimitCount = 0
+		perLabelSetSeriesLimitCount            = 0
+		perMetricSeriesLimitCount              = 0
+		discardedNativeHistogramCount          = 0
 
 		updateFirstPartial = func(errFn func() error) {
 			if firstPartialErr == nil {
@@ -1275,8 +1275,8 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte
 					return makeLimitError(perUserSeriesLimit, i.limiter.FormatError(userID, cause, copiedLabels))
 				})
 
-			case errors.Is(cause, errMaxNativeHistogramsSeriesPerUserLimitExceeded):
-				perUserNativeHistogramsSeriesLimitCount++
+			case errors.Is(cause, errMaxNativeHistogramSeriesPerUserLimitExceeded):
+				perUserNativeHistogramSeriesLimitCount++
 				updateFirstPartial(func() error {
 					return makeLimitError(perUserSeriesLimit, i.limiter.FormatError(userID, cause, copiedLabels))
 				})
@@ -1524,8 +1524,8 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte
 	if perUserSeriesLimitCount > 0 {
 		i.validateMetrics.DiscardedSamples.WithLabelValues(perUserSeriesLimit, userID).Add(float64(perUserSeriesLimitCount))
 	}
-	if perUserNativeHistogramsSeriesLimitCount > 0 {
-		i.validateMetrics.DiscardedSamples.WithLabelValues(perUserNativeHistogramsSeriesLimit, userID).Add(float64(perUserNativeHistogramsSeriesLimitCount))
+	if perUserNativeHistogramSeriesLimitCount > 0 {
+		i.validateMetrics.DiscardedSamples.WithLabelValues(perUserNativeHistogramSeriesLimit, userID).Add(float64(perUserNativeHistogramSeriesLimitCount))
 	}
 	if perMetricSeriesLimitCount > 0 {
 		i.validateMetrics.DiscardedSamples.WithLabelValues(perMetricSeriesLimit, userID).Add(float64(perMetricSeriesLimitCount))

pkg/ingester/ingester_test.go

@@ -868,10 +868,10 @@ func TestIngesterUserLimitExceeded(t *testing.T) {
 
 }
 
-func TestIngesterUserLimitExceededForNativeHistograms(t *testing.T) {
+func TestIngesterUserLimitExceededForNativeHistogram(t *testing.T) {
 	limits := defaultLimitsTestConfig()
 	limits.EnableNativeHistograms = true
-	limits.MaxLocalNativeHistogramsSeriesPerUser = 1
+	limits.MaxLocalNativeHistogramSeriesPerUser = 1
 	limits.MaxLocalSeriesPerUser = 1
 	limits.MaxLocalMetricsWithMetadataPerUser = 1
 
@@ -923,7 +923,7 @@ func TestIngesterUserLimitExceededForNativeHistograms(t *testing.T) {
 				httpResp, ok := httpgrpc.HTTPResponseFromError(err)
 				require.True(t, ok, "returned error is not an httpgrpc response")
 				assert.Equal(t, http.StatusBadRequest, int(httpResp.Code))
-				assert.Equal(t, wrapWithUser(makeLimitError(perUserNativeHistogramsSeriesLimit, ing.limiter.FormatError(userID, errMaxNativeHistogramsSeriesPerUserLimitExceeded, labels1)), userID).Error(), string(httpResp.Body))
+				assert.Equal(t, wrapWithUser(makeLimitError(perUserNativeHistogramSeriesLimit, ing.limiter.FormatError(userID, errMaxNativeHistogramSeriesPerUserLimitExceeded, labels1)), userID).Error(), string(httpResp.Body))
 
 				// Append two metadata, expect no error since metadata is a best effort approach.
 				_, err = ing.Push(ctx, cortexpb.ToWriteRequest(nil, nil, []*cortexpb.MetricMetadata{metadata1, metadata2}, nil, cortexpb.API))

pkg/ingester/limiter.go

@@ -12,11 +12,11 @@ import (
 )
 
 var (
-	errMaxSeriesPerMetricLimitExceeded               = errors.New("per-metric series limit exceeded")
-	errMaxMetadataPerMetricLimitExceeded             = errors.New("per-metric metadata limit exceeded")
-	errMaxSeriesPerUserLimitExceeded                 = errors.New("per-user series limit exceeded")
-	errMaxNativeHistogramsSeriesPerUserLimitExceeded = errors.New("per-user nativeHistograms series limit exceeded")
-	errMaxMetadataPerUserLimitExceeded               = errors.New("per-user metric metadata limit exceeded")
+	errMaxSeriesPerMetricLimitExceeded              = errors.New("per-metric series limit exceeded")
+	errMaxMetadataPerMetricLimitExceeded            = errors.New("per-metric metadata limit exceeded")
+	errMaxSeriesPerUserLimitExceeded                = errors.New("per-user series limit exceeded")
+	errMaxNativeHistogramSeriesPerUserLimitExceeded = errors.New("per-user native histogram series limit exceeded")
+	errMaxMetadataPerUserLimitExceeded              = errors.New("per-user metric metadata limit exceeded")
 )
 
 type errMaxSeriesPerLabelSetLimitExceeded struct {
@@ -96,14 +96,14 @@ func (l *Limiter) AssertMaxSeriesPerUser(userID string, series int) error {
 	return errMaxSeriesPerUserLimitExceeded
 }
 
-// AssertMaxNativeHistogramsSeriesPerUser limit has not been reached compared to the current
-// number of nativeHistograms series in input and returns an error if so.
-func (l *Limiter) AssertMaxNativeHistogramsSeriesPerUser(userID string, series int) error {
-	if actualLimit := l.maxNativeHistogramsSeriesPerUser(userID); series < actualLimit {
+// AssertMaxNativeHistogramSeriesPerUser limit has not been reached compared to the current
+// number of native histogram series in input and returns an error if so.
+func (l *Limiter) AssertMaxNativeHistogramSeriesPerUser(userID string, series int) error {
+	if actualLimit := l.maxNativeHistogramSeriesPerUser(userID); series < actualLimit {
 		return nil
 	}
 
-	return errMaxNativeHistogramsSeriesPerUserLimitExceeded
+	return errMaxNativeHistogramSeriesPerUserLimitExceeded
 }
 
 // AssertMaxMetricsWithMetadataPerUser limit has not been reached compared to the current
@@ -145,7 +145,7 @@ func (l *Limiter) FormatError(userID string, err error, lbls labels.Labels) erro
 	switch {
 	case errors.Is(err, errMaxSeriesPerUserLimitExceeded):
 		return l.formatMaxSeriesPerUserError(userID)
-	case errors.Is(err, errMaxNativeHistogramsSeriesPerUserLimitExceeded):
+	case errors.Is(err, errMaxNativeHistogramSeriesPerUserLimitExceeded):
 		return l.formatMaxNativeHistogramsSeriesPerUserError(userID)
 	case errors.Is(err, errMaxSeriesPerMetricLimitExceeded):
 		return l.formatMaxSeriesPerMetricError(userID, lbls.Get(labels.MetricName))
@@ -172,9 +172,9 @@ func (l *Limiter) formatMaxSeriesPerUserError(userID string) error {
 }
 
 func (l *Limiter) formatMaxNativeHistogramsSeriesPerUserError(userID string) error {
-	actualLimit := l.maxNativeHistogramsSeriesPerUser(userID)
-	localLimit := l.limits.MaxLocalNativeHistogramsSeriesPerUser(userID)
-	globalLimit := l.limits.MaxGlobalNativeHistogramsSeriesPerUser(userID)
+	actualLimit := l.maxNativeHistogramSeriesPerUser(userID)
+	localLimit := l.limits.MaxLocalNativeHistogramSeriesPerUser(userID)
+	globalLimit := l.limits.MaxGlobalNativeHistogramSeriesPerUser(userID)
 
 	return fmt.Errorf("per-user nativeHistograms series limit of %d exceeded, %s (local limit: %d global limit: %d actual local limit: %d)",
 		minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit)
@@ -270,11 +270,11 @@ func (l *Limiter) maxSeriesPerUser(userID string) int {
 	)
 }
 
-func (l *Limiter) maxNativeHistogramsSeriesPerUser(userID string) int {
+func (l *Limiter) maxNativeHistogramSeriesPerUser(userID string) int {
 	return l.maxByLocalAndGlobal(
 		userID,
-		l.limits.MaxLocalNativeHistogramsSeriesPerUser,
-		l.limits.MaxGlobalNativeHistogramsSeriesPerUser,
+		l.limits.MaxLocalNativeHistogramSeriesPerUser,
+		l.limits.MaxGlobalNativeHistogramSeriesPerUser,
 	)
 }
 

pkg/ingester/limiter_test.go

@@ -56,12 +56,12 @@ func TestLimiter_maxSeriesPerUser(t *testing.T) {
 
 func TestLimiter_maxNativeHistogramsSeriesPerUser(t *testing.T) {
 	applyLimits := func(limits *validation.Limits, localLimit, globalLimit int) {
-		limits.MaxLocalNativeHistogramsSeriesPerUser = localLimit
-		limits.MaxGlobalNativeHistogramsSeriesPerUser = globalLimit
+		limits.MaxLocalNativeHistogramSeriesPerUser = localLimit
+		limits.MaxGlobalNativeHistogramSeriesPerUser = globalLimit
 	}
 
 	runMaxFn := func(limiter *Limiter) int {
-		return limiter.maxNativeHistogramsSeriesPerUser("test")
+		return limiter.maxNativeHistogramSeriesPerUser("test")
 	}
 
 	runLimiterMaxFunctionTest(t, applyLimits, runMaxFn, false)
@@ -473,7 +473,7 @@ func TestLimiter_AssertMaxNativeHistogramsSeriesPerUser(t *testing.T) {
 			ringIngesterCount:                      10,
 			shardByAllLabels:                       true,
 			series:                                 300,
-			expected:                               errMaxNativeHistogramsSeriesPerUserLimitExceeded,
+			expected:                               errMaxNativeHistogramSeriesPerUserLimitExceeded,
 		},
 	}
 
@@ -488,13 +488,13 @@ func TestLimiter_AssertMaxNativeHistogramsSeriesPerUser(t *testing.T) {
 
 			// Mock limits
 			limits, err := validation.NewOverrides(validation.Limits{
-				MaxLocalNativeHistogramsSeriesPerUser:  testData.maxLocalNativeHistogramsSeriesPerUser,
-				MaxGlobalNativeHistogramsSeriesPerUser: testData.maxGlobalNativeHistogramsSeriesPerUser,
+				MaxLocalNativeHistogramSeriesPerUser:  testData.maxLocalNativeHistogramsSeriesPerUser,
+				MaxGlobalNativeHistogramSeriesPerUser: testData.maxGlobalNativeHistogramsSeriesPerUser,
 			}, nil)
 			require.NoError(t, err)
 
 			limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false, "")
-			actual := limiter.AssertMaxNativeHistogramsSeriesPerUser("test", testData.series)
+			actual := limiter.AssertMaxNativeHistogramSeriesPerUser("test", testData.series)
 
 			assert.Equal(t, testData.expected, actual)
 		})
@@ -656,11 +656,11 @@ func TestLimiter_FormatError(t *testing.T) {
 
 	// Mock limits
 	limits, err := validation.NewOverrides(validation.Limits{
-		MaxGlobalSeriesPerUser:                 100,
-		MaxGlobalNativeHistogramsSeriesPerUser: 100,
-		MaxGlobalSeriesPerMetric:               20,
-		MaxGlobalMetricsWithMetadataPerUser:    10,
-		MaxGlobalMetadataPerMetric:             3,
+		MaxGlobalSeriesPerUser:                100,
+		MaxGlobalNativeHistogramSeriesPerUser: 100,
+		MaxGlobalSeriesPerMetric:              20,
+		MaxGlobalMetricsWithMetadataPerUser:   10,
+		MaxGlobalMetadataPerMetric:            3,
 	}, nil)
 	require.NoError(t, err)
 
@@ -670,7 +670,7 @@ func TestLimiter_FormatError(t *testing.T) {
 	actual := limiter.FormatError("user-1", errMaxSeriesPerUserLimitExceeded, lbls)
 	assert.EqualError(t, actual, "per-user series limit of 100 exceeded, please contact administrator to raise it (local limit: 0 global limit: 100 actual local limit: 100)")
 
-	actual = limiter.FormatError("user-1", errMaxNativeHistogramsSeriesPerUserLimitExceeded, lbls)
+	actual = limiter.FormatError("user-1", errMaxNativeHistogramSeriesPerUserLimitExceeded, lbls)
 	assert.EqualError(t, actual, "per-user nativeHistograms series limit of 100 exceeded, please contact administrator to raise it (local limit: 0 global limit: 100 actual local limit: 100)")
 
 	actual = limiter.FormatError("user-1", errMaxSeriesPerMetricLimitExceeded, lbls)

pkg/ingester/user_state.go

@@ -16,10 +16,10 @@ import (
 
 // DiscardedSamples metric labels
 const (
-	perUserSeriesLimit                 = "per_user_series_limit"
-	perUserNativeHistogramsSeriesLimit = "per_user_native_histograms_series_limit"
-	perMetricSeriesLimit               = "per_metric_series_limit"
-	perLabelsetSeriesLimit             = "per_labelset_series_limit"
+	perUserSeriesLimit                = "per_user_series_limit"
+	perUserNativeHistogramSeriesLimit = "per_user_native_histogram_series_limit"
+	perMetricSeriesLimit              = "per_metric_series_limit"
+	perLabelsetSeriesLimit            = "per_labelset_series_limit"
 )
 
 const numMetricCounterShards = 128