-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[POEM] Allow Users To Configure Max Action Container Concurrency Under Their Namespace Limit #5288
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
[POEM] Allow Users To Configure Max Action Container Concurrency Under Their Namespace Limit #5288
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5288 +/- ##
==========================================
- Coverage 79.96% 75.42% -4.54%
==========================================
Files 238 238
Lines 14122 14194 +72
Branches 599 606 +7
==========================================
- Hits 11292 10706 -586
- Misses 2830 3488 +658 ☔ View full report in Codecov by Sentry. |
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.
It generally looks good to me.
## Proposed changes: Architecture Diagram (optional), and Design | ||
|
||
Add a optional `maxContainerConcurrency` limit field to action documents in the limits section. This limit will be used in the scheduler when deciding | ||
if there is capacity for the action to scale up more containers. Previously, the scheduler was completely naive of functions across a namespace when provisioning |
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.
if there is capacity for the action to scale up more containers. Previously, the scheduler was completely naive of functions across a namespace when provisioning | |
if there is capacity for the action to scale out more containers. Previously, the scheduler was completely naive of functions across a namespace when provisioning |
the namespace limit as the capacity limit. Therefore, there is no action required or side effects or coordination required from the system admin wanting to lower the namespace limit. | ||
However, if the user wants to redeploy the same function with the same limit that is now over the namespace limit; the api will now reject the action upload until the action limit | ||
is lowered below the new namespace limit. | ||
- A user may want to update their action to go back to just relying on the namespace limit. Since updates to action documents copy over limits in the update even if not |
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.
Would this be included in the PR too?
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.
Yes it will, just haven't gotten a chance to add it yet
the action will rely on the namespace limit again. | ||
- When creating an action, the api will validate that your action container concurrency limit is less than or equal to the namespace concurrency limit. If it is greater, | ||
the upload will fail with a BadRequest and error message that the limit must be less than the namespace limit with the namespace limit value included in the message. | ||
- If the system admin lowers a namespace's concurrency limit below an amount that an existing action document has already configured, it will not break the action. |
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.
BTW, if the sum of the maxContainerConcurrency
of all actions exceeds the namespace limit, how does it guarantee fairness?
Do we leave it to the users?
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.
Yes it's the user's responsibility. The idea is that users can configure high priority functions to have a high limit such that it can still potentially get more than another low priority function and that lower priority action may not get up to its lower limit. As an example of a namespace with a limit of 20 and 3 actions A, B, and C:
- A configures limit of 5
- B configures limit of 5
- C configures limit of 15
C gets a burst of traffic that uses all 15 of its limit, there is now only 5 remaining for both A and B. That's fine as a user might want this level of scaling at C at the expense of the max A and B can now scale to being 2 or 3 each instead of 5.
Or the user can configure their limits perfectly up to their namespace limit to guarantee fairness such that A gets 5, B gets 5, and C gets 10. I think this level of flexibility to the user is a good thing.
I didn't want to refernce aws lambda explicitly, but in comparison to their concept of reserved concurrency for individual functions I think this provides more flexibility to the user. Reserved concurrency on lambda takes away from the total pool when configuring on a function so if I configure 5 to an action and the account limit is 20, there's now 15 capacity for other functions at all times. Well what if that function is not doing something most of the time? You've now taken away capacity permanently from your pool for a function that barely runs in exchange for a guarantee it will always be able to scale up to 5. In this proposal, you still have the ability to give yourself that guarantee if you provision evenly across your namespace as the user; but also have the flexibility to be smarter with what your traffic patterns look like in giving yourself additional overprovisioned / high priority capacity across your functions (which is sort of what serverless is all about.)
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.
ok, so this is mainly to limit an action that may consume huge resources.
What is the relationship between actions with/without maxContainerConcurrency
when they are invoked concurrently?
Even if action A, B, and C are configured like the above limits, any actions can still be invoked as long as there is enough capacity in the namespace, is that correct?
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.
Yes every action except A, B, and C. A, B, and C can only have up to the max of whatever their configured limit is regardless of whether their is still additional capacity in the namespace. If action D came in with no limit configured for itself, its action limit is just inferred by the scheduler to be the namespace limit so it can have up to the full 20.
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.
And thus if no limits are configured for any action in a namespace, the behavior remains identical to the existing behavior of openwhisk; any action can have up to the full namespace limit making this feature completely opt-in / non-breaking to the existing paradigm
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.
It looks good to me.
It is fully backward compatible.
@bdoyle0182 Do you want to update the Todo part or merge this as-is?
Description
POEM for allowing users to configure fairness within their namespace across their actions. This allows users to control how much an action can autoscale within the confines of the system admin controlled namespace concurrency limit.
Related issue and scope
My changes affect the following components
Types of changes
Checklist: