Skip to content
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

sidecar: time limit requests to Prometheus remote read api #1267

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions cmd/thanos/sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ func registerSidecar(m map[string]setupFunc, app *kingpin.Application, name stri
promURL := cmd.Flag("prometheus.url", "URL at which to reach Prometheus's API. For better performance use local network.").
Default("http://localhost:9090").URL()

promRetention := modelDuration(cmd.Flag("prometheus.retention", "A limit on how much retention to query from Prometheus. 0d means query all TSDB data found on disk").Default("0d"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about this. Should we do retention? I would rather see storeapi.min-time with similar model to specify both relative and absolute time as here: #1077

This is to keep it consistent with potential store gateway time partitioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was what I struggled with most -- how to name and handle this argument. I'll take a look at implementing the suggestions. I'm all about consistency.

Copy link
Member

@povilasv povilasv Aug 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yy, if we are doing this then --storeapi.min-time, --storeapi.max-time sounds like good options.

You can copy paste the https://github.com/thanos-io/thanos/pull/1077/files#diff-dd29e6298d43e46bb651035051819cfcR14 class from my PR, to make it take same exact format.

And I will merge once I find time to finish my PR


dataDir := cmd.Flag("tsdb.path", "Data directory of TSDB.").
Default("./data").String()

Expand Down Expand Up @@ -71,6 +73,7 @@ func registerSidecar(m map[string]setupFunc, app *kingpin.Application, name stri
*clientCA,
*httpBindAddr,
*promURL,
time.Duration(*promRetention),
*dataDir,
objStoreConfig,
rl,
Expand All @@ -90,6 +93,7 @@ func runSidecar(
clientCA string,
httpBindAddr string,
promURL *url.URL,
promRetention time.Duration,
dataDir string,
objStoreConfig *pathOrContent,
reloader *reloader.Reloader,
Expand All @@ -100,8 +104,9 @@ func runSidecar(

// Start out with the full time range. The shipper will constrain it later.
// TODO(fabxc): minimum timestamp is never adjusted if shipping is disabled.
mint: 0,
maxt: math.MaxInt64,
mint: 0,
maxt: math.MaxInt64,
limit: promRetention,
}

confContentYaml, err := objStoreConfig.Content()
Expand Down Expand Up @@ -318,6 +323,7 @@ type promMetadata struct {
mtx sync.Mutex
mint int64
maxt int64
limit time.Duration
labels labels.Labels
}

Expand All @@ -337,8 +343,21 @@ func (s *promMetadata) UpdateLabels(ctx context.Context, logger log.Logger) erro
func (s *promMetadata) UpdateTimestamps(mint int64, maxt int64) {
s.mtx.Lock()
defer s.mtx.Unlock()
var limitt int64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var limitt int64
var limit int64


// Calculate our limit on how far back in time we query Prometheus.
// Long term retention should be accessible via a Store component,
// therefore, its possible we can ignore anything older than a few days on
// the local Prometheus. This is disabled if limit == 0.
if s.limit > 0 {
limitt = int64(model.Now().Add(-s.limit))
}

s.mint = mint
if mint < limitt {
s.mint = limitt
} else {
s.mint = mint
}
s.maxt = maxt
}

Expand Down
3 changes: 3 additions & 0 deletions docs/components/sidecar.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ Flags:
--prometheus.url=http://localhost:9090
URL at which to reach Prometheus's API. For
better performance use local network.
--prometheus.retention=0d A limit on how much retention to query from
Prometheus. 0d means query all TSDB data found
on disk
--tsdb.path="./data" Data directory of TSDB.
--reloader.config-file="" Config file watched by the reloader.
--reloader.config-envsubst-file=""
Expand Down
8 changes: 8 additions & 0 deletions pkg/store/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ func (p *PrometheusStore) Series(r *storepb.SeriesRequest, s storepb.Store_Serie
return nil
}
q := prompb.Query{StartTimestampMs: r.MinTime, EndTimestampMs: r.MaxTime}
// Check to see if we can limit the mint/maxt for a more efficient query.
mint, maxt := p.timestamps()
if mint > q.StartTimestampMs {
q.StartTimestampMs = mint
}
if maxt < q.EndTimestampMs {
q.EndTimestampMs = maxt
}

// TODO(fabxc): import common definitions from prompb once we have a stable gRPC
// query API there.
Expand Down
31 changes: 26 additions & 5 deletions pkg/store/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ func testPrometheusStoreSeriesE2e(t *testing.T, prefix string) {
proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar,
func() labels.Labels {
return labels.FromStrings("region", "eu-west")
}, nil)
},
func() (int64, int64) {
return baseT, math.MaxInt64
},
)
testutil.Ok(t, err)

// Query all three samples except for the first one. Since we round up queried data
Expand Down Expand Up @@ -136,7 +140,11 @@ func TestPrometheusStore_LabelValues_e2e(t *testing.T) {
u, err := url.Parse(fmt.Sprintf("http://%s", p.Addr()))
testutil.Ok(t, err)

proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar, getExternalLabels, nil)
proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar, getExternalLabels,
func() (int64, int64) {
return 0, math.MaxInt64
},
)
testutil.Ok(t, err)

resp, err := proxy.LabelValues(ctx, &storepb.LabelValuesRequest{
Expand Down Expand Up @@ -170,7 +178,11 @@ func TestPrometheusStore_ExternalLabelValues_e2e(t *testing.T) {
u, err := url.Parse(fmt.Sprintf("http://%s", p.Addr()))
testutil.Ok(t, err)

proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar, getExternalLabels, nil)
proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar, getExternalLabels,
func() (int64, int64) {
return 0, math.MaxInt64
},
)
testutil.Ok(t, err)

resp, err := proxy.LabelValues(ctx, &storepb.LabelValuesRequest{
Expand Down Expand Up @@ -217,7 +229,11 @@ func TestPrometheusStore_Series_MatchExternalLabel_e2e(t *testing.T) {
proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar,
func() labels.Labels {
return labels.FromStrings("region", "eu-west")
}, nil)
},
func() (int64, int64) {
return baseT, math.MaxInt64
},
)
testutil.Ok(t, err)
srv := newStoreSeriesServer(ctx)

Expand Down Expand Up @@ -345,7 +361,12 @@ func TestPrometheusStore_Series_SplitSamplesIntoChunksWithMaxSizeOfUint16_e2e(t
proxy, err := NewPrometheusStore(nil, nil, u, component.Sidecar,
func() labels.Labels {
return labels.FromStrings("region", "eu-west")
}, nil)
},
func() (int64, int64) {
return 0, math.MaxInt64
},
)

testutil.Ok(t, err)

return proxy
Expand Down