Skip to content

Replace postgres wait loop with Docker healthcheck#12339

Merged
RayBB merged 4 commits into
internetarchive:masterfrom
numericals-org:issue_#12322
Apr 9, 2026
Merged

Replace postgres wait loop with Docker healthcheck#12339
RayBB merged 4 commits into
internetarchive:masterfrom
numericals-org:issue_#12322

Conversation

@numericals-org

Copy link
Copy Markdown
Contributor

Closes #12322

Summary

Replaces the manual Postgres polling loop in docker/ol-home-start.sh with Docker's native healthcheck and depends_on mechanism.

Changes

  • Added healthcheck to the db service in compose.override.yaml using the pg_isready pattern
  • Updated depends_on in the home service in compose.override.yaml to use condition: service_healthy for the db service
  • Removed the until pg_isready --host db; do sleep 5; done polling loop from docker/ol-home-start.sh (lines 8–9)
  • Did not touch Solr-related changes to avoid conflicts with make home wait for solr in compose.override.yaml #12321

Testing

Run docker compose up and verify the home service starts only after Postgres is healthy, without the manual polling loop.

Screenshots

image image

Checklist

  • healthcheck added to db service using pg_isready in compose.override.yaml
  • home service depends_on updated with condition: service_healthy for db
  • Postgres wait loop removed from docker/ol-home-start.sh

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies local dev startup by replacing the ol-home-start.sh Postgres polling loop with Docker Compose-native gating: a db healthcheck plus home.depends_on conditioned on service_healthy.

Changes:

  • Add a Postgres healthcheck to db in compose.override.yaml using pg_isready.
  • Update home to wait for db to be healthy via depends_on: condition: service_healthy.
  • Remove the manual Postgres wait loop from docker/ol-home-start.sh.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
docker/ol-home-start.sh Removes the manual Postgres readiness polling loop from the home startup script.
compose.override.yaml Adds a db healthcheck and makes home depend on db health (similar to existing solr gating).

Comment thread compose.override.yaml Outdated
- ol-postgres:/var/lib/postgresql
healthcheck:
test: ["CMD-SHELL", "pg_isready -U postgres"]
interval: 5s

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

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

The db healthcheck has no start_period (or similar startup grace period). With interval: 5s and retries: 5, a first-run init (or slow disk) can mark the container unhealthy very quickly, which can be confusing and can delay depends_on: condition: service_healthy until the health status recovers. Consider adding a start_period (and/or increasing retries) to better match Postgres startup times during initialization.

Suggested change
interval: 5s
interval: 5s
start_period: 30s

Copilot uses AI. Check for mistakes.
Comment thread compose.override.yaml
- ./docker/ol-db-init.sh:/docker-entrypoint-initdb.d/ol-db-init.sh
- ol-postgres:/var/lib/postgresql
healthcheck:
test: ["CMD-SHELL", "pg_isready -U postgres"]

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

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

The healthcheck uses pg_isready -U postgres without -h. That checks the local Unix socket inside the db container, but other services connect over TCP via the db hostname. To make the healthcheck reflect the actual connection path, consider specifying -h localhost (and optionally -p 5432) and using the configured $${POSTGRES_USER} / $${POSTGRES_DB} rather than hardcoding postgres.

Suggested change
test: ["CMD-SHELL", "pg_isready -U postgres"]
test: ["CMD-SHELL", "pg_isready -h localhost -p 5432 -U $${POSTGRES_USER} -d $${POSTGRES_DB}"]

Copilot uses AI. Check for mistakes.
@mekarpeles

mekarpeles commented Apr 9, 2026

Copy link
Copy Markdown
Member

Thanks for the PR!

🤖 Copilot has been assigned for an initial review.

The linked issue (#12322) hasn't been triaged yet — priority assignment typically happens on Mondays and Fridays. The PR is currently assigned to @RayBB who has 12 PRs ahead of this one.

Possible improvements for this PR

  • Commit history contains unrelated commits — two commits appear to be from a different branch: "google signup screen issue fix with use of ResizeObserver" and its revert. These are unrelated to this Docker healthcheck change and add noise to the PR history. Consider cleaning up your branch by squashing or rebasing before review. See the git cheatsheet for tips.
PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

@github-actions github-actions Bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Apr 9, 2026

@RayBB RayBB left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Excellent work here. This is doing just what we expected.
I think we're gonna have a much more resilient dev environment thanks to this!

I did push up a few changes to the timing so that we check quicker. I encourage you to look into the commit if you'd like to learn more.

Cheers!

@RayBB RayBB merged commit c46d346 into internetarchive:master Apr 9, 2026
3 checks passed
@github-project-automation github-project-automation Bot moved this from Waiting Review/Merge from Staff to Done in Ray's Project Apr 9, 2026
IvanPisquiy06 pushed a commit to IvanPisquiy06/openlibrary that referenced this pull request Apr 27, 2026
…2339)

Co-authored-by: RayBB <RayBB@users.noreply.github.com>
Sadashii pushed a commit to Sadashii/openlibrary that referenced this pull request May 11, 2026
…2339)

Co-authored-by: RayBB <RayBB@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add docker health checks to compose.override.yaml for postgres and make home wait on db

4 participants