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

Retry strategy and use semaphores to prevent creating a large number of async tasks #36

Merged
merged 7 commits into from
Jun 30, 2023

Conversation

onkarbhardwaj
Copy link
Contributor

@onkarbhardwaj onkarbhardwaj commented Jun 14, 2023


Status

READY

Description

  • As a workaround for temporary network outages and temporary deployment issues, we are introducing retry with exponential backoff for asynchronous versions of generate and tokenize.
  • We are also introducing semphores to prevent a large number of simultaneous async tasks getting created in case a user thinks of sending hundreds of thousands of prompts.
  • Semaphores also enable Retry strategy in a subtle way by preventing pushing a request-to-be-retried behind a large number of async queries. If not prevented, such a query could potentially be pushed behind all these tasks and that would affect run-time for ordered mode of generate_async.
  • Semaphores also enabled running much larger number of prompts in async manner.

Impacted Areas in Library

AsyncResponseGenerator, RequestHandler.

Which issue(s) does this pull-request fix?

Closes: #15

Any special notes for your reviewer?

For testing the effect of semaphores, try sending a large number of prompts (>200k) with or without these changes. The exact number might differ system-to-system. For testing retry-strategy, check the dev example in this PR. This example randomly alters http return code and causes retry+backoff.


Checklist

  • Automated tests exist
  • Updated Package Requirements (if required, and with maintainers' approval)
  • Local unit tests performed
  • Documentation exists link
  • Local pre-commit hooks performed
  • Desired commit message set as PR title and description set above
  • Link to relevant GitHub issue provided

…getting created

Signed-off-by: Onkar Bhardwaj <onkarbhardwaj@ibm.com>
@coveralls
Copy link

coveralls commented Jun 14, 2023

Pull Request Test Coverage Report for Build 5417755269

  • 32 of 32 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 94.737%

Totals Coverage Status
Change from base Build 5379768299: 0.08%
Covered Lines: 1242
Relevant Lines: 1311

💛 - Coveralls

Signed-off-by: Onkar Bhardwaj <onkar.bhardwaj@gmail.com>
@onkarbhardwaj onkarbhardwaj requested review from 133martie, jpwsutton, vedem1192, mirianfsilva and moneill0 and removed request for 133martie June 14, 2023 18:56
@onkarbhardwaj onkarbhardwaj changed the title Retry strategy and use semaphores to prevent a large number of tasks … Retry strategy and use semaphores to prevent creating a large number of async tasks Jun 15, 2023
Copy link
Member

@mirianfsilva mirianfsilva left a comment

Choose a reason for hiding this comment

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

lgtm!

Signed-off-by: Onkar Bhardwaj <onkar.bhardwaj@gmail.com>
@onkarbhardwaj
Copy link
Contributor Author

Resolved merge conflict via GUI. Hope tabs don't create an indentation issue.

moneill0
moneill0 previously approved these changes Jun 27, 2023
@moneill0 moneill0 dismissed their stale review June 27, 2023 20:26

Has merge conflicts

Signed-off-by: Mírian Silva <mirianfrsilva@ibm.com>
Copy link
Member

@mirianfsilva mirianfsilva left a comment

Choose a reason for hiding this comment

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

tests are not passing after merge with develop. But the async examples works fine with +550 prompts.

Signed-off-by: Onkar Bhardwaj <onkar.bhardwaj@gmail.com>
Copy link
Member

@mirianfsilva mirianfsilva left a comment

Choose a reason for hiding this comment

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

LGTM, and works fine with +1000 prompts.

examples/dev/async-flaky-request-handler.py Show resolved Hide resolved
Co-authored-by: Mírian Silva <mirianfrsilva@gmail.com>
Signed-off-by: Onkar Bhardwaj <onkar.bhardwaj@gmail.com>
@vedem1192 vedem1192 merged commit c5105c7 into IBM:develop Jun 30, 2023
4 checks passed
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.

None yet

6 participants