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

Set memory limit on server's docker containers #468

Merged

Conversation

santiagorodriguez96
Copy link
Collaborator

@santiagorodriguez96 santiagorodriguez96 commented May 3, 2024

Motivation

We were running into issues where the docker containers were increasing the memory usage over time until after a couple of days taking too much memory from the server which causes the server to throttle and respond very slowly until eventually going down.

Details

By adding a memory limit, docker will kill the container when it gets to the limit of memory usage and the docker container will be restarted – as its started up with the --restart unless-stopped option.

However, we need to add the logic for cleaning the server.pid if exists when starting the server given that when the docker container starts itself after being killed it runs into an error when starting the server as the server.pid already exists:

a server is already running (pid: 1, file: /rails/tmp/pids/server.pid)

@@ -3,6 +3,11 @@
# If running the rails server then create or migrate existing database
if [ "${1}" == "./bin/rails" ] && [ "${2}" == "server" ]; then
./bin/rails db:prepare

file="/rails/tmp/pids/server.pid"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
file="/rails/tmp/pids/server.pid"
file="./tmp/pids/server.pid"

will this work?
so this file doesn't have to know where the app is running

@@ -8,6 +8,8 @@ servers:
traefik.http.routers.micarrera_secure-web-staging.rule: Host(`staging.micarrera.uy`)
traefik.http.routers.micarrera_secure-web-staging.tls: true
traefik.http.routers.micarrera_secure-web-staging.tls.certresolver: letsencrypt
options:
memory: 120MiB
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this going to be enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! Increased it to 250MiB 🙂

@santiagorodriguez96 santiagorodriguez96 merged commit 6b67837 into master May 4, 2024
1 check passed
@santiagorodriguez96 santiagorodriguez96 deleted the sr--kamal-docker-containers-memory-limit branch May 4, 2024 22:44
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

2 participants