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

fix(dispatcher): prevent possible dispatcher bad behavior #3646

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sauyon
Copy link
Contributor

@sauyon sauyon commented Mar 9, 2023

Previously, it was possible that if request load was fast enough and max latency set low enough, the dispatcher would enter a loop of waiting for new requests while cancelling the oldest ones while doing no work, this change seeks to fix that.

  • The optimizer values for slope and intercept are now bounded below at 0; we may want to eventually try to collect cases where the linear optimizer does this, but for now we do this to avoid the most pathological cases.
  • If the first request is in danger of being cancelled, we now stop waiting and begin processing immediately.

@sauyon sauyon requested a review from a team as a code owner March 9, 2023 04:13
@sauyon sauyon requested review from larme, bojiang, a team and aarnphm and removed request for a team March 9, 2023 04:13
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #3646 (06bcaab) into main (f503a68) will increase coverage by 0.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3646      +/-   ##
==========================================
+ Coverage   31.74%   31.89%   +0.14%     
==========================================
  Files         149      149              
  Lines       12149    12094      -55     
  Branches     2001     1984      -17     
==========================================
  Hits         3857     3857              
+ Misses       8008     7953      -55     
  Partials      284      284              
Impacted Files Coverage Δ
src/bentoml/_internal/marshal/dispatcher.py 0.00% <ø> (ø)

@sauyon sauyon force-pushed the dispatcher-fix branch 2 times, most recently from 576f2ec to cb82296 Compare March 9, 2023 04:31
@bojiang
Copy link
Member

bojiang commented Mar 16, 2023

@sauyon Is this ready? Maybe let's discuss tomorrow so that we can merge this and another related PR, shall we?

@sauyon
Copy link
Contributor Author

sauyon commented Mar 16, 2023

@sauyon Is this ready? Maybe let's discuss tomorrow so that we can merge this and another related PR, shall we?

Sounds good, when do you want to chat?

@bojiang bojiang added the pr/merge-hold Requires further discussions before a pull request can be merged label Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/merge-hold Requires further discussions before a pull request can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants