From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: thinko in basic_archive.c
Date: Sat, 15 Oct 2022 14:10:26 -0700
Message-ID: <20221015211026.GA1821022@nathanxps13> (raw)
In-Reply-To: <CALj2ACVMf0kv=MnYq3ctB-EfOuhpO+X_cN2CM2LYimUDigq79g@mail.gmail.com>
References: <20221014044106.GA1673343@nathanxps13>
<CALj2ACWhqfi393dZ5Sdo=0j38yPFmwdSkCe_7SBi93x3s5vfJQ@mail.gmail.com>
<20221014183310.GB1678424@nathanxps13>
<CALj2ACVMf0kv=MnYq3ctB-EfOuhpO+X_cN2CM2LYimUDigq79g@mail.gmail.com>
On Sat, Oct 15, 2022 at 10:19:05AM +0530, Bharath Rupireddy wrote:
> Can you please help me understand how name collisions can happen with
> temp file names including WAL file name, timestamp to millisecond
> scale, and PID? Having the timestamp is enough to provide a non-unique
> temp file name when PID wraparound occurs, right? Am I missing
> something here?
Outside of contrived cases involving multiple servers, inaccurate clocks,
PID reuse, etc., it seems unlikely.
> Hm, we cannot remove the temp file for all sorts of crashes, but
> having on_shmem_exit() or before_shmem_exit() or atexit() or any such
> callback removing it would help us cover some crash scenarios (that
> exit with proc_exit() or exit()) at least. I think the basic_archive
> module currently leaves temp files around even when the server is
> restarted legitimately while copying to or renaming the temp file, no?
I think the right way to do this would be to add handling for leftover
files in the sigsetjmp() block and a shutdown callback (which just sets up
a before_shmem_exit callback). While this should ensure those files are
cleaned up after an ERROR or FATAL, crashes and unlink() failures could
still leave files behind. We'd probably also need to avoid cleaning up the
temp file if copy_file() fails because it already exists, as we won't know
if it's actually ours. Overall, I suspect it'd be more trouble than it's
worth.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, bharath.rupireddyforpostgres@gmail.com
Subject: Re: thinko in basic_archive.c
In-Reply-To: <20221015211026.GA1821022@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox