`storage.Querier` interface not fitting for remote implementations

[juliusv]

The storage.Querier definition at:

https://github.com/prometheus/prometheus/blob/dev-2.0/storage/interface.go#L43-L53

...is not great for remote querier implementations. Remote querier implementations will want to eager-load all series data upon Select(), but some uses of Select() are only interested in the series metadata (such as /api/v1/series). In that case, eager-loading all the sample data would be very wasteful. Those users would need a way to tell the querier their future intentions or have a separate interface method that only provides metadata.

The concrete use case where this is hitting me right now is porting Cortex to Prometheus 2.0 packages. We reuse the Prometheus api/v1 package there to give us the Prometheus web API, but the API cannot tell the querier that we pass in whether it will only want to load series metadata or full sample data.

Would adding a separate method to storage.Querier for explicit metadata-only selection make sense? Or some other form of parametrization in Select()?

[juliusv]

@fabxc @gouthamve

[fabxc]

I think it's the right interface for the domain problem it is trying to solve. It can obviously be unfitting in the case you are describing though.

Just extending/mutating the core interface seems wrong. I would push this out into the instantiation of the storage.Querier, e.g. myRemoteStorage.MetadataQuerier() and myRemoteStorage.FullQuerier(). The former could just error when the sample data tries to be touched, or do it inefficiently anyway – implementers choice really.

What would need to be adapted then is the API package so that it doesn't take a full storage as a dependency but just the most basic methods to retrieve the relevant data. The amount of fan-in of entire components into the web handler is way to much anyway.

[juliusv]

Sounds good as well. Even better, NewAPI() already takes both a promql.Engine and a local.Storage, which will already allow me to do this without modifying the API package (by constructing the promql.Engine from a different querier).

So I'll take that approach. Thanks!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.