Skip to content
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

Proposal: Create a new "Per LabelSet" limit #5950

Merged
merged 4 commits into from
May 15, 2024

Conversation

alanprot
Copy link
Member

@alanprot alanprot commented May 14, 2024

What this PR does:

This pull request (PR) introduces a new limit Active Series "Per LabelSet." This limit functions similarly to -ingester.max-series-per-metric, but allowing users to define the maximum number of series per LabelSet.

These limits are configurable via runtime settings and with the following format:

max_series_per_label_set:
  - label_set:
      labelName1: LabelValue1
    limit: 10
  - label_set:
      labelName1: LabelValue1
      labelName2: LabelValue2
    limit: 3

The limits are applied "Per labelset", meaning that only metrics containing all the labels of the limit will be subjected to it.

Additionally, a new metric is implemented to monitor the active timeseries for each limit as follows:

# HELP cortex_ingester_active_series_per_labelset Number of currently active series per user and labelset.
# TYPE cortex_ingester_active_series_per_labelset gauge
cortex_ingester_active_series_per_labelset{labelset="{label1=\"value1\"}",user="1"} 3
cortex_ingester_active_series_per_labelset{labelset="{label2=\"value2\"}",user="1"} 2
cortex_ingester_active_series_per_labelset{labelset="{comp1=\"compValue1\", comp2=\"compValue2\"}",user="1"} 2
cortex_ingester_active_series_per_labelset{labelset="{comp1=\"compValue1\"}",user="1"} 7
cortex_ingester_active_series_per_labelset{labelset="{comp2=\"compValue2\"}",user="1"} 2

The motivation behind these limits is to enable service administrators to set constraints on a per labelset basis. This functionality helps mitigate the risk of unexpected increases in the number of timeseries.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: alanprot <alanprot@gmail.com>
Signed-off-by: alanprot <alanprot@gmail.com>
Signed-off-by: alanprot <alanprot@gmail.com>
@alanprot alanprot force-pushed the limits-labelset branch 2 times, most recently from 96dbf32 to e64c0e1 Compare May 14, 2024 19:11
Signed-off-by: alanprot <alanprot@gmail.com>
@alanprot alanprot marked this pull request as ready for review May 14, 2024 23:10
@alanprot
Copy link
Member Author

I still need to update the changelog but I think we can start reviewing it.

cc @yeya24 @friedrich-at-adobe

@alanprot alanprot requested a review from yeya24 May 14, 2024 23:12
@alanprot alanprot changed the title Proposal: Create a new "Per LabelSet" limit [WIP] Proposal: Create a new "Per LabelSet" limit May 15, 2024
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks

@yeya24 yeya24 merged commit 2948539 into cortexproject:master May 15, 2024
16 checks passed
perMetricSeriesLimitCount++
updateFirstPartial(func() error {
return makeMetricLimitError(perMetricSeriesLimit, copiedLabels, i.limiter.FormatError(userID, cause))
})
continue
case errors.As(cause, &errMaxSeriesPerLabelSetLimitExceeded{}):
updateFirstPartial(func() error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dont we want to create a new discarded samples reason label here?

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed.

I will create another PR with this and the chngelog in a bit.

@jeromeinsf
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants