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

fix(deployment): Postgres healthcheck, add username to pg_isready #10221

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

mmomjian
Copy link
Contributor

I believe this is the cause of the FATAL: role "root" does not exist error that has been popping up. In my testing pg_isready is available to all users, but that might not be the case in all installations.

Might fix #10219

@mmomjian mmomjian added the deployment Deployment related tasks label Jun 12, 2024
@mmomjian mmomjian marked this pull request as ready for review June 12, 2024 19:04
@mmomjian mmomjian requested a review from bo0tzz as a code owner June 12, 2024 19:04
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.

From https://pgpedia.info/p/pg_isready.html I understand that the db and user flags aren't even really relevant, so we shouldn't need to set them. I'm fine with whatever though :P

@mmomjian
Copy link
Contributor Author

mmomjian commented Jun 12, 2024

From https://pgpedia.info/p/pg_isready.html I understand that the db and user flags aren't even really relevant, so we shouldn't need to set them. I'm fine with whatever though :P

This explains why it still worked in testing. That wiki page states: “ However the server will log the validity of the connection attempt.”

FATAL: role "nosuchuser" does not exist

@bo0tzz
Copy link
Member

bo0tzz commented Jun 12, 2024

Yeah, I think that error log on #10219 is just a red herring and we don't actually need this change.

@mmomjian
Copy link
Contributor Author

Yeah, I think that error log on #10219 is just a red herring and we don't actually need this change.

The pg_pedia wiki page you linked explains this error exactly, doesn’t it? It says if a non existing user (root in this case) connects, it will still get a status 0 ready result, but PG will log it as a FATAL error. That doesn’t seem good.

@bo0tzz
Copy link
Member

bo0tzz commented Jun 13, 2024

The healthcheck succeeds. Postgres logs a spooky error, but that isn't actually a hard problem, it just doesn't look very nice.

@mmomjian
Copy link
Contributor Author

Oh yes, I agree with you there. I thought you were saying we shouldn’t make this change, but I guess you meant it’s not the true underlying source of that other issue?

@bo0tzz
Copy link
Member

bo0tzz commented Jun 13, 2024

Both, but with a very weak should - I think this change is fine but probably unnecessary.

@mertalev mertalev merged commit eff8392 into immich-app:main Jun 20, 2024
22 checks passed
@mmomjian mmomjian deleted the pg-health branch June 20, 2024 18:20
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.

Healthcheck on db fails
3 participants