Add new limit: max_series_label_size_bytes #4848
Conversation
harry671003
force-pushed
the
new_limit_series_bytes
branch
2 times, most recently
from
8f0f943 to
4c38c22
Compare
pkg/util/validation/validate.go
Outdated
| maxSeriesLabelSizeBytesMutex.Lock() | ||
| defer maxSeriesLabelSizeBytesMutex.Unlock() | ||
| if prev, ok := maxSeriesLabelSizeBytesMap[userID]; ok { | ||
| if prev > maxSeriesLabelSizeBytes { | ||
| maxSeriesLabelSizeBytesMap[userID] = maxSeriesLabelSizeBytes | ||
| MaxSeriesLabelSizeBytesPerUser.WithLabelValues(userID).Set(float64(maxSeriesLabelSizeBytes)) | ||
| } | ||
| } else { | ||
| maxSeriesLabelSizeBytesMap[userID] = maxSeriesLabelSizeBytes |
There was a problem hiding this comment.
Im worried about this lock here!
Could we do this not on the hot path? Maybe use a defer or even do it on compactor?
There was a problem hiding this comment.
What would be the use case for these metrics?
There was a problem hiding this comment.
Before we can enable this, we need to make sure existing users are not impacted. This metrics provide a way to monitor the current largest series in terms of total bytes that each user have.
There was a problem hiding this comment.
I've removed this metric for now. The plan is to add it later in the private repo and not upstream.
| # Maximum combined size in bytes of all labels and label values accepted for a | ||
| # series. 0 to disable the limit. | ||
| # CLI flag: -validation.max-series-label-size-bytes | ||
| [max_series_label_size_bytes: <int> | default = 0] |
There was a problem hiding this comment.
I think we should be in consistent naming with other label limits and have the config name to be max_labels_size_bytes
There was a problem hiding this comment.
Renamed the flag and config to max_labels_size_bytes
harry671003
force-pushed
the
new_limit_series_bytes
branch
from
4c38c22 to
343aff9
Compare
harry671003
force-pushed
the
new_limit_series_bytes
branch
from
343aff9 to
2016332
Compare
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
harry671003
force-pushed
the
new_limit_series_bytes
branch
from
2016332 to
8f55ff9
Compare
|
Thanks for the contribution @harry671003 ! |
Signed-off-by: 🌲 Harry 🌊 John 🏔 [email protected]
What this PR does:
This PR introduces a new limit
max_series_label_size_bytes.Currently, it is possible for users of Cortex to OOM kill distributors and ingesters by increasing the size of each label and label value. For instance, a user can send 1 million series with each series having 30 labels of maximum size.
1 million * 30 labels * 3 KB per label = 85 GB.This new limit can prevent this from happening by rejecting series that have a combined label size greater than this limit.
Example: if the
max_series_label_size_bytesis set to 10 KB, any series that exceed will be rejected. 1 million active series for a user will consume1 million * 10 KB = 9 GB.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]