docs: fix typos and add PR link #379
Conversation
|
@mumoshu I think the following benefit listed in the README.md for the It came from the below snipped taken from the #223: I don't quite get what the author is getting at. Are you able to figure it out so the benefit in the README can be updated to be correct? I'm struggling to grasp how this is a benefit unique to the |
|
@callum-tait-pbx Before #355, However,
It wasn't working like that until #380. |
|
I've reread the doc and I would say that this part of the doc:
looks obsolete and can be removed now. After #355 and #380, both scaling metrics(strategies) should work er eithper namespace or per cluster basis without any problem. |
|
@mumoshu that is probably worded better |
| 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. |
|
|
||
| **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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
LGTM. Thank you so much for your continuous effort @callum-tait-pbx
… enabled Relates to #379 (comment) Relates to #377 (comment) When you defined HRA.Spec.ScaleUpTriggers[] but HRA.Spec.Metrics[], the HRA controller will now enable ScaleUpTriggers alone and insteaed of automatically enabling TotalNumberOfQueuedAndInProgressWorkflowRuns. This allows you to use ScaleUpTriggers alone, so that the autoscaling is done without calling GitHub API at all, which should grealy decrease the change of GitHub API calls get rate-limited.
… enabled (#391) Relates to #379 (comment) Relates to #377 (comment) When you defined HRA.Spec.ScaleUpTriggers[] but HRA.Spec.Metrics[], the HRA controller will now enable ScaleUpTriggers alone and insteaed of automatically enabling TotalNumberOfQueuedAndInProgressWorkflowRuns. This allows you to use ScaleUpTriggers alone, so that the autoscaling is done without calling GitHub API at all, which should grealy decrease the change of GitHub API calls get rate-limited.
minor changes to docs I noticed