Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Commit 856e477

Browse files
JamesGuthrieJamesGuthrie
committed
Refactor out RemoteReadQuerier interface
The Querier interface was a weird mixture of "method to query" and "methods to get things with which to query". This change moves "method to query" into its own interface.
1 parent b1c5c08 commit 856e477

File tree

13 files changed

+99
-58
lines changed

13 files changed

+99
-58
lines changed

‎pkg/api/query_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ func (ms mockQuerier) Select(int64, int64, bool, *storage.SelectHints, *querier.
7070
return &mockSeriesSet{err: ms.selectErr}, nil
7171
}
7272

73+
func (m mockQuerier) RemoteReadQuerier() querier.RemoteReadQuerier {
74+
return nil
75+
}
76+
7377
func (m mockQuerier) ExemplarsQuerier(_ context.Context) querier.ExemplarQuerier {
7478
return mockExemplarQuerier{}
7579
}

‎pkg/pgclient/client.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,10 @@ func (c *Client) Read(req *prompb.ReadRequest) (*prompb.ReadResponse, error) {
216216
Results: make([]*prompb.QueryResult, len(req.Queries)),
217217
}
218218

219+
qr := c.querier.RemoteReadQuerier()
220+
219221
for i, q := range req.Queries {
220-
tts, err := c.querier.Query(q)
222+
tts, err := qr.Query(q)
221223
if err != nil {
222224
return nil, err
223225
}

‎pkg/pgclient/client_test.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,19 @@ func (q mockSamplesQuerier) Select(mint int64, maxt int64, sortSeries bool, hint
3636
return nil, nil
3737
}
3838

39-
func (q *mockQuerier) Query(*prompb.Query) ([]*prompb.TimeSeries, error) {
39+
func (q *mockQuerier) RemoteReadQuerier() querier.RemoteReadQuerier {
40+
return mockRemoteReadQuerier{
41+
tts: q.tts,
42+
err: q.err,
43+
}
44+
}
45+
46+
type mockRemoteReadQuerier struct {
47+
tts []*prompb.TimeSeries
48+
err error
49+
}
50+
51+
func (q mockRemoteReadQuerier) Query(_ *prompb.Query) ([]*prompb.TimeSeries, error) {
4052
return q.tts, q.err
4153
}
4254

‎pkg/pgmodel/querier/interface.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,24 @@ type SeriesSet interface {
2222
Close()
2323
}
2424

25-
// Querier queries the data using the provided query data and returns the
26-
// matching timeseries.
25+
// Querier provides access to the three query methods: remote read, samples,
26+
// and exemplars.
2727
type Querier interface {
28-
// Query returns resulting timeseries for a query.
29-
Query(*prompb.Query) ([]*prompb.TimeSeries, error)
28+
// RemoteReadQuerier returns a remote storage querier
29+
RemoteReadQuerier() RemoteReadQuerier
3030
// SamplesQuerier returns a sample querier.
3131
SamplesQuerier() SamplesQuerier
3232
// ExemplarsQuerier returns an exemplar querier.
3333
ExemplarsQuerier(ctx context.Context) ExemplarQuerier
3434
}
3535

36+
// RemoteReadQuerier queries the data using the provided query data and returns
37+
// the matching timeseries.
38+
type RemoteReadQuerier interface {
39+
// Query returns resulting timeseries for a query.
40+
Query(*prompb.Query) ([]*prompb.TimeSeries, error)
41+
}
42+
3643
// SamplesQuerier queries data using the provided query data and returns the
3744
// matching samples.
3845
type SamplesQuerier interface {

‎pkg/pgmodel/querier/querier.go

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,12 @@ package querier
66

77
import (
88
"context"
9-
"fmt"
109

1110
"github.com/prometheus/prometheus/model/labels"
1211
"github.com/prometheus/prometheus/storage"
1312
"github.com/timescale/promscale/pkg/pgmodel/cache"
1413
"github.com/timescale/promscale/pkg/pgmodel/lreader"
1514
"github.com/timescale/promscale/pkg/pgxconn"
16-
"github.com/timescale/promscale/pkg/prompb"
1715
"github.com/timescale/promscale/pkg/tenancy"
1816
)
1917

@@ -44,6 +42,10 @@ func NewQuerier(
4442
return querier
4543
}
4644

45+
func (q *pgxQuerier) RemoteReadQuerier() RemoteReadQuerier {
46+
return newQueryRemoteRead(q)
47+
}
48+
4749
func (q *pgxQuerier) SamplesQuerier() SamplesQuerier {
4850
return newQuerySamples(q)
4951
}
@@ -52,30 +54,6 @@ func (q *pgxQuerier) ExemplarsQuerier(ctx context.Context) ExemplarQuerier {
5254
return newQueryExemplars(q)
5355
}
5456

55-
// Query implements the Querier interface. It is the entry point for
56-
// remote-storage queries.
57-
func (q *pgxQuerier) Query(query *prompb.Query) ([]*prompb.TimeSeries, error) {
58-
if query == nil {
59-
return []*prompb.TimeSeries{}, nil
60-
}
61-
62-
matchers, err := fromLabelMatchers(query.Matchers)
63-
if err != nil {
64-
return nil, err
65-
}
66-
67-
qrySamples := newQuerySamples(q)
68-
sampleRows, _, err := qrySamples.fetchSamplesRows(query.StartTimestampMs, query.EndTimestampMs, nil, nil, nil, matchers)
69-
if err != nil {
70-
return nil, err
71-
}
72-
results, err := buildTimeSeries(sampleRows, q.tools.labelsReader)
73-
if err != nil {
74-
return nil, fmt.Errorf("building time-series: %w", err)
75-
}
76-
return results, nil
77-
}
78-
7957
// errorSeriesSet represents an error result in a form of a series set.
8058
// This behavior is inherited from Prometheus codebase.
8159
type errorSeriesSet struct {

‎pkg/pgmodel/querier/querier_sql_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,7 @@ func TestPGXQuerierQuery(t *testing.T) {
722722
}
723723
querier := pgxQuerier{&queryTools{conn: mock, metricTableNames: mockMetrics, labelsReader: lreader.NewLabelsReader(mock, clockcache.WithMax(0))}}
724724

725-
result, err := querier.Query(c.query)
725+
result, err := querier.RemoteReadQuerier().Query(c.query)
726726

727727
if err != nil {
728728
switch {

‎pkg/pgmodel/querier/query_exemplar.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,8 @@ func (q *queryExemplars) Select(start, end time.Time, matchersList ...[]*labels.
8484
return results, nil
8585
}
8686

87-
// fetchSingleMetricSamples returns all the result rows for a single metric using the
88-
// query metadata and the tools. It uses the hints and node path to try to push
89-
// down query functions where possible.
87+
// fetchSingleMetricExemplars returns all exemplar rows for a single metric
88+
// using the query metadata and tools.
9089
func fetchSingleMetricExemplars(tools *queryTools, metadata *evalMetadata) ([]exemplarSeriesRow, error) {
9190
sqlQuery := buildSingleMetricExemplarsQuery(metadata)
9291

@@ -107,7 +106,7 @@ func fetchSingleMetricExemplars(tools *queryTools, metadata *evalMetadata) ([]ex
107106
return exemplarSeriesRows, nil
108107
}
109108

110-
// queryMultipleMetrics returns all the result rows for across multiple metrics
109+
// fetchMultipleMetricsExemplars returns all the result rows across multiple metrics
111110
// using the supplied query parameters.
112111
func fetchMultipleMetricsExemplars(tools *queryTools, metadata *evalMetadata) ([]exemplarSeriesRow, error) {
113112
// First fetch series IDs per metric.

‎pkg/pgmodel/querier/query_remote_read.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package querier
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/timescale/promscale/pkg/prompb"
7+
)
8+
9+
type queryRemoteRead struct {
10+
*pgxQuerier
11+
}
12+
13+
func newQueryRemoteRead(qr *pgxQuerier) *queryRemoteRead {
14+
return &queryRemoteRead{qr}
15+
}
16+
17+
// Query implements the RemoteReadQuerier interface. It is the entrypoint for
18+
// remote read queries.
19+
func (q *queryRemoteRead) Query(query *prompb.Query) ([]*prompb.TimeSeries, error) {
20+
if query == nil {
21+
return []*prompb.TimeSeries{}, nil
22+
}
23+
24+
matchers, err := fromLabelMatchers(query.Matchers)
25+
if err != nil {
26+
return nil, err
27+
}
28+
29+
qrySamples := newQuerySamples(q.pgxQuerier)
30+
sampleRows, _, err := qrySamples.fetchSamplesRows(query.StartTimestampMs, query.EndTimestampMs, nil, nil, nil, matchers)
31+
if err != nil {
32+
return nil, err
33+
}
34+
results, err := buildTimeSeries(sampleRows, q.tools.labelsReader)
35+
if err != nil {
36+
return nil, fmt.Errorf("building time-series: %w", err)
37+
}
38+
return results, nil
39+
}

‎pkg/pgmodel/querier/query_sample.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func newQuerySamples(qr *pgxQuerier) *querySamples {
2121
return &querySamples{qr}
2222
}
2323

24-
// Select implements the Querier interface. It is the entry point for our
24+
// Select implements the SamplesQuerier interface. It is the entry point for our
2525
// own version of the Prometheus engine.
2626
func (q *querySamples) Select(mint, maxt int64, _ bool, hints *storage.SelectHints, qh *QueryHints, path []parser.Node, ms ...*labels.Matcher) (seriesSet SeriesSet, node parser.Node) {
2727
sampleRows, topNode, err := q.fetchSamplesRows(mint, maxt, hints, qh, path, ms)
@@ -112,8 +112,8 @@ func fetchSingleMetricSamples(tools *queryTools, metadata *evalMetadata) ([]samp
112112
return samplesRows, topNode, nil
113113
}
114114

115-
// queryMultipleMetrics returns all the result rows for across multiple metrics
116-
// using the supplied query parameters.
115+
// fetchMultipleMetricsSamples returns all the result rows for across multiple
116+
// metrics using the supplied query parameters.
117117
func fetchMultipleMetricsSamples(tools *queryTools, metadata *evalMetadata) ([]sampleRow, error) {
118118
// First fetch series IDs per metric.
119119
metrics, schemas, series, err := GetMetricNameSeriesIds(tools.conn, metadata)

‎pkg/tests/end_to_end_tests/multi_tenancy_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func TestMultiTenancyWithoutValidTenants(t *testing.T) {
7272
},
7373
}
7474

75-
result, err := qr.Query(&prompb.Query{
75+
result, err := qr.RemoteReadQuerier().Query(&prompb.Query{
7676
Matchers: []*prompb.LabelMatcher{
7777
{
7878
Type: prompb.LabelMatcher_EQ,
@@ -123,7 +123,7 @@ func TestMultiTenancyWithoutValidTenants(t *testing.T) {
123123
},
124124
}
125125

126-
result, err = qr.Query(&prompb.Query{
126+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
127127
Matchers: []*prompb.LabelMatcher{
128128
{
129129
Type: prompb.LabelMatcher_EQ,
@@ -187,7 +187,7 @@ func TestMultiTenancyWithoutValidTenants(t *testing.T) {
187187
},
188188
}
189189

190-
result, err = qr.Query(&prompb.Query{
190+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
191191
Matchers: []*prompb.LabelMatcher{
192192
{
193193
Type: prompb.LabelMatcher_EQ,
@@ -263,7 +263,7 @@ func TestMultiTenancyWithValidTenants(t *testing.T) {
263263
},
264264
},
265265
}
266-
result, err := qr.Query(&prompb.Query{
266+
result, err := qr.RemoteReadQuerier().Query(&prompb.Query{
267267
Matchers: []*prompb.LabelMatcher{
268268
{
269269
Type: prompb.LabelMatcher_EQ,
@@ -287,7 +287,7 @@ func TestMultiTenancyWithValidTenants(t *testing.T) {
287287
// ----- query-test: querying an invalid tenant (tenant-c) -----
288288
expectedResult = []prompb.TimeSeries{}
289289

290-
result, err = qr.Query(&prompb.Query{
290+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
291291
Matchers: []*prompb.LabelMatcher{
292292
{
293293
Type: prompb.LabelMatcher_EQ,
@@ -338,7 +338,7 @@ func TestMultiTenancyWithValidTenants(t *testing.T) {
338338
},
339339
}
340340

341-
result, err = qr.Query(&prompb.Query{
341+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
342342
Matchers: []*prompb.LabelMatcher{
343343
{
344344
Type: prompb.LabelMatcher_EQ,
@@ -372,7 +372,7 @@ func TestMultiTenancyWithValidTenants(t *testing.T) {
372372

373373
expectedResult = []prompb.TimeSeries{}
374374

375-
result, err = qr.Query(&prompb.Query{
375+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
376376
Matchers: []*prompb.LabelMatcher{
377377
{
378378
Type: prompb.LabelMatcher_RE,
@@ -464,7 +464,7 @@ func TestMultiTenancyWithValidTenantsAndNonTenantOps(t *testing.T) {
464464
},
465465
}
466466

467-
result, err := qr.Query(&prompb.Query{
467+
result, err := qr.RemoteReadQuerier().Query(&prompb.Query{
468468
Matchers: []*prompb.LabelMatcher{
469469
{
470470
Type: prompb.LabelMatcher_EQ,
@@ -510,7 +510,7 @@ func TestMultiTenancyWithValidTenantsAndNonTenantOps(t *testing.T) {
510510
},
511511
}
512512

513-
result, err = qr.Query(&prompb.Query{
513+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
514514
Matchers: []*prompb.LabelMatcher{
515515
{
516516
Type: prompb.LabelMatcher_EQ,
@@ -564,7 +564,7 @@ func TestMultiTenancyWithValidTenantsAndNonTenantOps(t *testing.T) {
564564
},
565565
}
566566

567-
result, err = qr.Query(&prompb.Query{
567+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
568568
Matchers: []*prompb.LabelMatcher{
569569
{
570570
Type: prompb.LabelMatcher_EQ,
@@ -581,7 +581,7 @@ func TestMultiTenancyWithValidTenantsAndNonTenantOps(t *testing.T) {
581581
verifyResults(t, expectedResult, result)
582582

583583
expectedResult = []prompb.TimeSeries{}
584-
result, err = qr.Query(&prompb.Query{
584+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
585585
Matchers: []*prompb.LabelMatcher{
586586
{
587587
Type: prompb.LabelMatcher_EQ,
@@ -658,7 +658,7 @@ func TestMultiTenancyWithValidTenantsAsLabels(t *testing.T) {
658658
},
659659
}
660660

661-
result, err := qr.Query(&prompb.Query{
661+
result, err := qr.RemoteReadQuerier().Query(&prompb.Query{
662662
Matchers: []*prompb.LabelMatcher{
663663
{
664664
Type: prompb.LabelMatcher_EQ,
@@ -709,7 +709,7 @@ func TestMultiTenancyWithValidTenantsAsLabels(t *testing.T) {
709709
},
710710
}
711711

712-
result, err = qr.Query(&prompb.Query{
712+
result, err = qr.RemoteReadQuerier().Query(&prompb.Query{
713713
Matchers: []*prompb.LabelMatcher{
714714
{
715715
Type: prompb.LabelMatcher_EQ,

0 commit comments

Comments
 (0)