Re: Some code cleanup for pgbench and pg_verifybackup - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Some code cleanup for pgbench and pg_verifybackup
Date
Msg-id YQDMdB+B68yePFeT@paquier.xyz
Whole thread Raw
In response to Re: Some code cleanup for pgbench and pg_verifybackup  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Some code cleanup for pgbench and pg_verifybackup  (Fabien COELHO <coelho@cri.ensmp.fr>)
Re: Some code cleanup for pgbench and pg_verifybackup  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: ATTACH PARTITION locking documentation for DEFAULT partitions
Next
From: Greg Nancarrow
Date:
Subject: Re: [bug?] Missed parallel safety checks, and wrong parallel safety