-
Notifications
You must be signed in to change notification settings - Fork 647
Add config option to exclude 'le' from series sharding to ingesters / partitions #12815
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: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||
---|---|---|---|---|---|---|
|
@@ -1794,6 +1794,27 @@ | |||||
"fieldValue": null, | ||||||
"fieldDefaultValue": null | ||||||
}, | ||||||
{ | ||||||
"kind": "block", | ||||||
"name": "sharding", | ||||||
"required": false, | ||||||
"desc": "", | ||||||
"blockEntries": [ | ||||||
{ | ||||||
"kind": "field", | ||||||
"name": "exclude_classic_histogram_bucket_label", | ||||||
"required": false, | ||||||
"desc": "When set to true, the distributor computes the series hash – used for sharding – excluding the 'le' label. This means that, when this configuration option is set to true, all buckets of a classic histogram metric are stored in the same shard. This configuration option should be set for distributors, rulers and ingesters.", | ||||||
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.
Suggested change
|
||||||
"fieldValue": null, | ||||||
"fieldDefaultValue": false, | ||||||
"fieldFlag": "distributor.sharding.exclude-classic-histogram-bucket-label", | ||||||
"fieldType": "boolean", | ||||||
"fieldCategory": "experimental" | ||||||
} | ||||||
], | ||||||
"fieldValue": null, | ||||||
"fieldDefaultValue": null | ||||||
}, | ||||||
{ | ||||||
"kind": "field", | ||||||
"name": "write_requests_buffer_pooling_enabled", | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -379,6 +379,7 @@ | |
"validation.max-cost-attribution-cardinality": 2000, | ||
"validation.cost-attribution-cooldown": 0, | ||
"ruler.evaluation-delay-duration": 60000000000, | ||
"ruler.evaluation-consistency-max-delay": 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. Know where this diff came from? 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. |
||
"ruler.tenant-shard-size": 0, | ||
"ruler.max-rules-per-rule-group": 20, | ||
"ruler.max-rule-groups-per-tenant": 70, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,19 @@ | |
package mimirpb | ||
|
||
import ( | ||
"flag" | ||
|
||
"github.com/prometheus/prometheus/model/labels" | ||
) | ||
|
||
type ShardingConfig struct { | ||
ExcludeClassicHistogramBucketLabel bool `yaml:"exclude_classic_histogram_bucket_label" category:"experimental"` | ||
} | ||
|
||
func (l *ShardingConfig) RegisterFlags(f *flag.FlagSet) { | ||
f.BoolVar(&l.ExcludeClassicHistogramBucketLabel, "distributor.sharding.exclude-classic-histogram-bucket-label", false, "When set to true, the distributor computes the series hash – used for sharding – excluding the 'le' label. This means that, when this configuration option is set to true, all buckets of a classic histogram metric are stored in the same shard. This configuration option should be set for distributors, rulers and ingesters.") | ||
} | ||
|
||
// ShardByMetricName returns the token for the given metric. The provided metricName | ||
// is guaranteed to not be retained. | ||
func ShardByMetricName(userID string, metricName string) uint32 { | ||
|
@@ -23,22 +33,30 @@ func ShardByUser(userID string) uint32 { | |
return h | ||
} | ||
|
||
// ShardByAllLabels returns the token for given user and series. | ||
// ShardBySeriesLabels returns the token that must be used to shard a series across ingesters / partitions for given user. | ||
// | ||
// ShardByAllLabels generates different values for different order of same labels. | ||
func ShardByAllLabels(userID string, ls labels.Labels) uint32 { | ||
// ShardBySeriesLabels generates different values for different order of same labels. | ||
func ShardBySeriesLabels(userID string, ls labels.Labels, cfg ShardingConfig) uint32 { | ||
h := ShardByUser(userID) | ||
ls.Range(func(l labels.Label) { | ||
if cfg.ExcludeClassicHistogramBucketLabel && l.Name == labels.BucketLabel { | ||
return | ||
} | ||
|
||
h = HashAdd32(h, l.Name) | ||
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 suppose it wouldn't improve anything to include the label name in the hash as all series in 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 don't think adding just the label name makes any practical difference. |
||
h = HashAdd32(h, l.Value) | ||
}) | ||
return h | ||
} | ||
|
||
// ShardByAllLabelAdapters is like ShardByAllLabel, but uses LabelAdapter type. | ||
func ShardByAllLabelAdapters(userID string, ls []LabelAdapter) uint32 { | ||
// ShardBySeriesLabelAdapters is like ShardByAllLabel, but uses LabelAdapter type. | ||
func ShardBySeriesLabelAdapters(userID string, ls []LabelAdapter, cfg ShardingConfig) uint32 { | ||
h := ShardByUser(userID) | ||
for _, l := range ls { | ||
if cfg.ExcludeClassicHistogramBucketLabel && l.Name == labels.BucketLabel { | ||
continue | ||
} | ||
|
||
h = HashAdd32(h, l.Name) | ||
h = HashAdd32(h, l.Value) | ||
} | ||
|
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.