Re: Some code cleanup for pgbench and pg_verifybackup
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-28T03:18:12Z
Lists: pgsql-hackers
Attachments
- log-fatal-exit.patch (text/x-diff) patch
On Tue, Jul 27, 2021 at 08:53:52AM +0900, Michael Paquier wrote: > On Mon, Jul 26, 2021 at 03:35:29PM -0400, Alvaro Herrera wrote: >> (In reality we cannot literally just exit(1) in pg_log_fatal(), because >> there are quite a few places that do some other thing after the log >> call and before exit(1), or terminate the program in some other way than >> exit().) > > Yes, that's obviously wrong. I am wondering if there is more of > that. I have been looking at that. Here are some findings: - In pg_archivecleanup, CleanupPriorWALFiles() does not bother exit()'ing with an error code. Shouldn't we worry about reporting that correctly? It seems strange to not inform users about paths that would be broken, as that could bloat the archives without one knowing about it. - In pg_basebackup.c, ReceiveTarAndUnpackCopyChunk() would not exit() when failing to change permissions for non-WIN32. - pg_recvlogical is missing a failure handling for fstat(), as of 5c0de38. - pg_verifybackup is in the wrong, as mentioned upthread. Thoughts? All that does not seem to enter into the category of things worth back-patching, except for pg_verifybackup. -- Michael
Commits
-
Add some missing exit() calls in error paths for various binaries
- 856de3b39cf6 15.0 landed
-
Add missing exit() in pg_verifybackup when failing to find pg_waldump
- efe169c90090 13.4 landed
- 67445deb7eca 14.0 landed
- 2ad98fdf53ed 15.0 landed