feat: chunk put allow retrying un-get-able chunks #1047
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
| println!("\"{file_name:?}\" {addr:x}"); | ||
| info!("Uploaded {file_name:?} to {addr:x}"); | ||
| // make sure we don't have any failed chunks in those | ||
| let chunks = chunk_manager.already_put_chunks(&files_path)?; |
There was a problem hiding this comment.
I think this would create chunks in the artifacts dir and if they're not marked as completed when failed_chunks is not empty (L172), then during the next run, we'd get resume the chunks at line 162 and we'll try to upload them again (L166 returns false).
There was a problem hiding this comment.
So should we make chunk_manager.already_put_chunks(&files_path)?; create the chunks to a temp dir for verification?
That might avoid complication.
There was a problem hiding this comment.
Nice catch! Marked the non-failed chunks as completed just below. And if they fail, it's reasonable to keep them around for next time!
grumbach
force-pushed
the
chunk_put_smart_retries
branch
2 times, most recently
from
467e87a
to
352e07b
Compare
RolandSherwin
approved these changes
Dec 5, 2023
grumbach
force-pushed
the
chunk_put_smart_retries
branch
from
352e07b
to
aec4f60
Compare
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.
Description
Depends on #1022, please merge #1022 first!
Summary generated by Reviewpad on 05 Dec 23 09:36 UTC
This pull request includes the following changes:
verify_uploaded_chunksmethod has been removed from theFilesstruct. Instead, the logic of this method has been moved to theupload_filemethod of the same struct, which now calls theverify_uploaded_chunksmethod on theclientfield.verify_uploaded_chunkshas been added to theClientstruct. This method takes a vector of chunk paths and a batch size, and returns a vector of chunks that could not be verified.get_register_from_recordhas been implemented, which converts a record into a signed register.upload_filesfunction in thesn_cli/src/subcommands/files/mod.rsfile has been modified. The changes include adding error handling to check if the wallet is empty before uploading files, initializing theChunkManagerwith the providedroot_dir, adding a call to determine the chunk path for the given files path, printing messages for uploaded and verified files, handling different scenarios based on the existence of chunks to upload, and logging and printing payment and upload information.io::BufReadandio::BufReaderhave been removed.childhas been assigned but is unused.These changes aim to refactor the code, improve the upload process, handle failed chunks, provide better information about the upload and payment details, and remove unused imports and variables.