From 6e08029b56ae17c49e133d92a2792f6f119f2cbd Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 2 Jan 2019 11:10:13 +0000 Subject: [PATCH] Move err to be the last return value from storage.Select. (#5054) Signed-off-by: Tom Wilkie --- promql/engine.go | 4 ++-- promql/engine_test.go | 4 ++-- rules/manager_test.go | 2 +- storage/fanout.go | 8 ++++---- storage/interface.go | 2 +- storage/noop.go | 2 +- storage/remote/read.go | 16 ++++++++-------- storage/remote/read_test.go | 6 +++--- storage/tsdb/tsdb.go | 4 ++-- web/api/v1/api.go | 4 ++-- web/api/v1/api_test.go | 2 +- web/federate.go | 2 +- 12 files changed, 28 insertions(+), 28 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index f38ffba8e..c10a9609d 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -563,7 +563,7 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev params.End = params.End - offsetMilliseconds } - set, err, wrn = querier.Select(params, n.LabelMatchers...) + set, wrn, err = querier.Select(params, n.LabelMatchers...) warnings = append(warnings, wrn...) if err != nil { level.Error(ng.logger).Log("msg", "error selecting series set", "err", err) @@ -582,7 +582,7 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev params.End = params.End - offsetMilliseconds } - set, err, wrn = querier.Select(params, n.LabelMatchers...) + set, wrn, err = querier.Select(params, n.LabelMatchers...) warnings = append(warnings, wrn...) if err != nil { level.Error(ng.logger).Log("msg", "error selecting series set", "err", err) diff --git a/promql/engine_test.go b/promql/engine_test.go index 15af85caa..a56d00141 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -169,8 +169,8 @@ type errQuerier struct { err error } -func (q *errQuerier) Select(*storage.SelectParams, ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { - return errSeriesSet{err: q.err}, q.err, nil +func (q *errQuerier) Select(*storage.SelectParams, ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { + return errSeriesSet{err: q.err}, nil, q.err } func (*errQuerier) LabelValues(name string) ([]string, error) { return nil, nil } func (*errQuerier) LabelNames() ([]string, error) { return nil, nil } diff --git a/rules/manager_test.go b/rules/manager_test.go index 365a4f51f..3c693a89e 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -538,7 +538,7 @@ func TestStaleness(t *testing.T) { matcher, err := labels.NewMatcher(labels.MatchEqual, model.MetricNameLabel, "a_plus_one") testutil.Ok(t, err) - set, err, _ := querier.Select(nil, matcher) + set, _, err := querier.Select(nil, matcher) testutil.Ok(t, err) samples, err := readSeriesSet(set) diff --git a/storage/fanout.go b/storage/fanout.go index 1b464c4d5..81ae99dd6 100644 --- a/storage/fanout.go +++ b/storage/fanout.go @@ -228,11 +228,11 @@ func NewMergeQuerier(primaryQuerier Querier, queriers []Querier) Querier { } // Select returns a set of series that matches the given label matchers. -func (q *mergeQuerier) Select(params *SelectParams, matchers ...*labels.Matcher) (SeriesSet, error, Warnings) { +func (q *mergeQuerier) Select(params *SelectParams, matchers ...*labels.Matcher) (SeriesSet, Warnings, error) { seriesSets := make([]SeriesSet, 0, len(q.queriers)) var warnings Warnings for _, querier := range q.queriers { - set, err, wrn := querier.Select(params, matchers...) + set, wrn, err := querier.Select(params, matchers...) q.setQuerierMap[set] = querier if wrn != nil { warnings = append(warnings, wrn...) @@ -244,12 +244,12 @@ func (q *mergeQuerier) Select(params *SelectParams, matchers ...*labels.Matcher) warnings = append(warnings, err) continue } else { - return nil, err, nil + return nil, nil, err } } seriesSets = append(seriesSets, set) } - return NewMergeSeriesSet(seriesSets, q), nil, warnings + return NewMergeSeriesSet(seriesSets, q), warnings, nil } // LabelValues returns all potential values for a label name. diff --git a/storage/interface.go b/storage/interface.go index 94ced8689..fea1c9166 100644 --- a/storage/interface.go +++ b/storage/interface.go @@ -52,7 +52,7 @@ type Queryable interface { // Querier provides reading access to time series data. type Querier interface { // Select returns a set of series that matches the given label matchers. - Select(*SelectParams, ...*labels.Matcher) (SeriesSet, error, Warnings) + Select(*SelectParams, ...*labels.Matcher) (SeriesSet, Warnings, error) // LabelValues returns all potential values for a label name. LabelValues(name string) ([]string, error) diff --git a/storage/noop.go b/storage/noop.go index 8ff69acc2..1c1fed4fe 100644 --- a/storage/noop.go +++ b/storage/noop.go @@ -26,7 +26,7 @@ func NoopQuerier() Querier { return noopQuerier{} } -func (noopQuerier) Select(*SelectParams, ...*labels.Matcher) (SeriesSet, error, Warnings) { +func (noopQuerier) Select(*SelectParams, ...*labels.Matcher) (SeriesSet, Warnings, error) { return NoopSeriesSet(), nil, nil } diff --git a/storage/remote/read.go b/storage/remote/read.go index ead43def4..97f2390fb 100644 --- a/storage/remote/read.go +++ b/storage/remote/read.go @@ -59,10 +59,10 @@ type querier struct { // Select implements storage.Querier and uses the given matchers to read series // sets from the Client. -func (q *querier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { +func (q *querier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { query, err := ToQuery(q.mint, q.maxt, matchers, p) if err != nil { - return nil, err, nil + return nil, nil, err } remoteReadGauge := remoteReadQueries.WithLabelValues(q.client.Name()) @@ -71,7 +71,7 @@ func (q *querier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) ( res, err := q.client.Read(q.ctx, query) if err != nil { - return nil, err, nil + return nil, nil, err } return FromQueryResult(res), nil, nil @@ -117,13 +117,13 @@ type externalLabelsQuerier struct { // Select adds equality matchers for all external labels to the list of matchers // before calling the wrapped storage.Queryable. The added external labels are // removed from the returned series sets. -func (q externalLabelsQuerier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { +func (q externalLabelsQuerier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { m, added := q.addExternalLabels(matchers) - s, err, warnings := q.Querier.Select(p, m...) + s, warnings, err := q.Querier.Select(p, m...) if err != nil { - return nil, err, warnings + return nil, warnings, err } - return newSeriesSetFilter(s, added), nil, warnings + return newSeriesSetFilter(s, added), warnings, nil } // PreferLocalStorageFilter returns a QueryableFunc which creates a NoopQuerier @@ -170,7 +170,7 @@ type requiredMatchersQuerier struct { // Select returns a NoopSeriesSet if the given matchers don't match the label // set of the requiredMatchersQuerier. Otherwise it'll call the wrapped querier. -func (q requiredMatchersQuerier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { +func (q requiredMatchersQuerier) Select(p *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { ms := q.requiredMatchers for _, m := range matchers { for i, r := range ms { diff --git a/storage/remote/read_test.go b/storage/remote/read_test.go index 00e2c7f09..b09933bef 100644 --- a/storage/remote/read_test.go +++ b/storage/remote/read_test.go @@ -42,7 +42,7 @@ func TestExternalLabelsQuerierSelect(t *testing.T) { externalLabels: model.LabelSet{"region": "europe"}, } want := newSeriesSetFilter(mockSeriesSet{}, q.externalLabels) - have, err, _ := q.Select(nil, matchers...) + have, _, err := q.Select(nil, matchers...) if err != nil { t.Error(err) } @@ -157,7 +157,7 @@ type mockSeriesSet struct { storage.SeriesSet } -func (mockQuerier) Select(*storage.SelectParams, ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { +func (mockQuerier) Select(*storage.SelectParams, ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { return mockSeriesSet{}, nil, nil } @@ -313,7 +313,7 @@ func TestRequiredLabelsQuerierSelect(t *testing.T) { requiredMatchers: test.requiredMatchers, } - have, err, _ := q.Select(nil, test.matchers...) + have, _, err := q.Select(nil, test.matchers...) if err != nil { t.Error(err) } diff --git a/storage/tsdb/tsdb.go b/storage/tsdb/tsdb.go index 4dccf3d89..8dce7d512 100644 --- a/storage/tsdb/tsdb.go +++ b/storage/tsdb/tsdb.go @@ -230,7 +230,7 @@ type querier struct { q tsdb.Querier } -func (q querier) Select(_ *storage.SelectParams, oms ...*labels.Matcher) (storage.SeriesSet, error, storage.Warnings) { +func (q querier) Select(_ *storage.SelectParams, oms ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { ms := make([]tsdbLabels.Matcher, 0, len(oms)) for _, om := range oms { @@ -238,7 +238,7 @@ func (q querier) Select(_ *storage.SelectParams, oms ...*labels.Matcher) (storag } set, err := q.q.Select(ms...) if err != nil { - return nil, err, nil + return nil, nil, err } return seriesSet{set: set}, nil, nil } diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 0bd5096ff..0e602c1f3 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -488,7 +488,7 @@ func (api *API) series(r *http.Request) apiFuncResult { var sets []storage.SeriesSet var warnings storage.Warnings for _, mset := range matcherSets { - s, err, wrn := q.Select(nil, mset...) //TODO + s, wrn, err := q.Select(nil, mset...) //TODO warnings = append(warnings, wrn...) if err != nil { return apiFuncResult{nil, &apiError{errorExec, err}, warnings, nil} @@ -883,7 +883,7 @@ func (api *API) remoteRead(w http.ResponseWriter, r *http.Request) { } } - set, err, _ := querier.Select(selectParams, filteredMatchers...) + set, _, err := querier.Select(selectParams, filteredMatchers...) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index bdc6c2d64..316f3cd8e 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -379,7 +379,7 @@ func setupRemote(s storage.Storage) *httptest.Server { } defer querier.Close() - set, err, _ := querier.Select(selectParams, matchers...) + set, _, err := querier.Select(selectParams, matchers...) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/web/federate.go b/web/federate.go index 5dad7d414..b0d94a9d7 100644 --- a/web/federate.go +++ b/web/federate.go @@ -87,7 +87,7 @@ func (h *Handler) federation(w http.ResponseWriter, req *http.Request) { var sets []storage.SeriesSet for _, mset := range matcherSets { - s, err, wrns := q.Select(params, mset...) + s, wrns, err := q.Select(params, mset...) if wrns != nil { level.Debug(h.logger).Log("msg", "federation select returned warnings", "warnings", wrns) federationErrors.Add(float64(len(wrns)))