Skip to content
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.

Sign up for GitHub

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

docs: fix typos and add PR link #379

Merged
merged 5 commits into from
Mar 11, 2021
Merged

docs: fix typos and add PR link #379

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ebd7fa1
docs: fix typos and add PR link
callum-tait-pbx Mar 8, 2021
853dcee
docs: changes based on feedback
callum-tait-pbx Mar 10, 2021
b0056f5
docs: fixing numbers in list
callum-tait-pbx Mar 10, 2021
a490e37
docs: grammer
callum-tait-pbx Mar 10, 2021
99c4f0d
docs: better wording
callum-tait-pbx Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,12 @@ The scale out performance is controlled via the manager containers startup `--sy

**Benefits of this metric**
1. Supports named repositories allowing you to restrict the runner to a specified set of repositories server side.
2. Scales quickly (within the bounds of the syncPeriod) as it will spin up the number of runners based on the depth of the workflow queue
2. Scales the runner count based on the actual queue depth of the jobs meaning a more 1:1 scaling of runners to queued jobs.
3. Like all scaling metrics, you can manage workflow allocation to the RunnerDeployment through the use of [Github labels](#runner-labels).

**Drawbacks of this metric**
1. Repositories must be named within the scaling metric, maintaining a list of repositories may not be viable in larger environments or self-serve environments.
2. May not scale quick enough for some users needs
2. May not scale quick enough for some users needs. This metric is pull based and so the queue depth is polled as configured by the sync period, as a result scaling performance is bound by this sync period meaning there is a lag to scaling activity.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanation 💡

3. Relatively large amounts of API requests required to maintain this metric, you may run in API rate limiting issues depending on the size of your environment and how aggressive your sync period configuration is


Expand Down Expand Up @@ -355,14 +355,14 @@ The `HorizontalRunnerAutoscaler` will pole GitHub based on the configuration syn
**Helm Config :** `syncPeriod`

**Benefits of this metric**
1. Allows for multiple controllers to be deployed as each controller deployed is responsible for scaling their own runner pods on a per namespace basis.
2. Supports named repositories server side the same as the `TotalNumberOfQueuedAndInProgressWorkflowRuns` metric [#313](https://github.com/summerwind/actions-runner-controller/pull/313)
3. Supports github organisation wide scaling without maintaining an explicit list of repositories, this is especially useful for those that are working at a larger scale. [#223](https://github.com/summerwind/actions-runner-controller/pull/223)
4. Like all scaling metrics, you can manage workflow allocation to the RunnerDeployment through the use of [Github labels](#runner-labels)
5. Supports scaling runner count on both a percentage increase / descrease basis as well as on a fixed runner count basis [#223](https://github.com/summerwind/actions-runner-controller/pull/223) [#315](https://github.com/summerwind/actions-runner-controller/pull/315)
1. Supports named repositories server side the same as the `TotalNumberOfQueuedAndInProgressWorkflowRuns` metric [#313](https://github.com/summerwind/actions-runner-controller/pull/313)
2. Supports GitHub organisation wide scaling without maintaining an explicit list of repositories, this is especially useful for those that are working at a larger scale. [#223](https://github.com/summerwind/actions-runner-controller/pull/223)
3. Like all scaling metrics, you can manage workflow allocation to the RunnerDeployment through the use of [Github labels](#runner-labels)
4. Supports scaling desired runner count on both a percentage increase / decrease basis as well as on a fixed increase / decrease count basis [#223](https://github.com/summerwind/actions-runner-controller/pull/223) [#315](https://github.com/summerwind/actions-runner-controller/pull/315)

**Drawbacks of this metric**
1. May not scale quick enough for some users needs as we are scaling up and down based on indicative information rather than a direct count of the workflow queue depth
1. May not scale quick enough for some users needs. This metric is pull based and so the number of busy runners are polled as configured by the sync period, as a result scaling performance is bound by this sync period meaning there is a lag to scaling activity.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably you could try combining two. Webhook-based scaling will quickly add (possibly more than necessary) runners on demand and PercentageRunnersBusy will periodically "correct" the number of runners depending on runner statuses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the webhook scaling work inconjuction with the regular metrics? I saw you removed the need to define a metric when using webhooks, can you still define a metric and it will compliment the webhook scaling?

Copy link
Collaborator

@mumoshu mumoshu Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the webhook scaling work inconjuction with the regular metrics?

Yes

I saw you removed the need to define a metric when using webhooks

Yes. Actually, it works only in conjunction with regular metrics before/after I've removed the need to explicitly define metrics. It wasn't working when repository runners + TotalNumberOfQueuedAndInProgressWorkers combination so I fixed it in #381. Other combinations should have worked and will keep working.

can you still define a metric and it will compliment the webhook scaling?

Exactly! In other words, the webhook-based scaling works in combination with either TotalNumberOfQueuedAndInProgressWorkers or PercentageRunnersBusy. When you omited HRA.Spec.Metrics it defaults to TotalNumberOfQueuedAndInProgressWorkers. So you can say that it is still working in conjunction with the regular metrics.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed my mind and will soon change the default Metrics[] when ScaleUpTriggers isn't empty, so that ScaleUpTrigger can be actually used standalone. But the general idea described above should still stand.

2. We are scaling up and down based on indicative information rather than a count of the actual number of queued jobs and so the desired runner count is likely to under provision new runners or overprovision them relative to actual job queue depth, this may or may not be a problem for you.


Examples of each scaling type implemented with a `RunnerDeployment` backed by a `HorizontalRunnerAutoscaler`:
Expand Down