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

feat(server): healthchecks for PG and redis #9590

Merged
merged 11 commits into from
May 22, 2024

Conversation

mmomjian
Copy link
Contributor

@mmomjian mmomjian commented May 19, 2024

implements #9583 in non-Immich services

this is a non breaking change. Users who add this will get the benefit of the healthcheck, if they have checksums enabled (new installs starting with 1.104.0)

Users with older setups will have a unhealthy status. This can be fixed by removing everything after the first exit 1 in the database health check test: line

@mmomjian mmomjian added the documentation Improvements or additions to documentation label May 19, 2024
@zackpollard
Copy link
Contributor

In #9583 I am requesting that we make the healthchecks active by default using the HEALTHCHECK command in the dockerfiles, so we shouldn't need the documentation around the docker compose changes. If that change makes it in the docs will need to be updated to reflect that

docs/docs/FAQ.mdx Outdated Show resolved Hide resolved
command: ["postgres", "-c" ,"shared_preload_libraries=vectors.so", "-c", 'search_path="$$user", public, vectors', "-c", "logging_collector=on", "-c", "max_wal_size=2GB", "-c", "shared_buffers=512MB", "-c", "wal_compression=on"]
restart: always
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any reason the database was the only core service not restart: always in prod?

Same question for grafana and prometheus, though I did not make a change there.

@mmomjian mmomjian marked this pull request as ready for review May 21, 2024 17:09
@mmomjian mmomjian requested a review from bo0tzz as a code owner May 21, 2024 17:09
@mmomjian mmomjian changed the title docs: healthchecks feat: healthchecks May 21, 2024
@mmomjian mmomjian added deployment Deployment related tasks and removed documentation Improvements or additions to documentation labels May 21, 2024
@mmomjian mmomjian changed the title feat: healthchecks feat: healthchecks for PG and redis May 21, 2024
@mmomjian mmomjian changed the title feat: healthchecks for PG and redis feat(server): healthchecks for PG and redis May 21, 2024
Copy link
Member

@bo0tzz bo0tzz left a comment

Choose a reason for hiding this comment

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

LGTM. I do worry a bit about the default compose file growing large again, but I don't think there's any other way for us to apply these healthchecks right?

@zackpollard
Copy link
Contributor

LGTM. I do worry a bit about the default compose file growing large again, but I don't think there's any other way for us to apply these healthchecks right?

Yea, this is the only way without pushing our own containers

@zackpollard zackpollard enabled auto-merge (squash) May 22, 2024 09:22
@zackpollard zackpollard disabled auto-merge May 22, 2024 09:22
@zackpollard zackpollard enabled auto-merge (squash) May 22, 2024 09:23
@zackpollard zackpollard merged commit f8ee977 into immich-app:main May 22, 2024
23 checks passed
@mmomjian mmomjian deleted the hc-doc branch May 22, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment Deployment related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants