store-gateway: retain lazy-loaded index headers between restarts #5606
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
lamida
changed the title
This was referenced Jul 28, 2023
…aded block in store gateway (#5095) * Add lazy loaded trackers of store gateway index headers Signed-off-by: Jon Kartago Lamida <[email protected]> * Add period to persist list of lazy loaded blocks Signed-off-by: Jon Kartago Lamida <[email protected]> * Track only lazy loaded block during persist time Signed-off-by: Jon Kartago Lamida <[email protected]> * Use protobuf to persist the tracker state of lazy loaded block Signed-off-by: Jon Kartago Lamida <[email protected]> * Add proto suffix for the binary file Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename protobuf name Signed-off-by: Jon Kartago Lamida <[email protected]> * Initiate HeaderLazyLoadedTracker in bucket per tenant Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix outdated constructor in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unused LazyLoaderTracker in binary_reader Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unnecessary newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix HeadersLazyLoadedTracker receiver must be pointer Signed-off-by: Jon Kartago Lamida <[email protected]> * Small grammar correction in comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename constant to lazyLoadedFile Signed-off-by: Jon Kartago Lamida <[email protected]> * Use hardcoded 1 minute to persist list of lazyLoadedBlock Signed-off-by: Jon Kartago Lamida <[email protected]> * Add layzLoaded tracker test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add logger when lazyLoadedTracker file to persist Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Unexport lazyLoadedTracker method Signed-off-by: Jon Kartago Lamida <[email protected]> * Try to set checksum Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Oleg Zaytsev <[email protected]> * Use one goroutine for closeIdleReader and lazyLoad persist Signed-off-by: Jon Kartago Lamida <[email protected]> * Use pb suffix in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Refactor the structure of tracker Signed-off-by: Jon Kartago Lamida <[email protected]> * Clean lint Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Use fsync Signed-off-by: Jon Kartago Lamida <[email protected]> * Return err Signed-off-by: Jon Kartago Lamida <[email protected]> * Use json Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Remove defer and use t.CleanUp Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Still fixing linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert idle timeout in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Apply PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Read the file to assert the lazyloaded heaeder Signed-off-by: Jon Kartago Lamida <[email protected]> * Don't use /tmp directory for lazyloaded header Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Apply PR suggestion Signed-off-by: Jon Kartago Lamida <[email protected]> * Extract fsync operation Signed-off-by: Jon Kartago Lamida <[email protected]> * Add assertion on LoadedBlocks Signed-off-by: Jon Kartago Lamida <[email protected]> * Update comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Add more proper LoadedBlock test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add license header Signed-off-by: Jon Kartago Lamida <[email protected]> * Update fsync method name Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Address PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Update method comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update method comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Add comments to explain why we use os.Rename Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unrelevant test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add assertion to make sure there is only one lazyReader Signed-off-by: Jon Kartago Lamida <[email protected]> * Use io.Reader for fsync data Signed-off-by: Jon Kartago Lamida <[email protected]> * Ramove table test for a single test case only Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/util/atomicfs/fsync_test.go Co-authored-by: Marco Pracucci <[email protected]> * Apply PR suggestions Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/util/atomicfs/fsync_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Clean import Signed-off-by: Jon Kartago Lamida <[email protected]> --------- Signed-off-by: Jon Kartago Lamida <[email protected]> Co-authored-by: Dimitar Dimitrov <[email protected]> Co-authored-by: Oleg Zaytsev <[email protected]> Co-authored-by: Charles Korn <[email protected]> Co-authored-by: Marco Pracucci <[email protected]>
* Add lazy loaded trackers of store gateway index headers Signed-off-by: Jon Kartago Lamida <[email protected]> * Add period to persist list of lazy loaded blocks Signed-off-by: Jon Kartago Lamida <[email protected]> * Track only lazy loaded block during persist time Signed-off-by: Jon Kartago Lamida <[email protected]> * Use protobuf to persist the tracker state of lazy loaded block Signed-off-by: Jon Kartago Lamida <[email protected]> * Add proto suffix for the binary file Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename protobuf name Signed-off-by: Jon Kartago Lamida <[email protected]> * Initiate HeaderLazyLoadedTracker in bucket per tenant Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix outdated constructor in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unused LazyLoaderTracker in binary_reader Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unnecessary newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix HeadersLazyLoadedTracker receiver must be pointer Signed-off-by: Jon Kartago Lamida <[email protected]> * Small grammar correction in comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename constant to lazyLoadedFile Signed-off-by: Jon Kartago Lamida <[email protected]> * Use hardcoded 1 minute to persist list of lazyLoadedBlock Signed-off-by: Jon Kartago Lamida <[email protected]> * Add layzLoaded tracker test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add logger when lazyLoadedTracker file to persist Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Unexport lazyLoadedTracker method Signed-off-by: Jon Kartago Lamida <[email protected]> * Try to set checksum Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Oleg Zaytsev <[email protected]> * Use one goroutine for closeIdleReader and lazyLoad persist Signed-off-by: Jon Kartago Lamida <[email protected]> * Use pb suffix in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Refactor the structure of tracker Signed-off-by: Jon Kartago Lamida <[email protected]> * Clean lint Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Use fsync Signed-off-by: Jon Kartago Lamida <[email protected]> * Return err Signed-off-by: Jon Kartago Lamida <[email protected]> * Use json Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Remove defer and use t.CleanUp Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Still fixing linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert idle timeout in test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Revert newline Signed-off-by: Jon Kartago Lamida <[email protected]> * Apply PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Read the file to assert the lazyloaded heaeder Signed-off-by: Jon Kartago Lamida <[email protected]> * Don't use /tmp directory for lazyloaded header Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Apply PR suggestion Signed-off-by: Jon Kartago Lamida <[email protected]> * Extract fsync operation Signed-off-by: Jon Kartago Lamida <[email protected]> * Add assertion on LoadedBlocks Signed-off-by: Jon Kartago Lamida <[email protected]> * Update comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Add more proper LoadedBlock test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add license header Signed-off-by: Jon Kartago Lamida <[email protected]> * Update fsync method name Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Address PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Update method comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update method comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Add comments to explain why we use os.Rename Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unrelevant test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add assertion to make sure there is only one lazyReader Signed-off-by: Jon Kartago Lamida <[email protected]> * Use io.Reader for fsync data Signed-off-by: Jon Kartago Lamida <[email protected]> * Ramove table test for a single test case only Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/util/atomicfs/fsync_test.go Co-authored-by: Marco Pracucci <[email protected]> * Apply PR suggestions Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/util/atomicfs/fsync_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Clean import Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix lint Signed-off-by: Jon Kartago Lamida <[email protected]> --------- Signed-off-by: Jon Kartago Lamida <[email protected]> Co-authored-by: Dimitar Dimitrov <[email protected]> Co-authored-by: Oleg Zaytsev <[email protected]> Co-authored-by: Charles Korn <[email protected]> Co-authored-by: Marco Pracucci <[email protected]>
* Start to do eager load of index header during startup Signed-off-by: Jon Kartago Lamida <[email protected]> * Make EagerLoading as public method Signed-off-by: Jon Kartago Lamida <[email protected]> * Add eager loading test Signed-off-by: Jon Kartago Lamida <[email protected]> * Small refactoring of test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add godoc Signed-off-by: Jon Kartago Lamida <[email protected]> * Update reader_pool_test with eager loading logic Signed-off-by: Jon Kartago Lamida <[email protected]> * Small refactoring to make test clearer Signed-off-by: Jon Kartago Lamida <[email protected]> * Appease the linter Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename private method for test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add config to enable or disable index headers eager loading Signed-off-by: Jon Kartago Lamida <[email protected]> * Changelog Signed-off-by: Jon Kartago Lamida <[email protected]> * Changelog Signed-off-by: Jon Kartago Lamida <[email protected]> * Go select can't be inside method Signed-off-by: Jon Kartago Lamida <[email protected]> * Run make doc Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix test Signed-off-by: Jon Kartago Lamida <[email protected]> * Update CHANGELOG.md Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/lazy_binary_reader.go Co-authored-by: Charles Korn <[email protected]> * Apply configs PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Pass BucketStoreConfig as argument Signed-off-by: Jon Kartago Lamida <[email protected]> * Pass eagerLoadIndexReaderEnabled into snapshotConfig Signed-off-by: Jon Kartago Lamida <[email protected]> * Don't pass lazyLoadedSnapshot into LazyBinaryReader Signed-off-by: Jon Kartago Lamida <[email protected]> * Only initialize tickerLazyLoad when flag is enabled Signed-off-by: Jon Kartago Lamida <[email protected]> * Apply PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Update config description Signed-off-by: Jon Kartago Lamida <[email protected]> * Update usedAt to current time in eager loading Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Dimitar Dimitrov <[email protected]> * Fix snapshotByte name Signed-off-by: Jon Kartago Lamida <[email protected]> * Address PR feedbacks Signed-off-by: Jon Kartago Lamida <[email protected]> * Rename initLazyBinaryForTest Signed-off-by: Jon Kartago Lamida <[email protected]> * Apply PR suggestion Signed-off-by: Jon Kartago Lamida <[email protected]> * Refactor test Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Apply more PR suggestions Signed-off-by: Jon Kartago Lamida <[email protected]> * Refactor configs Signed-off-by: Jon Kartago Lamida <[email protected]> * Header eager loading only must be done during initialSync Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove unnecesary comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Remove snapshot file regardless snapshot loading result Signed-off-by: Jon Kartago Lamida <[email protected]> * Fix compilation error Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/header.go Co-authored-by: Charles Korn <[email protected]> * Regenerate doc for updated command line args Signed-off-by: Jon Kartago Lamida <[email protected]> * Update error log Signed-off-by: Jon Kartago Lamida <[email protected]> * Update comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update comment Signed-off-by: Jon Kartago Lamida <[email protected]> * Update test Signed-off-by: Jon Kartago Lamida <[email protected]> * Add test case Signed-off-by: Jon Kartago Lamida <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool_test.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/lazy_binary_reader.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/reader_pool.go Co-authored-by: Charles Korn <[email protected]> * Update pkg/storegateway/indexheader/header.go Co-authored-by: Charles Korn <[email protected]> * Update changelog Signed-off-by: Jon Kartago Lamida <[email protected]> --------- Signed-off-by: Jon Kartago Lamida <[email protected]> Co-authored-by: Charles Korn <[email protected]> Co-authored-by: Dimitar Dimitrov <[email protected]>
lamida
force-pushed
the
lamida/sg-lazy-load
branch
from
402bc09 to
2109b06
Compare
Signed-off-by: Jon Kartago Lamida <[email protected]>
dimitarvdimitrov
approved these changes
Aug 14, 2023
| MaxIdleFileHandles uint `yaml:"max_idle_file_handles" category:"advanced"` | ||
| VerifyOnLoad bool `yaml:"verify_on_load" category:"advanced"` | ||
| MaxIdleFileHandles uint `yaml:"max_idle_file_handles" category:"advanced"` | ||
| IndexHeaderEagerLoadingStartupEnabled bool `yaml:"eager_loading_startup_enabled" category:"experimental"` |
There was a problem hiding this comment.
nit: can you remove the IndexHeader prefix of the field? The package is already called indexheader and the config is the index header config.
There was a problem hiding this comment.
I noticed this and addressed in my follow up PR for config refactoring: #5726
Is it ok to be addressed there?
Signed-off-by: Jon Kartago Lamida <[email protected]>
Signed-off-by: Jon Kartago Lamida <[email protected]>
dimitarvdimitrov
approved these changes
Aug 14, 2023
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.
What this PR does
Combining
storegateway: Detect OOM During Eagerly Loading Index Header #5644And also resolving plenty of conflicts against
mainbranch.Which issue(s) this PR fixes or relates to
Fixes #4762
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]