[WIP] bundle: Parallel download and decompression #4504
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test all |
|
@vyasgun: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
vyasgun
force-pushed
the
pr/parallel-decompress
branch
9 times, most recently
from
bb0b17c to
3a62d1a
Compare
There was a problem hiding this comment.
Overall, this is great work and it's functional.
My findings:
-
cancelling through web api (socket) works
-
better logging would be nice (currently it's skipping the download part) so that it's clear that the download and uncompression is being done simultaneously
-
progress bar could show more info about both processes
-
resuming interrupted download doesn't work - everything starts from the beginning
-
golangci-lint issues
Suggestions:
- add (cli/config) option to disable this functionality (revert back to old behavior)
| @@ -124,6 +125,36 @@ func (bundle *CrcBundleInfo) createSymlinkOrCopyPodmanRemote(binDir string) erro | |||
| return bundle.copyExecutableFromBundle(binDir, PodmanExecutable, constants.PodmanRemoteExecutableName) | |||
| } | |||
|
|
|||
| func (repo *Repository) ExtractWithReader(ctx context.Context, reader io.Reader, path string) error { | |||
There was a problem hiding this comment.
This function and Extract are very similar, could they be merged in some way?
There was a problem hiding this comment.
Extract could probably use os.Open and call into ExtractWithReader
| @@ -124,6 +125,36 @@ func (bundle *CrcBundleInfo) createSymlinkOrCopyPodmanRemote(binDir string) erro | |||
| return bundle.copyExecutableFromBundle(binDir, PodmanExecutable, constants.PodmanRemoteExecutableName) | |||
| } | |||
|
|
|||
| func (repo *Repository) ExtractWithReader(ctx context.Context, reader io.Reader, path string) error { | |||
There was a problem hiding this comment.
Extract could probably use os.Open and call into ExtractWithReader
| @@ -163,7 +163,7 @@ func downloadDataFiles(goos string, components []string, destDir string) ([]stri | |||
| if !shouldDownload(components, componentName) { | |||
| continue | |||
| } | |||
| filename, err := download.Download(context.TODO(), dl.url, destDir, dl.permissions, nil) | |||
| _, filename, err := download.Download(context.TODO(), dl.url, destDir, dl.permissions, nil) | |||
There was a problem hiding this comment.
I think I'd add a download.DownloadFile(…) (string, error) to make it clear when we don't need the reader.
| logging.Infof("Extracting bundle: %s...", bundleName) | ||
| if _, err := bundle.Extract(ctx, bundlePath); err != nil { | ||
| if _, err := bundle.Extract(ctx, reader, bundlePath); err != nil { |
There was a problem hiding this comment.
Having a bundlePath and a reader feels a bit redundant, ideally we could pass one or the other, but I'm not sure it is currently that easy.
This commit does the following: - Return a reader from the bundle Download function. - Use the reader to stream the bytes to Extract function. This commit replaces grab client with the net/http client to ensure that the bytes are streamed come in correct order to the Extract func. Currently, only zst decompression is being used in the UncompressWithReader function as it is the primary compression algorithm being used in crc.
vyasgun
force-pushed
the
pr/parallel-decompress
branch
from
3a62d1a to
bb33dee
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
This pull request does the following:
This commit replaces grab client with the net/http client to ensure that the bytes are streamed come in correct order to the Extract func. Currently, only zst decompression is being used in the UncompressWithReader function as it is the primary compression algorithm being used in crc.
The download progress bar has been removed temporarily and will be added back as part of refactoring the code.
Fixes: #4336
Type of change
test, version modification, documentation, etc.)
Proposed changes
Testing
Contribution Checklist