PR#117: Define resource limits for the database container - taskotron/resultsdb

taskotron / resultsdb

#117 Define resource limits for the database container

Merged 5 years ago by jskladan. Opened 5 years ago by csomh.

taskotron/ csomh/resultsdb resource-limits into develop

Define resource limits for the database container

Hunor Csomortáni • 5 years ago

ae3cb8e

file modified

		`@@ -137,6 +137,11 @@`
		`imagePullPolicy: Always`
		`ports:`
		`- containerPort: 5432`
		`+ protocol: TCP`
		`+ resources:`
		`+ limits:`
		`+ memory: 512Mi`
		`+ cpu: 0.4`
		`readinessProbe:`
		`timeoutSeconds: 1`
		`initialDelaySeconds: 5`
		`@@ -259,6 +264,7 @@`
		`resources:`
		`limits:`
		`memory: 384Mi`
		`+ cpu: 0.3`
		`volumes:`
		`- name: config-volume`
		`secret:`
		`@@ -345,6 +351,7 @@`
		`resources:`
		`limits:`
		`memory: 384Mi`
		`+ cpu: 0.3`
		`volumes:`
		`- name: config-volume`
		`secret:`

csomh commented 5 years ago

Otherwise PostgreSQL will run out of memory in projects where
the default limits are lower than this.

Signed-off-by: Hunor Csomortáni csomh@redhat.com

csomh commented 5 years ago

@mprahl or @ralph: please take a look.

mprahl commented 5 years ago

@csomh looks good to me. Any reason you picked 0.4 instead of a nice round 0.5 CPU limit? :)

rebased onto a05a35ee1cfe07affc3aa53736b5272b3d18b893

5 years ago

rebased onto a718f9ab3baf54674b0239e09c946925001e2c00

5 years ago

csomh commented 5 years ago

That's a good question @mprahl! I have no clue :p

Right now I would guess that it came from the default CPU limit for
UpShift projects (300 millicores), increased with 100 millicores,
which made things 'feel' better.

I've quickly read through these
docs
again, but could not find anything about recommended strategies for
setting these limits.

FYI: I've noticed that the resultsdb DeployementConfigs were missing
CPU limits, so I've added those too. Better to be explicit about these
things, they say :)

mprahl commented 5 years ago

Okay, still :thumbsup:

ralph commented 5 years ago

:+1:

jskladan commented 5 years ago

If this is fine by you, I'll happily merge this after it's rebased to current HEAD. Let me know, from the comments, this seemed to be under development/discussion still :) Thanks!

ralph commented 5 years ago

@csomh is away for a few days. Let's wait for him to come back to see what he thinks.

rebased onto ae3cb8e

5 years ago

csomh commented 5 years ago

This was rebased, it's ready to merge, @jskladan. Thanks!