Skip to content

Add global limit to the max series per user and metric #1760

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

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Oct 25, 2019

Following up to this design doc, in this PR I propose to introduce a global limit on the maximum number of series per user and metric.

The global limit has been implemented as a local (per-ingester) limit configured based on the number of ingesters in the cluster + replication factor.

Notes:

  • The ingester now requires -distributor.shard-by-all-labels config option as well, otherwise it doesn't know it. Not super nice, but I haven't find a better way to do it (better ideas?)
  • The ingester now depends on the read ring, to count the number of ingesters across the cluster
  • The local limit has been kept for backward compatibility, but can be disabled with a value of 0
  • The global limit is disabled by default

@pracucci pracucci force-pushed the add-global-max-series-per-user-and-metric branch from 28e8ea0 to 368588a Compare October 25, 2019 08:29
Copy link
Contributor

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM with some comments!

@pracucci pracucci force-pushed the add-global-max-series-per-user-and-metric branch 2 times, most recently from ef95857 to 53404e1 Compare October 28, 2019 16:50
@pracucci pracucci requested a review from gouthamve October 28, 2019 16:50
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Good job! LGTM.

@pracucci pracucci force-pushed the add-global-max-series-per-user-and-metric branch from d289680 to 049b2bd Compare October 30, 2019 14:55
@gouthamve gouthamve requested a review from jtlisi October 30, 2019 18:36
@pracucci pracucci force-pushed the add-global-max-series-per-user-and-metric branch 2 times, most recently from e9cae33 to d0ac12c Compare October 31, 2019 07:44
Copy link
Contributor

@jtlisi jtlisi left a comment

Choose a reason for hiding this comment

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

LGTM

- `-ingester.max-global-series-per-user`
- `-ingester.max-global-series-per-metric`

Signed-off-by: Marco Pracucci <[email protected]>
…o create a new ring client for the ingesters

Signed-off-by: Marco Pracucci <[email protected]>
…unters and the ring desc is nil

Signed-off-by: Marco Pracucci <[email protected]>
…s lifecycler updateCounters()

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci force-pushed the add-global-max-series-per-user-and-metric branch from d0ac12c to c0ef755 Compare October 31, 2019 15:44
@gouthamve gouthamve merged commit 00566f6 into cortexproject:master Oct 31, 2019
@pracucci pracucci deleted the add-global-max-series-per-user-and-metric branch November 1, 2019 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants