feat: add priority based holdback strategies in flow control#1592
Conversation
|
🚨 Unsigned commits detected! Please sign your commits. For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation. |
442afed to
d56a6b4
Compare
d56a6b4 to
b3a002b
Compare
|
Didn't get around to reviewing this today; sorry @loicmarchal! Will take a look when I am back in office Tuesday. Let's target this for v0.10 if possible and try to land this alongside eviction (which @RishabhSaini has a draft for). |
58bf51c to
5b5caef
Compare
Thank you @LukeAVanDrie. No worries. Sounds good for v0.10 |
|
@LukeAVanDrie I implemented a new plugin in its own package. I hesitated between this approach and making use of the existing usage limit. The way the usage limit policy was implemented uses a function that is simply called in the |
5b5caef to
3efe013
Compare
c375020 to
4b2d595
Compare
I agree. That was a main argument in my comment My proposal: parameters:
shape: linear # optional, default: linear (future: sigmoid) => clarify in the doc we currently only support linear
domain: rank # optional, default: rank (rank | value)
minCeiling: 0.4 # required
maxCeiling: 0.9 # optional, default: 1.0This way, we are future-proof for sigmoid, exponential, step, etc. At the same time, if we stay as it is, nothing will break. If ok with you, I'll implement that change, then maybe we can merge and add sigmoid (or others) as a future implementation? |
2a1c6a6 to
afb9b3f
Compare
|
Thank you for your review @LukeAVanDrie. I changed the config. |
LukeAVanDrie
left a comment
There was a problem hiding this comment.
/lgtm
Thanks @loicmarchal!
afb9b3f to
ecc544e
Compare
|
@ahg-g @liu-cong @elevran @shmuelk This PR is critical for batch-gateway integration with flow-control - #1185 (comment). Can you please look into merging this? Thanks. |
ecc544e to
bc0acde
Compare
bc0acde to
bc04820
Compare
Signed-off-by: Loic Marchal <lmarchal@redhat.com>
Signed-off-by: Loic Marchal <lmarchal@redhat.com>
Signed-off-by: Loic Marchal <lmarchal@redhat.com>
Signed-off-by: Loic Marchal <lmarchal@redhat.com>
bc04820 to
ad84e45
Compare
Added copyright notice and license information to priority_holdback.go Signed-off-by: Cong Liu <conliu@google.com>
…back/config.go Signed-off-by: Cong Liu <conliu@google.com>
…back/priority_holdback_test.go Signed-off-by: Cong Liu <conliu@google.com>
…back/config.go Signed-off-by: Cong Liu <conliu@google.com>
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add progressive holdback based on priorities.
It implements the priority-holdback-policy plugin (flowcontrol.UsageLimitPolicy), which computes differentiated admission ceilings per priority level. Lower-priority traffic is gated at lower saturation thresholds, reserving capacity for higher-priority work as the pool approaches saturation.
It replaces the default static usage limit policy's binary backpressure approach with priority-aware stepped gating.
Two gating strategies are supported:
Both are the same linear mapping applied to different input domains (rank vs value). With evenly distributed priorities, they converge.
It's a first implementation for a v1. More strategies could be added to offer more control of the ceilings distribution to the user (e.g. Sigmoid curve instead of linear).
Which issue(s) this PR fixes:
Fixes #1185
Release note (write
NONEif no user-facing change):