Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implements retrying logic for downloading models using --model-url flag #9255

Merged
merged 8 commits into from
Sep 11, 2024

Conversation

farbodbj
Copy link
Contributor

@farbodbj farbodbj commented Aug 30, 2024

This PR implements the improvement requested at #6288, the approach is simple and uses a while loop for retrying with exponential backoff.
I found two calls to curl_easy_perform which is the function responsible for transferring the data, and on the call sites, I added the while loop for retrying on the condition of failing.
For now, the retry count is fixed as a local variable inside the function, but passing the retry count as a CLI argument can further improve this feature.
I compiled the project after the changes with make LLAMA_CURL=1 and tested it both with and without internet connection and it worked okay.
It's my first time working with curl API in cpp so any suggestion for improvement is appreciated.

farbodbj and others added 3 commits August 31, 2024 17:09
@farbodbj
Copy link
Contributor Author

@ngxson
I applied your comments. wasn't sure where I had to put my #defines so I put them right above the function using them.

@farbodbj
Copy link
Contributor Author

farbodbj commented Sep 1, 2024

I would also appreciate if someone can help me to fix the editorconfig-checker CI job, I couldn't figure out how to run the job locally to fix formatting and white space problems

@farbodbj farbodbj requested a review from ngxson September 2, 2024 07:11
Copy link
Member

@slaren slaren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest extracting the retry logic to a function rather than duplicating the code.

@farbodbj
Copy link
Contributor Author

farbodbj commented Sep 6, 2024

I would suggest extracting the retry logic to a function rather than duplicating the code.

I agree on that. Do you suggest I do it in this PR or do it later? I think approving will be required again after i apply the change

PS: Done 😄

@farbodbj farbodbj force-pushed the add-retry-to-model-download branch from a032b0b to 192d4df Compare September 6, 2024 12:17
@farbodbj farbodbj force-pushed the add-retry-to-model-download branch from e12b07e to c8f2890 Compare September 6, 2024 12:28
@slaren
Copy link
Member

slaren commented Sep 10, 2024

@ngxson Please update your review.

Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll merge when CI passes

@ngxson ngxson merged commit 67155ab into ggml-org:master Sep 11, 2024
55 checks passed
@farbodbj farbodbj deleted the add-retry-to-model-download branch September 11, 2024 20:02
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
…url flag (ggml-org#9255)

* feat: Implements retrying logic for downloading models using --model-url flag

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* apply comments

* implements a retry function to avoid duplication

* fix editorconfig

* change function name

---------

Co-authored-by: farbod <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
Co-authored-by: slaren <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
…url flag (ggml-org#9255)

* feat: Implements retrying logic for downloading models using --model-url flag

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* apply comments

* implements a retry function to avoid duplication

* fix editorconfig

* change function name

---------

Co-authored-by: farbod <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
Co-authored-by: slaren <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
…url flag (ggml-org#9255)

* feat: Implements retrying logic for downloading models using --model-url flag

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* Update common/common.cpp

Co-authored-by: Xuan Son Nguyen <[email protected]>

* apply comments

* implements a retry function to avoid duplication

* fix editorconfig

* change function name

---------

Co-authored-by: farbod <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
Co-authored-by: slaren <[email protected]>
Co-authored-by: Xuan Son Nguyen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants