KEP-1040: periodically adjusted borrowing between priority levels in APF #3391

MikeSpreitzer · 2022-06-13T20:16:40Z

One-line PR description: describe how to add borrowing to API Priority and Fairness via dynamically adjusted concurrency limits

Issue link:

Other comments: This is one of four approaches being considered; the others are [Draft] KEP-1040: directed borrowing between priority levels in APF #3290, [Draft] KEP-1040: all-to-all borrowing between priority levels in APF #3389, and [Draft] KEP-1040: low-to-high borrowing between priority levels in APF #3394 . This is structured as two commits: the first equals [Draft] KEP-1040: directed borrowing between priority levels in APF #3290, and the second has the changes from [Draft] KEP-1040: directed borrowing between priority levels in APF #3290.

MikeSpreitzer · 2022-06-14T01:38:36Z

/cc @wojtek-t
/cc @lavalamp
@tkashem

keps/sig-api-machinery/1040-priority-and-fairness/README.md

deads2k · 2022-06-16T16:12:28Z

keps/sig-api-machinery/1040-priority-and-fairness/README.md

+the inputs to the adjustment logic in order to dampen control
+gyrations, in a way that lets a priority level reclaim lent seats at
+the nearest adjustment time.  The adjustments take into account the
+high watermark `HighSD(i)`, time-weighted average `AvgSD(i)`, and


letters are cheap. SD is SeatDepth?

"SeatDemand"

I expanded all the "SD" in the latest revision.

deads2k · 2022-06-16T16:15:18Z

keps/sig-api-machinery/1040-priority-and-fairness/README.md

+EnvelopeSD when it exceeds SmoothSD.  The rule for updating priority
+level `i`'s SmoothSD at the end of an adjustment period is
+`SmoothSD(i) := max( EnvelopeSD(i), A*SmoothSD(i) + (1-A)*Envelope(i)
+)`.  The command line flag `--seat-demand-history-fraction` with a


a cluster-admin doesn't seem likely to choose a better A than we will

That's what default values are for?

Also, I am not sure what value to use. I made a rough guess, but it is only that.

deads2k · 2022-06-16T16:27:49Z

keps/sig-api-machinery/1040-priority-and-fairness/README.md

+```
+CurrentCL(i) = FairFrac * Target(i)  if FairFrac * Target(i) >= MinCurrentCL(i)
+CurrentCL(i) = MinCurrentCL(i)       if FairFrac * Target(i) <= MinCurrentCL(i)
+```


At this point, are guaranteed that CurrentCLSum (sum of all CurrentCL(i)) is equal to ServerCL? I'm not seeing how that is guaranteed.

The point is that FairFrac is a variable here that has to be solved for. Higher FairFrac makes for higher sum of CurrentCL; the problem is to find the value of FairFrac that makes the CurrentCL sum to ServerCL. This is analogous to the max-min fairness problem, which is to pick a fair allocation that applies to all the requesters that want at least that much (the other requesters get only what they ask for, and the sum of allocations is limited by the available capacity).

I revised the write-up to go through this a little more slowly, hopefully it is clearer now (if a bit longer).

deads2k · 2022-06-16T18:22:13Z

This design is more complicated, but with the metrics listed at the bottom I think we have enough to explain why certain requests got 429s. I'm out next week, so I'll leave lgtm to @wojtek-t

/approve

MikeSpreitzer · 2022-06-17T04:29:06Z

BTW, @deads2k , what is your position on whether to keep the new Priority field?

wojtek-t

I'm fine with this PR modulo getting rid of priority field which we're not using now.

wojtek-t · 2022-06-19T08:53:25Z

keps/sig-api-machinery/1040-priority-and-fairness/README.md

+The following display shows the two new fields along with the updated
+description for the `AssuredConcurrencyShares` field, in `v1beta2`.
+
+**Note**: currently this design does not use the `Priority` field for


Let's get rid of this field now.

If it appears needed, we can always add that later, but until needed, let's remove it to avoid confusion.

I just pushed a revision that removes the Priority field.

wojtek-t · 2022-06-19T08:54:24Z

keps/sig-api-machinery/1040-priority-and-fairness/README.md

+administrators of the cluster have scripts/clients that maintain some
+out-of-tree PriorityLevelConfiguration objects; these, naturally, do
+not specify a value for the `priority` field.  The default behavior
+for `priority` is designed to do something more natural and convenient


This comment isn't up-to-date I think, as we don't use priority field at all.

Remove?

