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

Add resource.[requests|limits] to reconciled objects #703

Closed
embano1 opened this issue Apr 8, 2022 · 5 comments · Fixed by #771
Closed

Add resource.[requests|limits] to reconciled objects #703

embano1 opened this issue Apr 8, 2022 · 5 comments · Fixed by #771
Assignees
Milestone

Comments

@embano1
Copy link
Contributor

embano1 commented Apr 8, 2022

Problem
Skimming through the reconciler codebase/folders, it seems that none of the deployments/pods created have any resources specified. This can lead to performance and stability issues, especially not meeting the documented scaling behavior (#408).

Persona:
Operators and users.

Exit Criteria
Documented scaling limits (benchmarks) are met even when deploying the resources in a constrained (overutilized) cluster.

Time Estimate (optional):
Implementation is straightforward (add requests and/or limits), but finding the right values might take time (in benchmarks).

Additional context (optional)
Even without benchmarking, adding sane defaults (at least requests) should be done for the various reconciled resources.

cc/ @salaboy @gabo1208

@gab-satchi
Copy link
Contributor

cc @gabo1208 if we can get the resource requirements for when the performance tests were run, that could be a good baseline for the limit

@gabo1208
Copy link
Contributor

gabo1208 commented May 3, 2022

Sure but you mean the ones defined on the files, totals of the cluster or anything else?

@gab-satchi
Copy link
Contributor

Just for the ingress/dispatcher pods. The cluster requirements can be customized by users already but we'd need sane values for the ingress and dispatcher pods

@gabo1208
Copy link
Contributor

gabo1208 commented May 9, 2022

/assign

@gabo1208
Copy link
Contributor

gabo1208 commented May 18, 2022

For setting the current limits in the Dispatcher, Adapter:

Resources: corev1.ResourceRequirements{
        Requests: corev1.ResourceList{
	        corev1.ResourceCPU:    resource.MustParse("300m"),
		corev1.ResourceMemory: resource.MustParse("64Mi")},
	Limits: corev1.ResourceList{
		corev1.ResourceCPU:    resource.MustParse("4000m"),
		corev1.ResourceMemory: resource.MustParse("600Mi")}
}

and Ingress:

Resources: corev1.ResourceRequirements{
        Requests: corev1.ResourceList{
	        corev1.ResourceCPU:    resource.MustParse("150m"),
		corev1.ResourceMemory: resource.MustParse("32Mi")},
	Limits: corev1.ResourceList{
		corev1.ResourceCPU:    resource.MustParse("1000m"),
		corev1.ResourceMemory: resource.MustParse("400Mi")}
}

Env:
Knative v1.4.0
RabbitMQ v10.3.1
RabbitMQ's Topology operator version 1.5.0
RabbitMQ's Cluster Operator version 1.13.0
Running on a GKE 1.23 cluster e2-standard-16 instances

We used the performance tests as indicators, and divided them into three cases:

  • 500msgs/s constant load for 5 minutes, parallelism of 100: Here the consumption is not too high, Ingress: 9mb and 180m cpu, and Dispatcher/Adapter: 12mb and 270m cpu so the quests are a little above this values
  • For workloads 1000-1200 msgs/s of constant load for 5 minutes, parallelism of 1000: Ingress: 27mb and 270m and Dispatcher/Adapter: 90mb and 1000m cpu
  • For workloads > 1400msgs/s of constant load for 5 minutes: The system becomes unstable, so the limits are there to avoid spikes in the cpu and memory consumption when RabbitMQ starts to queue messages and the Dispatcher and Adapter behaves strange, i manage to get up to 2 gygs of memory being used and up to 13000m of cpu in this case.
  • The message memory was around 500bytes, that's why the memory have some extra room to grow on the values.

@gabo1208 gabo1208 reopened this May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants