For a sufficiently complex expression, having to render
thousands of nodes for every subquery type error can take
a while - so don't do that.
Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
* Testify: move to require
Moving testify to require to fail tests early in case of errors.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* More moves
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Refactor test assertions
This pull request gets rid of assert.True where possible to use
fine-grained assertions.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Separate tests into:
** Aggregators
** Functions
** Operators
** Selectors
* Remove simple files and place tests into other files.
* Eliminate some of the _over_time tests since there are already plenty of
edge cases present in the functions.test file.
Relates to #740
Signed-off-by: Harold Dost <harolddost@gmail.com>
* tsdb: Added ChunkQueryable implementations to db; unified compactor, querier and fanout block iterating.
Chained to https://github.com/prometheus/prometheus/pull/7059
* NewMerge(Chunk)Querier now takies multiple primaries allowing tsdb DB code to use it.
* Added single SeriesEntry / ChunkEntry for all series implementations.
* Unified all vertical, and non vertical for compact and querying to single
merge series / chunk sets by reusing VerticalSeriesMergeFunc for overlapping algorithm (same logic as before)
* Added block (Base/Chunk/)Querier for block querying. We then use populateAndTomb(Base/Chunk/) to iterate over chunks or samples.
* Refactored endpoint tests and querier tests to include subtests.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Addressed comments from Brian and Beorn.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed snapshot test and added chunk iterator support for DBReadOnly.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed race when iterating over Ats first.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed tests.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed populate block tests.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed endpoints test.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed test.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Added test & fixed case of head open chunk.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed DBReadOnly tests and bug producing 1 sample chunks.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Added cases for partial block overlap for multiple full chunks.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Added extra tests for chunk meta after compaction.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed small vertical merge bug and added more tests for that.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Implement go leak test for promql
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Implement go leak test for Consul SD
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Implement go leak test in discovery manager
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* promql: Removed global and add ability to have better interval for subqueries if not specified
## Changes
* Refactored tests for better hints testing
* Added various TODO in places to enhance.
* Moved DefaultEvalInterval global to opts with func(rangeMillis int64) int64 function instead
Motivation: At Thanos we would love to have better control over the subqueries step/interval.
This is important to choose proper resolution. I think having proper step also does not harm for
Prometheus and remote read users. Especially on stateless querier we do not know evaluation interval
and in fact putting global can be wrong to assume for Prometheus even.
I think ideally we could try to have at least 3 samples within the range, the same
way Prometheus UI and Grafana assumes.
Anyway this interfaces allows to decide on promQL user basis.
Open question: Is taking parent interval a smart move?
Motivation for removing global: I spent 1h fighting with:
=== RUN TestEvaluations
TestEvaluations: promql_test.go:31: unexpected error: error evaluating query "absent_over_time(rate(nonexistant[5m])[5m:])" (line 687): unexpected error: runtime error: integer divide by zero
--- FAIL: TestEvaluations (0.32s)
FAIL
At the end I found that this fails on most of the versions including this master if you run this test alone. If run together with many
other tests it passes. This is due to SetDefaultEvaluationInterval(1 * time.Minute)
in test that is ran before TestEvaluations. Thanks to globals (:
Let's fix it by dropping this global.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Added issue links for TODOs.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Removed irrelevant changes.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Add errors and Warnings to SeriesSet
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Change Querier interface and refactor accordingly
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Refactor promql/engine to propagate warnings at eval stage
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Address review issues
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Make sure all the series from all Selects are pre-advanced
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Address review issues
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Separate merge series sets
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Clean
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Refactor merge querier failure handling
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Refactored and simplified fanout with improvements from incoming chunk iterator PRs.
* Secondary logic is hidden, instead of weird failed series set logic we had.
* Fanout is well commented
* Fanout closing record all errors
* MergeQuerier improved API (clearer)
* deferredGenericMergeSeriesSet is not needed as we return no samples anyway for failed series sets (next = false).
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fix formatting
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Fix CI issues
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Added final tests for error handling.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Addressed Brian's comments.
* Moved hints in populate to be allocated only when needed.
* Used sync.Once in secondary Querier to achieve all-or-nothing partial response logic.
* Select after first Next is done will panic.
NOTE: in lazySeriesSet in theory we could just panic, I think however we can
totally just return error, it will panic in expand anyway.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Utilize errWithWarnings
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Fix recently introduced expansion issue
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Add tests for secondary querier error handling
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Implement lazy merge
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Add name to test cases
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Reorganize
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Address review comments
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Address review comments
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Remove redundant warnings
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
* Fix rebase mistake
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Use go1.14 new hash/maphash to hash both RHS and LHS instead of XOR'ing
which has been resulting in hash collisions.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Refactor engine labelset signature generation, just use labels.Labels
instead of hashes.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Address review comments; function comments + store result of
lhs.String+rhs.String as key.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Replace all signatureFunc usage with signatureFuncString.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Make optimizations to labels String function and generation of rhs+lhs
as string in resultMetric.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Use separate string functions that don't use strconv just for engine
maps.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Use a byte invalid separator instead of quoting and have a buffer
attached to EvalNodeHelper instead of using a global pool in the labels
package.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Address review comments.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Address more review comments, labels has a function that now builds a
byte slice without turning it into a string.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* Use two different non-ascii hex codes as byte separators between labels
and between sets of labels when building bytes of a Labels struct.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
* We only need the 2nd byte invalid sep. at the beginning of a
labels.Bytes
Signed-off-by: Callum Styan <callumstyan@gmail.com>
time.Unix attaches the local timezone, which can then
leak out (e.g. in the alert json). While this is harmless,
we should be consistent.
Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
* PromQL: Fix lexer error handling
This fixes bugs in the handling of lexer errors that are only noticeable for users of the language server and caused https://github.com/prometheus-community/promql-langserver/issues/104 .
Signed-off-by: Tobias Guggenmos <tobias.guggenmos@uni-ulm.de>
* Add test for error position ranges
Signed-off-by: Tobias Guggenmos <tobias.guggenmos@uni-ulm.de>
This addresses fabxc's TODO.
More importantly, it now properly defers the
querier.Close(). Previously, if a panic happened after creation of the
querier within the populateSeries function, querier.Close() was never called.
The latter was responsible for #6977.
Signed-off-by: beorn7 <beorn@grafana.com>
This is technically BREAKING CHANGE, but it was like this from the beginning: I just notice that we rely in
Prometheus on remote read being sorted. This is because we use selected data from remote reads in MergeSeriesSet
which rely on sorting.
I found during work on https://github.com/prometheus/prometheus/pull/5882 that
we do so many repetitions because of this, for not good reason. I think
I found a good balance between convenience and readability with just one method.
Smaller the interface = better.
Also I don't know what TestSelectSorted was testing, but now it's testing sorting.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
This PR fixes the regression tests for the issue fixed in #6931 .
The reason for that is that all of the invalid queries that triggered the regression have become more or less valid syntax in #6933 (they might still fail typechecking).
Signed-off-by: Tobias Guggenmos <tobias.guggenmos@uni-ulm.de>
This is part of https://github.com/prometheus/prometheus/pull/5882 that can be done to simplify things.
All todos I added will be fixed in follow up PRs.
* querier.Querier, querier.Appender, querier.SeriesSet, and querier.Series interfaces merged
with storage interface.go. All imports that.
* querier.SeriesIterator replaced by chunkenc.Iterator
* Added chunkenc.Iterator.Seek method and tests for xor implementation (?)
* Since we properly handle SelectParams for Select methods I adjusted min max
based on that. This should help in terms of performance for queries with functions like offset.
* added Seek to deletedIterator and test.
* storage/tsdb was removed as it was only a unnecessary glue with incompatible structs.
No logic was changed, only different source of abstractions, so no need for benchmarks.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Make lookbackDelta a option of QueryEngine
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* julius' suggestion
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* remove trivial getter
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Assume lookback delta is always > 0
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* add debug log
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* don't expose loopback delta
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Specify that lookack delta is also used in federation
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Fix federation test
While we have added some logic to the promql engine to keep it backwards
compatible and have a 5 minute loopback by default, the web/ package is
likely to really be internal to Prometheus and we should not add the
same kind of heuritstics here.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* loopback delta: Fix debug log
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Since we use ActiveQueryTracker to check for concurrency in
d992c36b3a it does not make sense to keep
the MaxConcurrent value as an option of the PromQL engine.
This pull request removes it from the PromQL engine options, sets the
max concurrent metric to -1 if there is no active query tracker, and use
the value of the active query tracker otherwise.
It removes dead code and also will inform people who import the promql
package that we made that change, as it breaks the EngineOpts struct.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
The windows clock is sometime off by 25ms, and as precise as 15ms.
Let's give it more time to avoid flaky tests.
Fix#6672
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Move check for empty VectorSelector to typeChecking
* Move check for twice set metric name to typeChecking
* Make child of MatrixSelector a general Node
* rename checkType to checkAST
* Rename fail to addParseErr
* Remove trailing whitespace
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
Fixes#6649.
The crash is fixed here, was caused because some AST sanity checks were performed on the syntax tree while assembling it. In case of previous parsing errors this could lead to undefined behaviour.
The fix is to move the checks to the typechecking phase, which runs only when a syntax tree was assembled without there being parsing errors.
There are other places, where similiar checks are performed while assembling the syntax tree. It might be a good idea to move those to the typechecking phase, too. Should I do this in the same or a separate PR?
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* PromQL: Use a sync.Pool for the generated parser structure
The generated PromQL parser allocates a struct about 4kb in size on every run.
This puts a high load on the garbage collector.
To reduce that load, a sync.Pool is used to recycle these structures.
On small queries this makes parsing 2-3 times faster.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
During the PromQL parser rewrite there was some logic put in place that allowed switching between the non generated and the generated parser. Since the parser is now fully generated this is not needed anymore.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* Cleanup PromQL functions
The engine ensures, for Matrix functions, that functions are called with exactly one series at the time.
Therefore a lot of code can be inlined and we can directly assume the first element of the arguments exists and contains all the samples needed.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
The parser benchmarks called the `ParseMetric` function instead of the `ParseExpr` function, which resulted in parsing failing every time.
This means only the case of PromQL parser failure was benchmarked.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* Add parser method to produce errors messages about unexpected items
* PromQL: use parser.unexpected in generated parser
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* Add grammar for label_sets
* Parse label Sets using the generated parser
* Allow trailing commas for label sets and selectors
* Add test to trigger all possible error messages for label matchers
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
The most common format (used by go, gcc and clang) for compiler error positions seems to be
`filename:line:char:` or `line:char:` if the filename is unknown.
This PR adapts the PromQL parser to use this convention.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
This PR exports the list of supported PromQL functions and their signatures.
The reason for that is that the PromQL language server likes to use that list.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* promql: Allow injecting fake tokens into the generated parser
Yacc grammars do not support having multiple start symbols.
To work around that restriction, it is possible to inject fake tokens into the lexer stream,
as described here https://www.gnu.org/software/bison/manual/html_node/Multiple-start_002dsymbols.html .
This is part of the parser rewrite effort described in #6256.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
For yacc generated parsers there is the convention to capitalize the names of item types provided by the lexer, which makes it easy to distinct lexer tokens (capitalized) from nonterminal symbols (not capitalized) in language grammars.
This convention is also followed by the (non generated) go compiler (see https://golang.org/pkg/go/token/#Token).
Part of the parser rewrite described in #6256.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
This is the first step towards a generated lexer as described in #6256.
It adds methods to the parser struct, that make it implement the yyLexer interface required by a yacc generated parser, as described here: https://godoc.org/golang.org/x/tools/cmd/goyacc .
The yyLexer interface is implemented by the parser struct instead of the lexer struct for the following reasons:
* Both parsers have a lookahead that the lexer does not know about. This solution makes it possible to synchronize these lookaheads when switching parsers.
* The routines to handle parser errors are not accessible to the lexer.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* promql: Clean up parser struct
The parser struct used two have two somewhat misused fields:
peekCount int
token [3]item
By reading the code carefully one notices, that peekCount always has the value 0 or 1 and that only the first element of token is ever accessed.
To make this clearer, this commit replaces the token array with a single variable and the peekCount int with a boolean.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
Before this commit, the PromQL parser ran in two goroutines:
* The lexer goroutine that splits the input into tokens and sent them over a channel to
* the parser goroutine which produces the abstract syntax tree
The Problem with this approach is that the parser spends more time on goroutine creation
and syncronisation than on actual parsing.
This commit removes that concurrency and replaces the channel by a slice based buffer.
Benchmarks show that this makes the up to 7 times faster than before.
Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
* Include tsdb tool in builds (#6085) (#6089)
Add the tsdb tool to promu so that it's included in the release
tarballs.
Signed-off-by: Ben Kochie <superq@gmail.com>
* web/ui: fix for CVE-2019-10215 (#6098)
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
* cut 2.13 release (#6099)
Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com>
* Fix panic in ARM builds of Prometheus (#6110)
An extra sync.Pool was added during a refactor which caused some 64 bit,
atomically accessed variables to no longer be 64 bit aligned. By moving
all atomically accessed variables to the beginning of the struct they
are guaranteed to be 64 bit aligned.
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
* promql: fix potential panic in the query logger (#6094)
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
* Cut release 2.13.1 (#6145)
Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com>
* promql: Move tests to testutil
Signed-off-by: Alex Dzyoba <alex@dzyoba.com>
* promql: Match error type via errors.As in tests
Signed-off-by: Alex Dzyoba <alex@dzyoba.com>
* promql: Remove unused `expectedList` func from lex_test.go
Signed-off-by: Alex Dzyoba <alex@dzyoba.com>
