| From: | Neil Chen <carpenter(dot)nail(dot)cz(at)gmail(dot)com> |
|---|---|
| To: | Christophe Pettus <xof(at)thebuild(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [Patch] Don't stop a `vacuumdb --all` on a connection failure |
| Date: | 2026-07-29 03:45:05 |
| Message-ID: | CAA3qoJkYNee3dP=jGx7GsjytiKi+V5u3HGnSdiaxALjOGxpL+g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Christophe,
Interesting idea. I took a look and tested it, here are my thoughts:
1. reindexdb and clusterdb appear to have the same problem — they also
abort on an unconnectable database. Should we apply the same fix to those
tools as well? Or is there a reason they should behave differently from
vacuumdb?
2. vacuumdb --all now exits 0 even when some databases were skipped.
Scripts that rely on exit codes to detect failures would silently miss
skipped databases.
Might be worth considering whether the exit code should reflect that not
all databases were processed.
3. I ran the test on current master(dd50eb9), and it fails. The test
appends to 100_vacuumdb.pl, but earlier tests in that file create
superuser-owned schemas "Foo" and "Bar". Since databases are processed
alphabetically, postgres comes before regress_vacuumdb_noconn. The
unprivileged user hits ERROR: permission denied for schema Foo during
ANALYZE, which aborts the run before reaching the unconnectable database.
The test output:
not ok 116 - --all skips databases that cannot be connected to status (got
1 vs expected 0)
not ok 117 - --all skips databases that cannot be connected to stdout
/(?^:^$)/
not ok 118 - --all skips databases that cannot be connected to stderr
/(?^:warning: skipping database "regress_vacuumdb_noconn": )/
1..118
# test failed
----------------------------------- stderr
-----------------------------------
# Failed test '--all skips databases that cannot be connected to status
(got 1 vs expected 0)'
# at /home/neil/postgres/src/bin/scripts/t/100_vacuumdb.pl line 383.
# Failed test '--all skips databases that cannot be connected to stdout
/(?^:^$)/'
# at /home/neil/postgres/src/bin/scripts/t/100_vacuumdb.pl line 383.
# 'vacuumdb: vacuuming database "postgres"
# '
# doesn't match '(?^:^$)'
# Failed test '--all skips databases that cannot be connected to stderr
/(?^:warning: skipping database "regress_vacuumdb_noconn": )/'
# at /home/neil/postgres/src/bin/scripts/t/100_vacuumdb.pl line 383.
# vacuumdb: error: processing of database "postgres" failed: ERROR:
permission denied for schema Foo
# '
# doesn't match '(?^:warning: skipping database
"regress_vacuumdb_noconn": )'
# Looks like you failed 3 tests of 118.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-07-29 03:58:37 | Re: [PATCH] Preserve replication origin OIDs in pg_upgrade |
| Previous Message | shveta malik | 2026-07-29 03:44:09 | Re: Missing list_free in publicationcmds.c:OpenTableList |