Reuse iterators #1574
Merged
Reuse iterators #1574
Conversation
This file contains hidden or 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
added 3 commits
August 7, 2019 17:35
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
|
This is the benchmark results of existing benchmark. Mostly better for the And this is for a 650MB tarball chunk for a query as described in this comment prometheus/prometheus#5707 (comment) |
codesome
changed the title
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
|
Updated numbers for BigChunk after last few changes |
|
Also, the allocation improvements from the tarball is not merely the query but also reading the tarball (but is that also a part of the query? I haven't checked yet) |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
changed the title
|
I have updated the PR to use Prometheus 2.12.0. Hence now we no longer vendor tsdb and instead use the tsdb inside prometheus. And, as I had done before, this prometheus vendoring is from this branch https://github.com/prometheus/prometheus/tree/remove-alertmanager, which is just a commit to remove altermanager on top of 2.12.0. |
added 4 commits
August 19, 2019 21:37
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
|
Is this replaced by #1594? Or you will merge one and rebase the other one? |
|
(I assume you mean #1597) I plan to get the other vendoring PR merged and rebase this one to only include the reuse of iterators. So this can be considered still WIP. |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
changed the title
|
After #1597 was merged I have rebased this PR and is ready for review. |
bboreham
reviewed
Sep 10, 2019
| @@ -34,26 +34,21 @@ github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV | |||
| github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= | |||
| github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= | |||
| github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= | |||
| github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY= | |||
There was a problem hiding this comment.
why all these changes in go.sum?
There was a problem hiding this comment.
Apparently there were 2 versions of the same module in go.sum. I just did go mod tidy and all those old versions were removed. I wonder why it was not removed in the vendoring PR.
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
tomwilkie
approved these changes
Sep 11, 2019
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 includes prometheus-junkyard/tsdb#642. I have added similar re-use of iterators here. There is a possibility of allocs improvements in ingester push and I am trying to realize that.
I will post the benchmark results soon.
Update: allocs improvement in push path is not happening. Not attempting anymore in this PR.