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

Avoid parsing labels when tailer is sending from a stream. #2973

Merged
merged 2 commits into from
Nov 24, 2020

Conversation

cyriltovena
Copy link
Contributor

This also include some code cleanup.

Signed-off-by: Cyril Tovena cyril.tovena@gmail.com

This also include some code cleanup.

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

Codecov Report

Merging #2973 (84fa498) into master (8ea5fd1) will decrease coverage by 0.05%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2973      +/-   ##
==========================================
- Coverage   61.84%   61.79%   -0.06%     
==========================================
  Files         182      182              
  Lines       14870    14863       -7     
==========================================
- Hits         9197     9184      -13     
- Misses       4824     4834      +10     
+ Partials      849      845       -4     
Impacted Files Coverage Δ
pkg/ingester/stream.go 68.96% <0.00%> (+0.58%) ⬆️
pkg/ingester/tailer.go 41.32% <53.84%> (+0.37%) ⬆️
pkg/promtail/positions/positions.go 46.80% <0.00%> (-11.71%) ⬇️

Copy link
Collaborator

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Great addition, nice alloc improvement on streams too.

@owen-d owen-d merged commit 9807f7c into grafana:master Nov 24, 2020
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
* Move regex opt to after lookup

fixes grafana#2906

When we use the caching client (which is what is used in most cases), we
load the entire row (tableName+HashKey) irrespective of what the
rangeKey parameters are. Which means with the optimisation, we are
loading the same row multiple times and then operating on the same data.
This PR moves the optimisation to after the data is loaded which should
be faster.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Add benchmark

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Add changelog entry

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* Address feedback

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

Co-authored-by: Peter Štibraný <peter.stibrany@grafana.com>
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

4 participants