Store-gateway: add support to lazy mmap index-headers #3431
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
pracucci
force-pushed
the
lazy-index-header-reader
branch
from
84ab39d
to
e570fd6
Compare
|
OK @bwplotka. I've manually tested it and, so far, I haven't noticed any issue. I think it's ready for a review. |
|
We've tested it on a Cortex cluster where each store-gateway has >65K blocks and we can finally run it without hitting in the max mmap areas kernel limit. It's still a bit slow to startup, but that's another story.
|
|
Indexes loaded (green) and indexes unloaded (yellow) with a 15m idle timeout.
|
bwplotka
approved these changes
Nov 12, 2020
| lazyIndexReaderEnabled := cmd.Flag("store.enable-index-header-lazy-reader", "If true, Store Gateway will lazy memory map index-header only once required by a query."). | ||
| Hidden().Default("false").Bool() | ||
|
|
||
| lazyIndexReaderIdleTimeout := cmd.Flag("store.index-header-lazy-reader-idle-timeout", "If index-header lazy reader is enabled and this idle timeout setting is > 0, memory map-ed index-headers will be automatically released after 'idle timeout' inactivity."). |
There was a problem hiding this comment.
This can stay hidden, maybe not sure if needed to be tweaked (less flags = simler to use thing is )
There was a problem hiding this comment.
I would suggest to keep the hidden flag, to allow all of us to experiment with that and find a better idle timeout. I just put a "random" value so far.
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pracucci
force-pushed
the
lazy-index-header-reader
branch
from
ad30aab
to
bc167bd
Compare
Signed-off-by: Marco Pracucci <marco@pracucci.com>
|
@bwplotka Thanks for the review! I've address your comments or commented otherwise. Could you take another look, please? |
Signed-off-by: Marco Pracucci <marco@pracucci.com>
3 tasks
Oghenebrume50
pushed a commit
to Oghenebrume50/thanos
that referenced
this pull request
Dec 7, 2020
* Experimental lazy index-header reader Signed-off-by: Marco Pracucci <marco@pracucci.com> * Implemented lazy index-header reader Signed-off-by: Marco Pracucci <marco@pracucci.com> * Renamed CLI flags Signed-off-by: Marco Pracucci <marco@pracucci.com> * Added copyright to new files Signed-off-by: Marco Pracucci <marco@pracucci.com> * Track metrics in the lazy index-header reader Signed-off-by: Marco Pracucci <marco@pracucci.com> * Ensure BucketStore.Close() is called in tests Signed-off-by: Marco Pracucci <marco@pracucci.com> * Ensure BucketStore.Close() is called in e2e tests too Signed-off-by: Marco Pracucci <marco@pracucci.com> * Shorten metric names Signed-off-by: Marco Pracucci <marco@pracucci.com> * Added CHANGELOG entry Signed-off-by: Marco Pracucci <marco@pracucci.com> * Addressed review comments Signed-off-by: Marco Pracucci <marco@pracucci.com> * Removed readerTracker Signed-off-by: Marco Pracucci <marco@pracucci.com> * Fixed test and comments Signed-off-by: Marco Pracucci <marco@pracucci.com> Signed-off-by: Oghenebrume50 <raphlbrume@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.
Changes
As discussed during the Thanos community meeting, I would like to propose to add support for lazy mmaping of index-headers in the store-gateway with the ability to automatically unload them after X time of inactivity. This PR aims to do it.
Draft because:
Verification
Unit tests.