cmd,query: bump promql-engine and make subqueries for thanos engine configurable #6648
Conversation
MichaHoffmann
force-pushed
the
mhoffm-bump-promql-engine-make-subqueries-configurable
branch
2 times, most recently
from
94233c0
to
e5ec239
Compare
…onfigurable Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
MichaHoffmann
force-pushed
the
mhoffm-bump-promql-engine-make-subqueries-configurable
branch
from
e5ec239
to
bf89e7d
Compare
| engine.Opts{ | ||
| EngineOpts: engineOpts, | ||
| Engine: promql.NewEngine(engineOpts), | ||
| EnableSubqueries: enableThanosEngineSubqueries, |
There was a problem hiding this comment.
Feel like it is an overkill to me to have a flag for it
There was a problem hiding this comment.
I added it by popular request to have this feature configurable ( for a short time ). Once this has proven stable enough we can probably remove the flag again.
There was a problem hiding this comment.
Having said that im fairly confident that it works like in upstream promql since it passes acceptance tests and fuzzing
There was a problem hiding this comment.
➕ maybe we can just enable subqueries? The more people exposed to this, the faster bugs will be fixed (if any)
There was a problem hiding this comment.
Sadly subqueries are super prevalent in our alerting pipeline; if it has a bug we would need to quickly roll it back. I would prefer the switch because it would be critical for us if this has an issue.
There was a problem hiding this comment.
Can we have it for a short time and we run a nightly build and once we are happy ill take out the switch again?
There was a problem hiding this comment.
I guess we could use prometheus engine in case of issues too, so maybe its not so bad.
There was a problem hiding this comment.
Consulted with a teammate; ill take out the flag in a couple hours!
|
Can we take a look at the failed unit test? https://app.circleci.com/pipelines/github/MichaHoffmann/thanos/79/workflows/475701d9-0860-424d-8f75-ccb828384599/jobs/83 |
Oh thats maybe related to the prometheus version bump and looks a bit scary |
Changes
Bumped promql-engine; fixed fallout from the bump; added a configuration flag for enabling subqueries in the thanos engine
Verification
Not tested yet.