Skip to content

Commit

Permalink
Fixes from review comments
Browse files Browse the repository at this point in the history
Signed-off-by: György Krajcsovits <[email protected]>
  • Loading branch information
krajorama committed Feb 7, 2023
1 parent ca0d58b commit a302848
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 15 deletions.
2 changes: 1 addition & 1 deletion cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -3089,7 +3089,7 @@
"kind": "field",
"name": "accept_native_histograms",
"required": false,
"desc": "Flag to enable the ingestion of native histogram samples.",
"desc": "Enable ingestion of native histogram samples. If false, native histogram samples are ignored without an error.",
"fieldValue": null,
"fieldDefaultValue": false,
"fieldFlag": "ingester.accept-native-histograms",
Expand Down
2 changes: 1 addition & 1 deletion cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ Usage of ./cmd/mimir/mimir:
-http.prometheus-http-prefix string
HTTP URL path under which the Prometheus api will be served. (default "/prometheus")
-ingester.accept-native-histograms
[experimental] Flag to enable the ingestion of native histogram samples.
[experimental] Enable ingestion of native histogram samples. If false, native histogram samples are ignored without an error.
-ingester.active-series-custom-trackers value
Additional active series metrics, matching the provided matchers. Matchers should be in form <name>:<matcher>, like 'foobar:{foo="bar"}'. Multiple matchers can be provided either providing the flag multiple times or providing multiple semicolon-separated values to a single flag.
-ingester.active-series-metrics-enabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2566,7 +2566,8 @@ The `limits` block configures default and per-tenant limits imposed by component
# CLI flag: -ingester.max-global-exemplars-per-user
[max_global_exemplars_per_user: <int> | default = 0]

# (experimental) Flag to enable the ingestion of native histogram samples.
# (experimental) Enable ingestion of native histogram samples. If false, native
# histogram samples are ignored without an error.
# CLI flag: -ingester.accept-native-histograms
[accept_native_histograms: <boolean> | default = false]

Expand Down
14 changes: 6 additions & 8 deletions pkg/ingester/ingester.go
Original file line number Diff line number Diff line change
Expand Up @@ -1612,21 +1612,19 @@ func (i *Ingester) queryStreamSamples(ctx context.Context, db *userTSDB, from, t

it = series.Iterator(it)
for valType := it.Next(); valType != chunkenc.ValNone; valType = it.Next() {
if valType == chunkenc.ValFloat {
switch valType {
case chunkenc.ValFloat:
t, v := it.At()
ts.Samples = append(ts.Samples, mimirpb.Sample{
Value: v,
TimestampMs: t,
})
} else if valType == chunkenc.ValHistogram {
ts.Samples = append(ts.Samples, mimirpb.Sample{Value: v, TimestampMs: t})
case chunkenc.ValHistogram:
// TODO when read path for native histograms is ready, return Histogram samples
// t, v := it.AtHistogram()
// ts.Histograms = append(ts.Histograms, mimirpb.FromHistogramToHistogramProto(t, v))
} else if valType == chunkenc.ValFloatHistogram {
case chunkenc.ValFloatHistogram:
// TODO when read path for native histograms is ready, return Histogram samples
// t, v := it.AtFloatHistogram()
// ts.Histograms = append(ts.Histograms, mimirpb.FromFloatHistogramToHistogramProto(t, v))
} else {
default:
return 0, 0, 0, fmt.Errorf("unsupported value type: %v", valType)
}
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/mimirpb/mimir.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
// Provenance-includes-location: https://github.com/prometheus/prometheus/blob/main/prompb/types.proto
// Provenance-includes-license: Apache-2.0
// Provenance-includes-copyright: Prometheus Team.
// Provenance-includes-location: https://github.com/prometheus/common/blob/main/model/value_histogram.go
// Provenance-includes-license: Apache-2.0
// Provenance-includes-copyright: The Prometheus Authors.

syntax = "proto3";

Expand Down
2 changes: 1 addition & 1 deletion pkg/util/validation/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
f.IntVar(&l.MaxGlobalExemplarsPerUser, "ingester.max-global-exemplars-per-user", 0, "The maximum number of exemplars in memory, across the cluster. 0 to disable exemplars ingestion.")
f.Var(&l.ActiveSeriesCustomTrackersConfig, "ingester.active-series-custom-trackers", "Additional active series metrics, matching the provided matchers. Matchers should be in form <name>:<matcher>, like 'foobar:{foo=\"bar\"}'. Multiple matchers can be provided either providing the flag multiple times or providing multiple semicolon-separated values to a single flag.")
f.Var(&l.OutOfOrderTimeWindow, "ingester.out-of-order-time-window", "Non-zero value enables out-of-order support for most recent samples that are within the time window in relation to the TSDB's maximum time, i.e., within [db.maxTime-timeWindow, db.maxTime]). The ingester will need more memory as a factor of rate of out-of-order samples being ingested and the number of series that are getting out-of-order samples. A lower TTL of 10 minutes will be set for the query cache entries that overlap with this window.")
f.BoolVar(&l.AcceptNativeHistograms, "ingester.accept-native-histograms", false, "Flag to enable the ingestion of native histogram samples.")
f.BoolVar(&l.AcceptNativeHistograms, "ingester.accept-native-histograms", false, "Enable ingestion of native histogram samples. If false, native histogram samples are ignored without an error.")

f.StringVar(&l.SeparateMetricsGroupLabel, "validation.separate-metrics-group-label", "", "Label used to define the group label for metrics separation. For each write request, the group is obtained from the first non-empty group label from the first timeseries in the incoming list of timeseries. Specific distributor and ingester metrics will be further separated adding a 'group' label with group label's value. Currently applies to the following metrics: cortex_discarded_samples_total")

Expand Down

0 comments on commit a302848

Please sign in to comment.