Cache label strings in ingester to improve memory usage. #2926
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
This reduce allocations by a lot, but assume that labels comes in sorted which is not ensure in the distributors. The distributors will take a performance hit, but that's easier to scale or improve later.(added some todos) see benchmark: ``` ❯ benchcmp before.txt after.txt benchmark old ns/op new ns/op delta Benchmark_PushInstance-16 43505 4950 -88.62% benchmark old allocs new allocs delta Benchmark_PushInstance-16 240 12 -95.00% benchmark old bytes new bytes delta Benchmark_PushInstance-16 42568 1787 -95.80% ``` Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #2926 +/- ##
==========================================
- Coverage 61.72% 61.68% -0.05%
==========================================
Files 181 181
Lines 14724 14733 +9
==========================================
- Hits 9089 9088 -1
- Misses 4801 4813 +12
+ Partials 834 832 -2
|
cyriltovena
added a commit
to cyriltovena/loki
that referenced
this pull request
Nov 13, 2020
Also moving labels parsing into logql package, and I'm not shy to say that promql is faster for this. So we're using promql for parser, it's faster because their lexer is more sophisticated than ours. Result are impressive. ``` ❯ benchcmp before.txt after.txt benchmark old ns/op new ns/op delta Benchmark_ParseLabels-16 24394 8745 -64.15% benchmark old allocs new allocs delta Benchmark_ParseLabels-16 156 20 -87.18% benchmark old bytes new bytes delta Benchmark_ParseLabels-16 24976 2099 -91.60% ``` Combined with grafana#2926, I think we should be able to get rid of `util.ToClientLabels`. But that's for another PR. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
cyriltovena
added a commit
that referenced
this pull request
Nov 13, 2020
* Improve logql parser allocations. Also moving labels parsing into logql package, and I'm not shy to say that promql is faster for this. So we're using promql for parser, it's faster because their lexer is more sophisticated than ours. Result are impressive. ``` ❯ benchcmp before.txt after.txt benchmark old ns/op new ns/op delta Benchmark_ParseLabels-16 24394 8745 -64.15% benchmark old allocs new allocs delta Benchmark_ParseLabels-16 156 20 -87.18% benchmark old bytes new bytes delta Benchmark_ParseLabels-16 24976 2099 -91.60% ``` Combined with #2926, I think we should be able to get rid of `util.ToClientLabels`. But that's for another PR. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Fixes a test. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
cyriltovena
pushed a commit
to cyriltovena/loki
that referenced
this pull request
Jun 11, 2021
* retry processing of failed delete requests Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> * fix tests by adding a mode to mockstorage Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> * add changelog entry Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> * changes suggested from PR review Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces allocations in ingesters, but assume that labels comes in sorted which is now ensured in the distributors.
The distributors will take a performance hit, but that's easier to scale or improve later.(added some todos)
see benchmark: