-
Notifications
You must be signed in to change notification settings - Fork 823
Add active series limit for nativeHistogram samples #6796
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: master
Are you sure you want to change the base?
Changes from 6 commits
4f66785
710de24
20bbde4
24e6573
580b7a1
ab7970d
1085c91
555c6c1
6a5e237
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 |
---|---|---|
|
@@ -3606,6 +3606,11 @@ 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 native histogram series per user, per ingester. 0 | ||
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. Add to the description that this limit needs active series tracker to be enabled. 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. Thanks updated. |
||
# to disable. | ||
# 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 | ||
# is true. | ||
|
@@ -3617,6 +3622,12 @@ 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 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-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] | ||
[limits_per_label_set: <list of LimitsPerLabelSet> | default = []] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -448,6 +448,11 @@ func (u *userTSDB) PreCreation(metric labels.Labels) error { | |
} | ||
} | ||
|
||
// Total nativeHistograms series limit. | ||
if err := u.limiter.AssertMaxNativeHistogramSeriesPerUser(u.userID, u.activeSeries.ActiveNativeHistogram()); err != nil { | ||
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. Can activeSeries be nil? Should we handle for that? 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. I think null check should not be required as preallocation is done while initialization: https://github.com/cortexproject/cortex/blob/master/pkg/ingester/active_series.go#L44 |
||
return err | ||
} | ||
|
||
// Total series limit. | ||
if err := u.limiter.AssertMaxSeriesPerUser(u.userID, int(u.Head().NumSeries())); err != nil { | ||
return err | ||
|
@@ -1219,21 +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 | ||
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 { | ||
|
@@ -1269,6 +1275,12 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte | |
return makeLimitError(perUserSeriesLimit, i.limiter.FormatError(userID, cause, copiedLabels)) | ||
}) | ||
|
||
case errors.Is(cause, errMaxNativeHistogramSeriesPerUserLimitExceeded): | ||
perUserNativeHistogramSeriesLimitCount++ | ||
updateFirstPartial(func() error { | ||
return makeLimitError(perUserSeriesLimit, i.limiter.FormatError(userID, cause, copiedLabels)) | ||
}) | ||
|
||
case errors.Is(cause, errMaxSeriesPerMetricLimitExceeded): | ||
perMetricSeriesLimitCount++ | ||
updateFirstPartial(func() error { | ||
|
@@ -1512,6 +1524,9 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte | |
if perUserSeriesLimitCount > 0 { | ||
i.validateMetrics.DiscardedSamples.WithLabelValues(perUserSeriesLimit, userID).Add(float64(perUserSeriesLimitCount)) | ||
} | ||
if perUserNativeHistogramSeriesLimitCount > 0 { | ||
i.validateMetrics.DiscardedSamples.WithLabelValues(perUserNativeHistogramSeriesLimit, userID).Add(float64(perUserNativeHistogramSeriesLimitCount)) | ||
} | ||
if perMetricSeriesLimitCount > 0 { | ||
i.validateMetrics.DiscardedSamples.WithLabelValues(perMetricSeriesLimit, userID).Add(float64(perMetricSeriesLimitCount)) | ||
} | ||
|
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.
This changelog item needs to be grouped with other enhancement items
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.
Thanks. Updated.