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

Improves checkpointerWriter memory usage #3188

Merged
merged 7 commits into from
Jan 20, 2021

Conversation

cyriltovena
Copy link
Contributor

I've improve marshalling and pooling to reduce memory usage. I've also fixes a small mistake (1>>20 =0).

I've also added some more debug logs, I want to see how many each series weight and at what size do we actually flush them.

benchmp:

> benchcmp  before.txt after.txt4
benchmark                        old ns/op     new ns/op     delta
Benchmark_CheckpointWrite-16     5218516       1046717       -79.94%

benchmark                        old allocs     new allocs     delta
Benchmark_CheckpointWrite-16     165            220            +33.33%

benchmark                        old bytes     new bytes     delta
Benchmark_CheckpointWrite-16     31645483      23673         -99.93%

This will allows to find information about received size and total entries per tenant.

Example of a log from my dev testing:

```
level=debug ts=2021-01-15T11:16:21.735663076Z caller=http.go:67 org_id=3927 traceID=11c4774c6ec4bbf4 msg="push request parsed" path=/loki/api/v1/push content-type=application/x-protobuf body-size="11 kB" streams=5 entries=298 streamLabelsSize="1.9 kB" entriesSize="45 kB" totalSize="47 kB"
```

Of course this means we can use LogQL on this.

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>

// 1MB
if w.bufSize > 1>>20 {
if w.bufSize > 1<<20 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@owen-d :)

Copy link
Member

Choose a reason for hiding this comment

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

ughh

pkg/ingester/checkpoint_test.go Outdated Show resolved Hide resolved
pkg/ingester/checkpoint_test.go Show resolved Hide resolved
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #3188 (9e864f1) into master (40a637e) will decrease coverage by 0.05%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3188      +/-   ##
==========================================
- Coverage   63.33%   63.27%   -0.06%     
==========================================
  Files         191      191              
  Lines       16455    16462       +7     
==========================================
- Hits        10421    10417       -4     
- Misses       5089     5102      +13     
+ Partials      945      943       -2     
Impacted Files Coverage Δ
pkg/ingester/checkpoint.go 69.23% <92.85%> (+0.94%) ⬆️
pkg/promtail/positions/positions.go 46.80% <0.00%> (-11.71%) ⬇️
pkg/logql/evaluator.go 89.87% <0.00%> (-0.36%) ⬇️
pkg/querier/queryrange/limits.go 95.83% <0.00%> (+4.16%) ⬆️

Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

LGTM

if cap(buf) < size+1 {
buf = make([]byte, size+1)
}
_, err := m.MarshalTo(buf[1 : size+1])
Copy link
Member

Choose a reason for hiding this comment

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

@owen-d owen-d merged commit 0a81f70 into grafana:master Jan 20, 2021
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.

5 participants