Support Custom Timeout for Queries

[gouthamve]

This is a WIP, tests are yet to be added. Just wanted to get feedback if this was the right approach.

Some tests I added were failing as a lot of time (>30secs) was being spent here: https://github.com/prometheus/prometheus/blob/master/storage/local/storage.go#L547 and context was ignored there. Any reason for ignoring context?

Closes #1399

[fabxc]

Generally contexts are per request. The timeout the query engine wraps the provided context with is merely a upper bound applied to guard against too-high user-defined timeouts. Whether this should be set in the query engine or rather the client boundary (e.g. API or rule evaluation code) is a different question :)

For the introduction of custom timeouts though, this should not happen in the query engine itself but rather by the caller setting a timeout on the context passed to Query.Exec(ctx).
That would make things a bit simpler too, as we'd not have to change anything in the usage of templating and rule evaluation.

[gouthamve]

// WithTimeout returns a copy of parent whose Done channel is closed as soon as
// parent.Done is closed, cancel is called, or timeout elapses. The new
// Context's Deadline is the sooner of now+timeout and the parent's deadline, if
// any. If the timer is still running, the cancel function releases its
// resources.

Did not know that before! Will make a lot of things easy!

[gouthamve]

@fabxc Fixed it.

But, I find sometimes that I am stuck here: https://github.com/prometheus/prometheus/blob/master/storage/local/storage.go#L547 for more than 10mins when running a heavy server. Just trying to understand why we are not cancelling and cleaning up midway.

[fabxc]

Yes, that code path should respect cancelation as well. Just nobody got around to making use of the cancelation signal throughout that bit of code yet.

[fabxc]

Code looks good in general. @brian-brazil final concerns on HTTP API extension?

[fabxc]

1 -> one

[fabxc]

"Alertmanagers" is how we write it.
Sentence should end with period.

[fabxc]

1 -> one, terminate with period.

[brian-brazil]

Seems fine from an API standpoint.

I've been considering having scrapes pass down their timeout via a header, and remote read could do the same, but I think those APIs will necessarily be different.

[fabxc]

Okay, this one seems good to go then. 👍