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

Logqv2 optimization #2778

Merged
merged 56 commits into from
Oct 21, 2020
Merged

Conversation

cyriltovena
Copy link
Contributor

benchmark before:

pkg: github.com/grafana/loki/pkg/logql/log
Benchmark_Pipeline
Benchmark_Pipeline-16    	  107580	     10271 ns/op	    6387 B/op	      67 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/logql/log	1.228s

after

pkg: github.com/grafana/loki/pkg/logql/log
Benchmark_Pipeline
Benchmark_Pipeline-16    	  199170	      5670 ns/op	    3307 B/op	      54 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/logql/log	1.201s

Way less allocations !

cyriltovena and others added 30 commits October 1, 2020 20:38
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Also add duration convertion for unwrap.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
The auth middleware was happening after the stats one and so org_id was not set 🤦.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
This patch extends the duration label filter with support for byte sizes
such as `1kB` and `42MiB`.
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
We should never advance an iterator in parallel. Unfortunately before the code was building iterators while advancing previous one, building iterator can advance iterator and thus creates a race condition. This changeset make sure we only fetch chunks in advance and build iterator and iterate over them in sequence.

Also add support for labels in the cacheIterator which is required for logqlv2.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
This could cause Loki to crash if a panic happens in the store since it was happening in another goroutine.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
benchmark before:

```
pkg: github.com/grafana/loki/pkg/logql/log
Benchmark_Pipeline
Benchmark_Pipeline-16    	  107580	     10271 ns/op	    6387 B/op	      67 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/logql/log	1.228s
```

after
```
pkg: github.com/grafana/loki/pkg/logql/log
Benchmark_Pipeline
Benchmark_Pipeline-16    	  199170	      5670 ns/op	    3307 B/op	      54 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/logql/log	1.201s
```

Way less allocations !

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@codecov-io
Copy link

codecov-io commented Oct 19, 2020

Codecov Report

Merging #2778 into master will increase coverage by 0.04%.
The diff coverage is 66.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2778      +/-   ##
==========================================
+ Coverage   61.15%   61.19%   +0.04%     
==========================================
  Files         177      177              
  Lines       14278    14325      +47     
==========================================
+ Hits         8731     8766      +35     
- Misses       4737     4746       +9     
- Partials      810      813       +3     
Impacted Files Coverage Δ
pkg/logql/log/filter.go 73.71% <0.00%> (ø)
pkg/logql/log/pipeline.go 13.63% <11.11%> (-2.16%) ⬇️
pkg/logql/log/metrics_extraction.go 51.66% <37.50%> (+0.81%) ⬆️
pkg/logql/log/label_filter.go 54.16% <38.88%> (+1.71%) ⬆️
pkg/logql/functions.go 19.26% <40.00%> (ø)
pkg/logql/log/labels.go 93.47% <93.47%> (-6.53%) ⬇️
pkg/logql/ast.go 81.79% <100.00%> (ø)
pkg/logql/log/fmt.go 77.41% <100.00%> (+1.98%) ⬆️
pkg/logql/log/parser.go 91.66% <100.00%> (-0.44%) ⬇️
pkg/querier/queryrange/downstreamer.go 95.29% <0.00%> (-2.36%) ⬇️
... and 2 more

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

Few nits then LGTM

pkg/logql/log/labels.go Outdated Show resolved Hide resolved
pkg/logql/log/labels.go Show resolved Hide resolved
Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

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

LGTM for 4am 😉

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena cyriltovena merged commit dd5fceb into grafana:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants