Skip to content

Conversation

@haouc
Copy link
Contributor

@haouc haouc commented Sep 6, 2023

Issue #, if available:
#295
Description of changes:
We are seeing pod queue latency in some extreme use case. Increasing the workers to help on performance.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haouc haouc requested a review from a team as a code owner September 6, 2023 23:55
@sushrk
Copy link
Contributor

sushrk commented Sep 7, 2023

General comment for my understanding, how did we choose these values?

@haouc
Copy link
Contributor Author

haouc commented Sep 7, 2023

We are still in the process to tune the values. I don't think there is any standards or guidance from upstream on those values. Since they are pretty lightweight and increasing to 20 for pods and 10 for nodes are not large number, I think it is worth to make the change and watch how it works.
In small/medium scale load tests, I didn't see significant improvement by increasing the goroutines. But using 3 for pods and 4 for nodes are likely constraining queue performance during large scaling. Also considering the queueing frequency, pods need more goroutines than nodes to work on queues.

@haouc
Copy link
Contributor Author

haouc commented Sep 7, 2023

After offline sync with @sushrk , we agree to increase the concurrency on node and pod controller by stages to minimize risks. If this change works stably, we will increase them to 10 for node and 20 for pod controller in the next release.

Copy link
Contributor

@sushrk sushrk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Hao.
We will be tuning this in the future as per performance metrics.

@haouc haouc merged commit bbe40dc into aws:master Sep 7, 2023
@haouc haouc deleted the workers branch September 7, 2023 21:41
sushrk pushed a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Oct 13, 2023
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.

2 participants