[Patch] Don't stop a `vacuumdb --all` on a connection failure

Started by Christophe Pettus27 days ago8 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253217
psql -h localhost -U postgres

Built from patchset v8 (message #8), August 23, 2026 at 08:21 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253217_8 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253217_8 && git checkout t253217_8

Patchset v8 (message #8) is on t253217_8

Jump to latest
#1Christophe Pettus
xof@thebuild.com

Currently, `vacuumdb --all` returns FATAL and stops if it can't connect to one of the discovered databases. On some hosted providers (in particular, CloudSQL), there are reserved databases that the credentials supplied to the user can't connect to. This makes `vacuumdb --all` less than useful in those environments.

Attached is a patch that de-escalates the error so that vacuumdb continues if it can't connect to a database. It does mean that if the connection info is fundamentally wrong, or the instance is unreachable, the command doesn't error out immediately. I don't think there's anything particularly valuable about the current behavior, so it's a general change rather than yet another switch.

Attachments:

t253217_1
0001-vacuumdb-skip-unconnectable-databases.patchapplication/octet-stream; name=0001-vacuumdb-skip-unconnectable-databases.patch; x-unix-mode=0644Download+52-7
#2Neil Chen
carpenter.nail.cz@gmail.com
In reply to: Christophe Pettus (#1)
Re: [Patch] Don't stop a `vacuumdb --all` on a connection failure

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.

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Neil Chen (#2)
Re: [Patch] Don't stop a `vacuumdb --all` on a connection failure

On 29 Jul 2026, at 05:45, Neil Chen <carpenter.nail.cz@gmail.com> wrote:

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?

It seems reasonable to make them all work the same. (I didn't read the patch
so might be missing some technical reasoning.)

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.

While I agree that avoiding a new switch would be good, for this reason alone I
think we should consider adding a --continue to trigger the skipping behaviour
and keep --all with the current exit code handling. If we allow skipping then
we really can't return anything other 0 even if we skip all databases.

--
Daniel Gustafsson

#4Christophe Pettus
xof@thebuild.com
In reply to: Daniel Gustafsson (#3)
Re: [PATCH v2] Don't stop a `vacuumdb --all` on a connection failure

Thank you, Neil and Daniel!

There was no particular reason besides lack of ambition that reindexdb and clusterdb were not included. I've added them.

Agreed with Daniel that a switch to control the new behavior makes sense (I hadn't through about the status code issue).

Revised patch attached (I've kept the patch name and Subject: consistent for indexing purposes). It applies cleanly to 9fa2c1ebd17.

Attachments:

v2-0001-vacuumdb-skip-unconnectable-databases,patchapplication/octet-stream; name="v2-0001-vacuumdb-skip-unconnectable-databases,patch"; x-unix-mode=0644Download+303-51
#5Daniel Gustafsson
daniel@yesql.se
In reply to: Christophe Pettus (#4)
Re: [PATCH v2] Don't stop a `vacuumdb --all` on a connection failure

On 29 Jul 2026, at 18:25, Christophe Pettus <xof@thebuild.com> wrote:

There was no particular reason besides lack of ambition that reindexdb and clusterdb were not included. I've added them.

Agreed with Daniel that a switch to control the new behavior makes sense (I hadn't through about the status code issue).

Revised patch attached (I've kept the patch name and Subject: consistent for indexing purposes). It applies cleanly to 9fa2c1ebd17.

I've only skimmed the patch thus far, but in general this seems like a good
idea to me.

--
Daniel Gustafsson

#6Neil Chen
carpenter.nail.cz@gmail.com
In reply to: Daniel Gustafsson (#5)
Re: [PATCH v2] Don't stop a `vacuumdb --all` on a connection failure

Hi Christophe,

Thanks for the v2. All three points from my earlier review are
addressed, and the --continue approach looks good to me.

+1.

Best regards,
--
Ze Chen (Neil)
HighGo Software Co., Ltd.
https://www.highgo.com/

#7Christophe Pettus
xof@thebuild.com
In reply to: Neil Chen (#6)
Re: [PATCH v2] Don't stop a `vacuumdb --all` on a connection failure

On Jul 29, 2026, at 17:45, Neil Chen <carpenter.nail.cz@gmail.com> wrote:
Thanks for the v2. All three points from my earlier review are
addressed, and the --continue approach looks good to me.

Thank you! (And thank you, Daniel.) I've added it to the open CF.

#8Christophe Pettus
xof@thebuild.com
In reply to: Christophe Pettus (#7)
Re: [PATCH v3] Don't stop a `vacuumdb/reindex/ --all` on a connection failure

Another turn on this patch to clear cfbot errors.

Attachments:

t253217_8
0001-vacuumdb-clusterdb-reindexdb-add-continue.patchapplication/octet-stream; name=0001-vacuumdb-clusterdb-reindexdb-add-continue.patch; x-unix-mode=0644Download+306-55