refactor: Migrate from legacy storage implementations to disk.Store and tiered.Store - #658
Open
Anton-Kalpakchiev wants to merge 2 commits into
Open
refactor: Migrate from legacy storage implementations to disk.Store and tiered.Store#658Anton-Kalpakchiev wants to merge 2 commits into
Anton-Kalpakchiev wants to merge 2 commits into
Conversation
Currently, the disk.Store assumes that all its use cases will be to store blobs of varying sizes, thus it assumes 1. that its capacity will be in bytes 2. that the blob's client-provided size passed in Create will be in bytes 3. and thus, that when rebooting a blob from disk after a crash, that it can use the blob's real size, as the store operates in bytes. However, one of the use cases that disk.Store will replace is SimpleStore used in build-index, where all files are the same small size (a tag) and thus build-index runs an LRU cache capped at the number of entries, not the total size of all entries, i.e. an unweighted LRU cache. To support that use case, disk.Store must remove its assumption that its size and capacity are all in bytes and accept that their unit might be `number of entries`, i.e. each call to Create will pass `size == 1` and Capacity will represent the total number of entries the store can hold. This commit makes the necessary changes to support the use case: - Ensure that when rebooting a blob, we always look at the client- provided size (we currently look at the blob's real size sometimes) - Change all `sizeBytes` and `capacityBytes` variable names in Store to just `size` and `capacity`. Same applies for metric/log field names - Add a comment in the `Config` struct's `Capacity` field to explain how the store supports both use cases. Also explain how the Capacity limit is soft and not hard, due to Linux semantics. - Add a test for the new behavior. - A few small improvements here and there.
… disk.Store and tiered.Store Replaces all usage of CAStore and SimpleStore with usage of disk.Store and tiered.Store. I had to migrate origin and build-index in the same commit, as they share some libraries (e.g. writeback) which use CAStore. The next commits will include agent and proxy. Small changes were necessary to support the migration. Most notably: - origin's API to upload blobs now stores each file as it's being uploaded in disk.Store, using its digest as key, whereas before it was the randomly- generated uid that was used. There are some subtle changes in the API's behavior due to this: 1) now multiple clients cannot upload the same blob at the same time. Previously, they were allowed to fully upload the blob, but they would race for success at the commit phase of the API. Now, they race at the start phase instead. 2) if for some reason, clients of the API stop halfway-through, the file will be leaked. I will add a metric to observe and alert on such cases in the next commit. I am thinking of adding a TTL for incomplete blobs, after which they get cleaned, to ensure that they don't get leaked to disk. - add Close method to tiered.Store used for testing. - change the closers.Close log from Debug to Warn level so we can flag any bugs after rollout instead of silently dropping them - delete the forceCleanupHandler in origin, as it is no longer needed (forceCleanupHandlerV2 replaced it) - add fixtures for disk.Store and tiered.Store - base.yaml files had to have their configs changed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Now that
disk.Storeandtiered.Storeare implemented, we can finally move away from using CAStore, SimpleStore, etc. and deprecate them. This PR implements the migration with a subsequent PR to follow to delete all the no longer necessary legacy code.For more context, check #633.
Please review commit by commit, as there will be a few extra necessary changes in this PR!