Yes, I deleted it as part of removing the Priority field.

Also updated outdated text about directed borrowing.

wojtek-t · 2022-06-20T16:27:55Z

/label tide/merge-method-squash

/lgtm
/approve

We should probably also add that to the tracking sheet:
https://docs.google.com/spreadsheets/d/1Lnft8598eIQsqBp8W6X_LwaqBNZViYssQoFgGS8aJ3g/edit?pli=1#gid=0

k8s-ci-robot · 2022-06-20T16:28:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, MikeSpreitzer, wojtek-t

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:

~~keps/sig-api-machinery/OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

MikeSpreitzer added 2 commits June 13, 2022 14:43

Draft borrowing between priority levels in APF, high borrows from low

bd3a3f3

Do borrowing by periodic adjustment of concurrency limits

a3fd87c

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 13, 2022

k8s-ci-robot requested a review from deads2k June 13, 2022 20:16

k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jun 13, 2022

k8s-ci-robot requested a review from fedebongio June 13, 2022 20:16

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 13, 2022

This was referenced Jun 13, 2022

[Draft] KEP-1040: all-to-all borrowing between priority levels in APF #3389

Closed

[Draft] KEP-1040: directed borrowing between priority levels in APF #3290

Closed

[Draft] KEP-1040: low-to-high borrowing between priority levels in APF #3394

Closed

k8s-ci-robot requested review from lavalamp and wojtek-t June 14, 2022 01:38

deads2k reviewed Jun 15, 2022

View reviewed changes

keps/sig-api-machinery/1040-priority-and-fairness/README.md Outdated Show resolved Hide resolved

deads2k reviewed Jun 15, 2022

View reviewed changes

keps/sig-api-machinery/1040-priority-and-fairness/README.md Outdated Show resolved Hide resolved

MikeSpreitzer force-pushed the apf-add-borrowing-adjustment branch from a92ba9d to 41cf392 Compare June 16, 2022 04:29

Update borrowing adjustment: smooth input, max-min problem

fbedda1

MikeSpreitzer force-pushed the apf-add-borrowing-adjustment branch from 41cf392 to fbedda1 Compare June 16, 2022 04:34

deads2k reviewed Jun 16, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 16, 2022

Revise borrowing adjustment write-up in response to review

b2017df

MikeSpreitzer force-pushed the apf-add-borrowing-adjustment branch from 2c0b127 to b2017df Compare June 17, 2022 04:31

wojtek-t reviewed Jun 19, 2022

View reviewed changes

Removed unused Priority field

2998b40

Also updated outdated text about directed borrowing.

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 20, 2022

k8s-ci-robot assigned wojtek-t Jun 20, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2022

k8s-ci-robot merged commit ae30fc8 into kubernetes:master Jun 20, 2022

k8s-ci-robot added this to the v1.25 milestone Jun 20, 2022

palnabarun mentioned this pull request Jul 26, 2022

KEP Manifest Generator palnabarun/enhancements#2

Closed

MikeSpreitzer deleted the apf-add-borrowing-adjustment branch August 26, 2022 19:22

tkashem mentioned this pull request Sep 16, 2022

add v1beta3 for Priority And Fairness kubernetes/kubernetes#112306

Merged

tkashem mentioned this pull request Oct 3, 2022

add new fields for borrowing in flowcontrol api kubernetes/kubernetes#112830

Closed

MikeSpreitzer changed the title ~~[Draft] KEP-1040: periodically adjusted borrowing between priority levels in APF~~ KEP-1040: periodically adjusted borrowing between priority levels in APF Oct 27, 2022

This was referenced Oct 31, 2022

Define metrics for API Priority and Fairness borrowing kubernetes/kubernetes#113483

Merged

Add borrowing between priority levels in APF kubernetes/kubernetes#113485

Merged

KEP-1040: periodically adjusted borrowing between priority levels in APF #3391

KEP-1040: periodically adjusted borrowing between priority levels in APF #3391

Uh oh!

Conversation

MikeSpreitzer commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeSpreitzer commented Jun 14, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeSpreitzer Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deads2k commented Jun 16, 2022

Uh oh!

MikeSpreitzer commented Jun 17, 2022

Uh oh!

wojtek-t left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojtek-t commented Jun 20, 2022

Uh oh!

k8s-ci-robot commented Jun 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MikeSpreitzer commented Jun 13, 2022 •

edited

Loading

MikeSpreitzer Jun 16, 2022 •

edited

Loading