Repository navigation
Add script for calculating DB memory settings - #3541
denniskuijs wants to merge 11 commits into
Conversation
Ekhorn
left a comment
Ekhorn
left a comment
There was a problem hiding this comment.
This is an initial AI-assisted review.
The sizing steps in calculate_db_memory.sh follow the system administration guide, apart from work_mem, which comes out as 32 MB for the guide's 2 GB example where the guide uses 16 MB. Reading the metric before the stack goes down is the right moment for it. Three things in the current state would stop or break deployments, so they are worth fixing before this leaves draft:
- the sourced script leaves
set -eon for the rest of the deployment, so commands that may fail today abort it - the RAM values are calculated on the CI runner instead of the host
- the script exports bare MB values, while
profile/deploy.ymland the guide's compose example still expect the unit in the value
These were checked with the script from this PR and the same heredoc structure using stubbed curl and free, plus docker compose config and PostgreSQL 17.9 for value parsing. No real deployment was run.
| @@ -0,0 +1,30 @@ | |||
| #!/bin/bash | |||
| set -eo pipefail | |||
There was a problem hiding this comment.
deploy.sh sources this file, so set -eo pipefail stays on for the rest of the deployment, whichever branch below runs. Commands that may fail today now abort it:
docker volume rm or_deployment-datafails when the volume does not exist. A new host never gets past this, because the stack that would create the volume never starts. The rootdocker-compose.ymlhas no such volume, so deployments using it would fail every time, after the existing stack has been stopped.TOKEN=$(curl ...)fails on hosts without an EC2 metadata service, also with the stack stopped.aws s3 cpfails on hosts without that bucket, which reports a failed deployment after a successful start.
The docker volume rm case was reproduced with this script and a stubbed curl in a shell fed from a heredoc. The curl pipeline already has || true, so nothing here needs these options:
| set -eo pipefail | |
| # Sourced by deploy.sh, so this must not change shell options |
| export OR_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 40 / 100 ))" | ||
| export OR_KC_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 20 / 100 ))" | ||
| export OR_DB_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 25 / 100 ))" |
There was a problem hiding this comment.
The heredoc at line 191 is unquoted, so these $(free ...) and $(( )) expressions run on the CI runner before the script reaches the host. Elsewhere in the heredoc, host-side expansions are escaped as \$. With a stubbed 16 GB runner and 8 GB host, OR_DB_RAM came out as 3997 instead of 1975.
No compose file in this repo reads OR_RAM or OR_KC_RAM. Is a compose change still coming for those, or can they be removed?
| export OR_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 40 / 100 ))" | |
| export OR_KC_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 20 / 100 ))" | |
| export OR_DB_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 25 / 100 ))" | |
| HOST_RAM="\$(free -m | awk '/^Mem:/{print \$2}')" | |
| export OR_RAM="\$(( HOST_RAM * 40 / 100 ))" | |
| export OR_KC_RAM="\$(( HOST_RAM * 20 / 100 ))" | |
| export OR_DB_RAM="\$(( HOST_RAM * 25 / 100 ))" |
| MINIMUM_RAM_SIZE="$((((4 * CHUNK_SIZE + 1024 - 1) / 1024) * 1024))" | ||
|
|
||
| # Set shared buffers to 25% of minimum RAM size. | ||
| export OR_DB_SHARED_BUFFERS="$((MINIMUM_RAM_SIZE / 4))" |
There was a problem hiding this comment.
Exporting bare MB values works when compose adds the unit, as in shm_size: ${OR_DB_SHM:-156}MB. profile/deploy.yml and the compose example in the system administration guide still have the unit inside the default (${OR_DB_SHM:-156mb}), and this PR does not change them. In that form, a 1 GB chunk on an 8 GB host is read as:
| Variable | Value | Read as |
|---|---|---|
OR_DB_RAM |
1975 | 1975 bytes, which Docker rejects with Minimum memory limit allowed is 6MB |
OR_DB_SHM |
2048 | 2048 bytes |
OR_DB_SHARED_BUFFERS |
1024 | 8 MB (8 kB pages) |
OR_DB_CACHE_SIZE |
3072 | 24 MB (8 kB pages) |
OR_DB_MAINTENANCE_WORK_MEM |
512 | 512 kB |
OR_DB_WORK_MEM |
48 | 48 kB, below the 64 kB minimum, so PostgreSQL 17.9 exits with FATAL |
profile/deploy.yml and the guide need to change together with this script. With the values above, this form was checked against Compose and PostgreSQL:
shm_size: ${OR_DB_SHM:-156}MB
deploy:
resources:
limits:
memory: ${OR_DB_RAM:-512}MB
command:
- postgres
- -c
- shared_buffers=${OR_DB_SHARED_BUFFERS:-128}MB
- -c
- effective_cache_size=${OR_DB_CACHE_SIZE:-384}MB
- -c
- maintenance_work_mem=${OR_DB_MAINTENANCE_WORK_MEM:-56}MB
- -c
- work_mem=${OR_DB_WORK_MEM:-4}MBThis also affects custom projects. A compose file that follows the guide's current example gets the misread values above, and in the new form a value such as OR_DB_RAM=2G becomes 2GMB, which Compose rejects with invalid suffix. Both are worth a release note.
| echo 'Could not retrieve chunk size from query-exporter service, using defaults from Docker Compose' | ||
| else | ||
| # Calculate minimum RAM size and round up to the next whole GB (1.2 GB becomes 2 GB) | ||
| MINIMUM_RAM_SIZE="$((((4 * CHUNK_SIZE + 1024 - 1) / 1024) * 1024))" |
There was a problem hiding this comment.
A metric value of 0 passes the -z check above and turns every setting into 0. The query rounds to whole MB, so a largest chunk under 0.5 MB reports 0, as can happen on a new or lightly used install. PostgreSQL then exits with FATAL: 0 8kB is outside the valid range for parameter "shared_buffers". Treating 0 like a missing metric avoids that:
if [ "${CHUNK_SIZE:-0}" -eq 0 ]; then| # Calculate host RAM | ||
| export OR_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 40 / 100 ))" | ||
| export OR_KC_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 20 / 100 ))" | ||
| export OR_DB_RAM="$(( $(free -m | awk '/^Mem:/{print $2}') * 25 / 100 ))" |
There was a problem hiding this comment.
OR_DB_RAM is 25% of host RAM regardless of the chunk-based sizing, while the system administration guide treats P = 4 x N as the RAM the container needs: the largest chunk should never exceed 25% of the container limit. On an 8 GB host the limit is about 2 GB, so chunks above about 512 MB break that rule. With a 3 GB chunk, shared_buffers alone exceeds the limit, which also breaks the guide's golden rule and leads to the OOM kill it warns about. The fallback branch also says Compose defaults are used, but OR_DB_RAM is still replaced there.
Should OR_DB_RAM be at least MINIMUM_RAM_SIZE? When that does not fit on the host, the guide's remedy is a smaller chunk_time_interval, so a warning in the deployment log may be more useful than starting with settings the container cannot honor.
| fi | ||
|
|
||
| # Calculate DB memory settings | ||
| . ./temp/host_init/calculate_db_memory.sh |
There was a problem hiding this comment.
temp/env is loaded just above, so this script and the OR_DB_RAM export below overwrite any value a project set in .ci_cd/env or a secret, which .ci_cd/README.md describes as the way to configure compose variables. A host tuned by hand would change on its next deployment. Calculating only the values that are unset would keep those settings, and the README's variables section should mention the new behavior.
|
No description provided.