tigerquoll commented on code in PR #1060:
URL: https://github.com/apache/yunikorn-k8shim/pull/1060#discussion_r3766751955
##########
pkg/conf/schedulerconf.go:
##########
@@ -91,8 +93,10 @@ const (
DefaultOperatorPlugins = "general"
DefaultDisableGangScheduling = false
DefaultEnableConfigHotRefresh = true
- DefaultKubeQPS = 1000
- DefaultKubeBurst = 1000
+ DefaultKubeQPS = 0 // client side write
limiting is opt-in: <= 0 means no limiter
Review Comment:
Your understanding of client-go is right - a raw 0 in `rest.Config` lands on
the hidden 5/10 defaults. That's exactly why the configured value never reaches
the `rest.Config` as-is: `rateLimitPolicy` translates anything <= 0 into `QPS =
-1` before the client is built, which client-go documents as "disable
client-side ratelimiting". So the -1 you're suggesting is there - it's applied
at the point where client-go acts on it, and 0 in the configuration was just
the "not set" sentinel, with the documented convention being "<= 0 disables".
The same convention applies to `eventQPS`.
It's pinned by tests at each layer: `TestRateLimitPolicy` covers the 0 -> -1
translation, `TestNewRestConfig` asserts the resulting config carries QPS=-1
with no pre-built limiter, and `TestNewClientSetAcceptsRestConfig` runs every
shape we generate through `kubernetes.NewForConfig`. The kind run in the
description is the live confirmation - ~1,900 binds/s on defaults couldn't
happen under a 5/10 limiter.
That said, a careful reader taking the wrong meaning from the 0 is the best
argument that -1 communicates better - I've changed the defaults to -1 in the
latest push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
