Re: thinko in basic_archive.c

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, robertmhaas@gmail.com, pgsql-hackers@postgresql.org
Date: 2022-11-05T21:46:51Z
Lists: pgsql-hackers
On Fri, Oct 21, 2022 at 09:30:16PM +0530, Bharath Rupireddy wrote:
> On Fri, Oct 21, 2022 at 10:43 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
>> Thanks, but we don't need to wipe out the all bytes. Just putting \0
>> at the beginning of the buffer is sufficient.
> 
> Nah, that's not a clean way IMO.

Why not?  This is a commonly-used technique.  I see over 80 existing useѕ
in PostgreSQL.  Plus, your shutdown callback only checks for the first
byte, anyway.

+	if (tempfilepath[0] == '\0')
+		return;

As noted upthread [0], I think we should be cautious to only remove the
temporary file if we know we created it.  I still feel that trying to add
this cleanup logic to basic_archive is probably more trouble than it's
worth, but if there is a safe and effective way to do so, I won't object.

[0] https://postgr.es/m/20221015211026.GA1821022%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com



Commits

  1. Fix calculation related to temporary WAL segment name in basic_archive