Fix races during parallel downloads #2903
Merged
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
nirs
changed the title
|
Tested using a script creating 10 instances in parallel. Test scripts and logs: |
|
Another test, creating and starting 3 instances in parallel: |
We tested that modified last modified time on the server cause a redownload, but we did not test that after a redownload we update the cache, so the next attempt will used the cached download. Add a failing test verifying the issue, and improve test comments and configuration to make it more clear. Part-of: lima-vm#2902 Signed-off-by: Nir Soffer <[email protected]>
Extract getCache() to handle getting and file from the cache, and fetch() for fetching a file from the remote to the cache. This will allow locking around the code checking and updating the cache, and much easier to maintain and understand. Signed-off-by: Nir Soffer <[email protected]>
To solve the races during concurrent downloads and avoid unneeded work and bandwidth, we allow one concurrent download of the same image. When a limactl process try to access the cache, it takes a lock on the file cache directory. If multiple processes try to get the lock in the same time, only one will take the lock, and the other will block. The process that took the lock tries to get the file from the cache. This is the fast path and the common case. This can fail if the file is not in the cache, the digest does not match, or the cached last modified time does not match the last modified returned from the server. If the process cannot get the file from the cache, it downloads the file from the remote server, and update the cached data and metadata files. Finally the process release the lock on the cache directory. Other limactl processes waiting on the lock wake up and take the lock. In the common case they will find the image in the cache and will release the lock quickly. Since we have exactly one concurrent download, updating the metadata files is trivial and we don't need the writeFirst() helper. Fixes: lima-vm#2902 Fixes: lima-vm#2732 Signed-off-by: Nir Soffer <[email protected]>
We can assert now that only one thread downloaded the file, and all other threads used the cache. Signed-off-by: Nir Soffer <[email protected]>
Checking if an image is cached races with parallel downloads. Take the lock when validating the digest or the data file to ensure that we validate the cached when it is in consistent state. If an image is being downloaded, the check will block until the download completes. Signed-off-by: Nir Soffer <[email protected]>
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.
Fix races during parallel downloads by holding a per file lock while checking or updating the cache.
Fixes #2902, #2732