Add scheduler overprovision for new actions before namespace throttling #5284

bdoyle0182 · 2022-07-14T22:54:34Z

Description

This is a wip and trying to get feedback if this is an option we could support. This would add two new configs for the scheduler:

  ```allow-over-provision-before-throttle = true
  namespace-over-provision-before-throttle-ratio = 1.5``` (would it be preferred by people that this is a fixed value rather than ratio)

If the first config is false, the ratio is not used anywhere. But the purpose of this is to prevent deadlocking on the new scheduler for actions within a namespace due to the new concept of namespace throttling. Since the scheduler can be aggressive with initially over-provisioning, you can get into a scenario where a single action uses up all of the container concurrency of the namespace before another action gets a change to run. What the code change below attempts to do is up the container concurrency limit to the over provision threshold to allow new actions to have a chance to run. This is really needed if one action depends on another. i.e. action a attempts to execute action b, but action a has all of the action concurrency for the namespace. The workload now will never make progress and action a will only ever fail all of its executions. Ultimately the right thing to do is scale out the namespace, but that's manual by a human and can't be done until impact has already occurred and this helps mitigate by slowing throughput rather than total failures.

Example of how it works:

Namespace A has a container concurrency limit of 30
Action A uses all 30 containers
With an over provision before throttle ratio of 1.5, the namespace can actually have a max total of 45 containers.
Action B comes in and since the namespace has hit its initial limit, the scheduler gives it max of 1 container until the number of containers for the namespace goes below it's normal limit of 30. Since it's not likely provisioned enough it will get action throttled which is still better than not letting any traffic at all.
The namespace can get a total of 15 additional actions with a max of 1 container before it is now completely over-provisioned and gets namespace throttled so no additional containers can be created.

I'll admit this is a little hacky, but I do think we need to account for this throttling case until we are able to support action level container concurrency limits. So any better ideas are welcome.

Related issue and scope

