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.

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

Do not delay min/maxReplicas propagation from HRA to RD due to caching #406

Merged

Conversation

mumoshu
Copy link
Collaborator

@mumoshu mumoshu commented Mar 19, 2021

As part of #282, I have introduced some caching mechanism to avoid excessive GitHub API calls due to the autoscaling calculation involving GitHub API calls is executed on each Webhook event.

Apparently, it was saving the wrong value in the cache- The value was one after applying HRA.Spec.{Max,Min}Replicas so manual changes to {Max,Min}Replicas doesn't affect RunnerDeployment.Spec.Replicas until the cache expires. This isn't what I had wanted.

This patch fixes that, by changing the value being cached to one before applying {Min,Max}Replicas.

Additionally, I've also updated logging so that you observe which number was fetched from cache, and what number was suggested by either TotalNumberOfQueuedAndInProgressWorkflowRuns or PercentageRunnersBusy, and what was the final number used as the desired-replicas(after applying {Min,Max}Replicas).

Follow-up for #282

@mumoshu mumoshu force-pushed the do-not-delay-min-max-replicas-propagation-due-to-caching branch from cf75b80 to c4eb558 Compare March 19, 2021 02:39
As part of #282, I have introduced some caching mechanism to avoid excessive GitHub API calls due to the autoscaling calculation involving GitHub API calls is executed on each Webhook event.

Apparently, it was saving the wrong value in the cache- The value was one after applying `HRA.Spec.{Max,Min}Replicas` so manual changes to {Max,Min}Replicas doesn't affect RunnerDeployment.Spec.Replicas until the cache expires. This isn't what I had wanted.

This patch fixes that, by changing the value being cached to one before applying {Min,Max}Replicas.

Follow-up for #282
@mumoshu mumoshu force-pushed the do-not-delay-min-max-replicas-propagation-due-to-caching branch from c4eb558 to 92fdda9 Compare March 19, 2021 03:43
@mumoshu mumoshu merged commit f6ab66c into master Mar 19, 2021
@mumoshu mumoshu deleted the do-not-delay-min-max-replicas-propagation-due-to-caching branch March 19, 2021 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant