Increase api latency threshold for cluster-scoped list calls #52732
Conversation
shyamjvs
added
the
release-note-none
k8s-ci-robot
added
sig/scalability
|
/test all |
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
|
@liggitt @smarterclayton - Can one of you PTAL? |
|
/test pull-kubernetes-e2e-gce-gpu |
test/e2e/framework/metrics_util.go
Outdated
| // as list response sizes are bigger in general for big clusters. We also use a higher threshold | ||
| // for list calls with cluster scope (all namespaces). | ||
| apiListCallLatencyThreshold time.Duration = 5 * time.Second | ||
| apiClusterScopeListCallThreshold time.Duration = 10 * time.Second |
There was a problem hiding this comment.
Do you literally mean cluster scoped (i.e., node objects) or cross-namespace lists? If the latter, please choose a less confusing name.
There was a problem hiding this comment.
I meant the former (which includes both non-namespaced and all-namespaced calls) - changed the comment to make it clearer.
test/e2e/framework/metrics_util.go
Outdated
| if !isListCall || | ||
| !isBigCluster || | ||
| (!isClusterScopedCall && latency > apiListCallLatencyThreshold) || | ||
| (latency > apiClusterScopeListCallThreshold) { |
There was a problem hiding this comment.
This is a really confusing logic statement. I suggest one if/switch to set the threshold, and then this if should read if latency > threshold.
shyamjvs
force-pushed
the
fix-metrics-perf-tests
branch
from
6738e88
to
f373645
Compare
k8s-github-robot
removed
the
lgtm
|
Yay, more precise metrics! :) |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gmarek, shyamjvs, spiffxp Associated issue requirement bypassed by: spiffxp The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
/test pull-kubernetes-kubemark-e2e-gce-big |
|
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.. |
|
cc @jpbetz |
Recent change from @smarterclayton (#52237) added scope to apiserver metrics. As a result, our current threshold for list calls is no longer sufficient for all-namespace calls which are now being measured separately from namespaced lists. For e.g (from our last 5k run):
cc @kubernetes/sig-scalability-misc @kubernetes/sig-api-machinery-misc @wojtek-t