I opened an issue to propose and discuss this change (#????)

My changes affect the following components

Types of changes

Bug fix (generally a non-breaking change which closes an issue).
Enhancement or new feature (adds new functionality).
Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

I signed an Apache CLA.
I reviewed the style guides and followed the recommendations (Travis CI will check :).
I added tests to cover my changes.
My changes require further changes to the documentation.
I updated the documentation where necessary.

core/scheduler/src/main/resources/application.conf

bdoyle0182 · 2022-07-14T22:58:16Z

...duler/src/main/scala/org/apache/openwhisk/core/scheduler/queue/SchedulingDecisionMaker.scala

@@ -70,7 +70,17 @@ class SchedulingDecisionMaker(
 case _ => Future.successful(DecisionResults(Pausing, 0))
 }
 } else {
- val capacity = limit - existingContainerCountInNs - inProgressContainerCountInNs
+ val capacity = if (schedulingConfig.allowOverProvisionBeforeThrottle && totalContainers == 0) {


if feature is turned on and the action has no containers yet, give it a capacity of 1 on first decision iteration if total containers in use for the namespace are less than the limit * over provision ratio value, thereafter do the normal capacity calculation.

If this is still over the limit, give 0 capacity and follow the normal code path to turn on namespace throttling

bdoyle0182 · 2022-07-14T23:35:38Z

...duler/src/main/scala/org/apache/openwhisk/core/scheduler/queue/SchedulingDecisionMaker.scala

@@ -79,7 +89,7 @@ class SchedulingDecisionMaker(
 *
 * However, if the container exists(totalContainers != 0), the activation is not treated as a failure and the activation is delivered to the container.
 */
- case Running =>
+ case Running if totalContainers == 0 || !schedulingConfig.allowOverProvisionBeforeThrottle =>


If capacity == 0 and feature is turned on and still no containers for action could be created, then turn on namespace throttling. If capacity == 0 and feature is turned off and no containers exist, just skip any considerations and turn on namespace throttling same as existing behavior

I feel there is a subtle difference in the semantics when the feature is enabled.
Let's say there are one action utilizing 30 containers, and 15 actions with 1 container each.
The totalContainers is always bigger than 0. Then isn't throttling enabled?

The throttling would be enabled when an activation for another action arrives, and no container is created.
I think it would be better to enable the namespace throttling when the number of containers reaches namespace limit * overprovisoinRatio.

the reasoning for this is so that activations can continue processing when the 15th action gets the last 1 container. If the 15th action can handle the throughput at this point it wouldn't want namespace throttling enabled so it can continue processing activations. You wouldn't want to stop activation processing until the 16th action comes in.

Or am I misremembering and namespace throttling only impacts new actions with 0 containers?

Or am I misremembering and namespace throttling only impacts new actions with 0 containers?

This is correct.
The namespace throttling is used when there is no action throttling data in ETCD which means no queue is created for the action.
https://github.com/apache/openwhisk/blob/master/core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/FPCPoolBalancer.scala#L683

I believe I have updated this behavior and added a commit to what you requested in the latest commit.

I think this pr should be good to go except for one remaining issue I just thought of if one of the 15 actions that got the overprovisioning slots and namespace throttling is enabled and one of those actions containers are removed then I'm not sure if namespace throttling will be correctly disabled, I just need to verify that before merging tomorrow.

Okay added case to correctly disable namespace throttling if the container that frees up space is a container from the over-provisioning space in the latest commit. Would appreciate approval on this when you have some time @style95

bdoyle0182 · 2022-10-25T00:29:29Z

this is now ready for review @style95

codecov-commenter · 2022-10-25T01:01:25Z

Codecov Report

Merging #5284 (4924e56) into master (0f4b0c2) will increase coverage by 18.31%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master    #5284       +/-   ##
===========================================
+ Coverage   58.25%   76.57%   +18.31%     
===========================================
  Files         240      240               
  Lines       14383    14391        +8     
  Branches      614      605        -9     
===========================================
+ Hits         8379    11020     +2641     
+ Misses       6004     3371     -2633

Impacted Files	Coverage Δ
...rg/apache/openwhisk/core/scheduler/Scheduler.scala	`10.75% <ø> (+0.63%)`	⬆️
...core/scheduler/queue/SchedulingDecisionMaker.scala	`97.89% <100.00%> (+1.34%)`	⬆️
...enwhisk/core/entitlement/ActivationThrottler.scala	`80.00% <0.00%> (-20.00%)`	⬇️
...ntainerpool/v2/FunctionPullingContainerProxy.scala	`78.34% <0.00%> (ø)`
...e/openwhisk/core/scheduler/queue/MemoryQueue.scala	`82.12% <0.00%> (+0.19%)`	⬆️
.../apache/openwhisk/core/controller/Controller.scala	`83.47% <0.00%> (+0.82%)`	⬆️
...pache/openwhisk/core/entitlement/Entitlement.scala	`82.64% <0.00%> (+0.82%)`	⬆️
.../org/apache/openwhisk/core/entity/EntityPath.scala	`100.00% <0.00%> (+1.88%)`	⬆️
... and 131 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…rprovisioning has space

style95

LGTM with minor nits.

...duler/src/main/scala/org/apache/openwhisk/core/scheduler/queue/SchedulingDecisionMaker.scala

...test/scala/org/apache/openwhisk/core/scheduler/queue/test/SchedulingDecisionMakerTests.scala

…ng (apache#5284) * initial attempt * tests * fix tests * enable throttling when last capacity used in overprovisioning * add case to correctly disable namespace throttling when namespace overprovisioning has space * feedback Co-authored-by: Brendan Doyle <brendand@qualtrics.com>

Brendan Doyle added 3 commits June 16, 2022 09:33

initial attempt

0400198

Merge branch 'master' into add-scheduler-overprovision-for-new-actions

83674e5

Merge branch 'master' into add-scheduler-overprovision-for-new-actions

a6ea663

bdoyle0182 commented Jul 14, 2022

View reviewed changes

core/scheduler/src/main/resources/application.conf Outdated Show resolved Hide resolved

bdoyle0182 commented Jul 14, 2022

View reviewed changes

bdoyle0182 requested a review from style95 July 14, 2022 23:35

Brendan Doyle added 2 commits October 19, 2022 16:17

Merge branch 'master' into add-scheduler-overprovision-for-new-actions

4478b64

tests

0d586ac

bdoyle0182 changed the title ~~WIP: Add scheduler overprovision for new actions before namespace throttling~~ Add scheduler overprovision for new actions before namespace throttling Oct 25, 2022

Brendan Doyle added 3 commits October 27, 2022 17:59

fix tests

bbcd96a

enable throttling when last capacity used in overprovisioning

664f620

add case to correctly disable namespace throttling when namespace ove…

136a117

…rprovisioning has space

style95 approved these changes Nov 1, 2022

View reviewed changes

Brendan Doyle added 3 commits October 31, 2022 22:27

Merge branch 'master' into add-scheduler-overprovision-for-new-actions

eb30c00

feedback

9480428

Merge branch 'master' into add-scheduler-overprovision-for-new-actions

4924e56

bdoyle0182 merged commit 077fb6d into apache:master Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add scheduler overprovision for new actions before namespace throttling #5284

Add scheduler overprovision for new actions before namespace throttling #5284

bdoyle0182 commented Jul 14, 2022

bdoyle0182 Jul 14, 2022

bdoyle0182 Jul 14, 2022

style95 Oct 26, 2022

bdoyle0182 Oct 26, 2022

style95 Oct 28, 2022

bdoyle0182 Oct 28, 2022

bdoyle0182 Oct 31, 2022

bdoyle0182 commented Oct 25, 2022

codecov-commenter commented Oct 25, 2022 •

edited

Loading

style95 left a comment

Add scheduler overprovision for new actions before namespace throttling #5284

Add scheduler overprovision for new actions before namespace throttling #5284

Conversation

bdoyle0182 commented Jul 14, 2022

Description

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

bdoyle0182 Jul 14, 2022

Choose a reason for hiding this comment

bdoyle0182 Jul 14, 2022

Choose a reason for hiding this comment

style95 Oct 26, 2022

Choose a reason for hiding this comment

bdoyle0182 Oct 26, 2022

Choose a reason for hiding this comment

style95 Oct 28, 2022

Choose a reason for hiding this comment

bdoyle0182 Oct 28, 2022

Choose a reason for hiding this comment

bdoyle0182 Oct 31, 2022

Choose a reason for hiding this comment

bdoyle0182 commented Oct 25, 2022

codecov-commenter commented Oct 25, 2022 • edited Loading

Codecov Report

style95 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 25, 2022 •

edited

Loading