-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Add feature gates for switching between the legacy inflight limiting #76413
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ package options | |
|
||
import ( | ||
"fmt" | ||
"k8s.io/apiserver/pkg/features" | ||
"net" | ||
"time" | ||
|
||
|
@@ -49,8 +50,9 @@ type ServerRunOptions struct { | |
// decoded in a write request. 0 means no limit. | ||
// We intentionally did not add a flag for this option. Users of the | ||
// apiserver library can wire it to a flag. | ||
MaxRequestBodyBytes int64 | ||
TargetRAMMB int | ||
MaxRequestBodyBytes int64 | ||
TargetRAMMB int | ||
EnableInfightQuotaHandler bool | ||
} | ||
|
||
func NewServerRunOptions() *ServerRunOptions { | ||
|
@@ -104,11 +106,27 @@ func (s *ServerRunOptions) Validate() []error { | |
if s.TargetRAMMB < 0 { | ||
errors = append(errors, fmt.Errorf("--target-ram-mb can not be negative value")) | ||
} | ||
if s.MaxRequestsInFlight < 0 { | ||
errors = append(errors, fmt.Errorf("--max-requests-inflight can not be negative value")) | ||
} | ||
if s.MaxMutatingRequestsInFlight < 0 { | ||
errors = append(errors, fmt.Errorf("--max-mutating-requests-inflight can not be negative value")) | ||
|
||
yue9944882 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if s.EnableInfightQuotaHandler { | ||
if !utilfeature.DefaultFeatureGate.Enabled(features.RequestManagement) { | ||
errors = append(errors, fmt.Errorf("--enable-inflight-quota-handler can not be set if feature "+ | ||
"gate RequestManagement is disabled")) | ||
} | ||
if s.MaxMutatingRequestsInFlight != 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we'd be checking if the user specified the flag or not, it's a bit weird to make them explicitly set the flag to 0 :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmm.. yeah ideally.. while the cli framework doesn't seem support that yet. |
||
errors = append(errors, fmt.Errorf("--max-mutating-requests-inflight=%v "+ | ||
"can not be set if enabled inflight quota handler", s.MaxMutatingRequestsInFlight)) | ||
} | ||
if s.MaxRequestsInFlight != 0 { | ||
errors = append(errors, fmt.Errorf("--max-requests-inflight=%v "+ | ||
"can not be set if enabled inflight quota handler", s.MaxRequestsInFlight)) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to do something to warn people that this feature isn't actually working yet, either in the help text or by making an error here, or both? Let's do it in a followup since most of this change is fine. |
||
} else { | ||
if s.MaxRequestsInFlight < 0 { | ||
errors = append(errors, fmt.Errorf("--max-requests-inflight can not be negative value")) | ||
} | ||
if s.MaxMutatingRequestsInFlight < 0 { | ||
errors = append(errors, fmt.Errorf("--max-mutating-requests-inflight can not be negative value")) | ||
} | ||
} | ||
|
||
if s.RequestTimeout.Nanoseconds() < 0 { | ||
|
@@ -174,5 +192,8 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) { | |
"handler, which picks a randomized value above this number as the connection timeout, "+ | ||
"to spread out load.") | ||
|
||
fs.BoolVar(&s.EnableInfightQuotaHandler, "enable-inflight-quota-handler", s.EnableInfightQuotaHandler, ""+ | ||
"If true, replace the max-in-flight handler with an enhanced one that queues and dispatches with priority and fairness") | ||
|
||
utilfeature.DefaultMutableFeatureGate.AddFlag(fs) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Infight/Inflight. I can make this change when I add the error/warning messages.