Conversation
Every failed setup step called `'stop_postgres' 1`, which neither exits nor stops a daemonized server: stop_postgres ignores its argument and, in --daemonize mode, only prints a message. A failing init script or migration therefore let the script carry on with the remaining scripts, restart the server and exit 0 with a partially initialized database. Add abort_setup, which stops the setup server with pg_ctl and exits 1, and use it for every failure path.
Contributor
|
Closing as this is 100% AI generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Bug fix (developer tooling:
nix run .#start-server,nix/tools/run-server.sh.in).What is the current behavior?
Every setup failure path in
run-server.sh.incalls'stop_postgres' 1. There are 9 of these: the startup wait, role creation,--migration-file, init scripts, the supabase_admin password, the pgbouncer and stat schema files, migrations, and the final schema.stop_postgresignores its argument and never exits. In--daemonizemode it also doesn't stop anything; it only prints "PostgreSQL is running in daemon mode...". So when an init script or migration fails, the script:The error is only visible in scrollback. Anyone using start-server to test a new migration can miss that it failed.
What is the new behavior?
A new
abort_setuphelper stops the setup server withpg_ctl stop -D "$DATDIR" -m fast(in either mode, since setup always runs a daemonized server) and exits 1. All 9'stop_postgres' 1calls now use it. The Ctrl-C trap and the normal shutdown/restart flow are unchanged.Additional context
How I tested it: I rendered the
.sh.inwith its@...@placeholders pointed at a stockpostgres:17container, using a minimal config and a migrations dir. The init scripts were00-ok.sql,01-broken.sql(select 1/0) and02-after.sql, plus one migration. I ran it asstart-postgres-server 17 --daemonize --datdir /tmp/dat.PGOPTIONSclearedsession_preload_libraries, because the stock image has no supautils.after_failure, migrated, ok1(kept going after the error)Error: database setup failed, stopping PostgreSQLafter_failure, migrated, ok1(same as develop)bash -npasses. The.sh.inisn't covered by shfmt, and I kept its existing 4-space style.This PR was prepared by an AI agent (Claude) working for breken-ai. I reproduced the bug and checked the fix as described above, and I'm happy to change anything.