Extend the API priority and fairness KEP with observed request details #1218
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/L
MikeSpreitzer
force-pushed
the
add-request-evidence
branch
5 times, most recently
from
0619cce
to
80249f1
Compare
|
/cc @yue9944882 |
MikeSpreitzer
force-pushed
the
add-request-evidence
branch
2 times, most recently
from
959ae6a
to
652f908
Compare
| user.Info=&user.DefaultInfo{Name:"system:kube-scheduler", UID:"", | ||
| Groups:[]string{"system:authenticated"}, | ||
| Extra:map[string][]string(nil)} | ||
| as longrunning |
There was a problem hiding this comment.
there are other long running requests, like exec, port-forward, log, ... are they checked?
There was a problem hiding this comment.
The point of this KEP update is not to prove that the implementation is doing a good job of classification, so do not pay attention to the classification results shown here. The point of this update is only to provide some input to the process of designing the classification.
The existing implementation is identifying longrunning requests the same way that the max-in-flight filter does. Do you have reason to doubt that this is good?
There was a problem hiding this comment.
ok, that makes sense.
i don't have reason to doubt it, just figured they are very different than WATCH.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
MikeSpreitzer
force-pushed
the
add-request-evidence
branch
from
652f908
to
aafb153
Compare
k8s-ci-robot
removed
the
lgtm
|
I just revised slightly to emphasize the point I made in #1218 (comment) |
| implementation of this KEP, because that is the only easy source of | ||
| the relevant request details. Do not pay attention to the | ||
| classification results evident in these log messages, they are not | ||
| what is important here; what is important is the `requestInfo` and the |
There was a problem hiding this comment.
Thanks for calling out what you're trying to represent. Because these are simple %v statements, you can only interpret the values if you know exactly which level of code they came from. This makes sense for debugging purposes (you know which version you are), but doesn't make as much sense for a doc like this that needs to span multiple releases.
Perhaps you could render them as json or yaml or something that is self-identifying for fields in the future. Future us will want more detail than true for instance.
MikeSpreitzer
force-pushed
the
add-request-evidence
branch
2 times, most recently
from
1eb770c
to
a4f96b6
Compare
MikeSpreitzer
force-pushed
the
add-request-evidence
branch
from
a4f96b6
to
a2ee24e
Compare
|
My latest revision replaces the cryptic |
|
Thanks, that helps. /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, MikeSpreitzer, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
The purpose of this PR is to add to the KEP some supporting details of observed requests, so that we can understand the traffic we are trying to classify.