Date: Fri, 28 Jan 2022 15:39:35 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "Bossart, Nathan" <bossartn@amazon.com>,
	David Steele <david@pgmasters.net>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael@paquier.xyz>,
	Magnus Hagander <magnus@hagander.net>,
	Stephen Frost <sfrost@snowman.net>
Subject: Re: archive modules
Message-ID: <20220128233935.GA6472@nathanxps13>
References: <4c01ce36-fbed-104a-ef9c-6046343dc845@pgmasters.net>
 <316CAF77-A0AC-49AE-A175-1C1ECF6C1981@amazon.com>
 <71E7A5D1-E418-4516-A21D-BD838BA759CE@amazon.com>
 <0CF39706-6952-49B1-987D-F012CDF5EEC4@amazon.com>
 <90D3CEE4-00C1-450C-80B1-88B69D9318DE@amazon.com>
 <3842CF04-528B-42A5-A12D-BA3970EF1E53@amazon.com>
 <5D92CC59-47C8-4463-9150-44033DDDD0D8@amazon.com>
 <CA+Tgmobow79kHwCa3hJ6oALk7wL6JmCZW9gkDEJw9udvPSVO9A@mail.gmail.com>
 <20220128200138.GA3398@nathanxps13>
 <CA+TgmoaSkSmo22SwJaV+ycNPoGpxe0JV=TcTbh4ip8Cwjr0ULQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoaSkSmo22SwJaV+ycNPoGpxe0JV=TcTbh4ip8Cwjr0ULQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20220128233935.GA6472%40nathanxps13>
Precedence: bulk

On Fri, Jan 28, 2022 at 03:20:41PM -0500, Robert Haas wrote:
> On Fri, Jan 28, 2022 at 3:01 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I discussed the two main deficiencies I'm aware of with basic_archive
>> earlier [0].  The first one is the issue with "incovenient" server crashes
>> (mentioned below).
> 
> Seems easy enough to rectify, if it's just a matter of silently-succeed-if-same.

Yes.

>> The second is that there is no handling for multiple
>> servers writing to the same location since the temporary file is always
>> named "archtemp."  I thought about a few ways to pick a unique file name
>> (or at least one that is _probably_ unique), but that began adding a lot of
>> complexity for something I intended as a test module.  I can spend some
>> more time on this if you think it's worth fixing for a contrib module.
> 
> Well, my first thought was to wonder whether we even care about that
> scenario, but I guess we probably do, at least a little bit.
> 
> How about:
> 
> 1. Name temporary files like
> archive_temp.${FINAL_NAME}.${PID}.${SOME_RANDOM_NUMBER}. Create them
> with O_EXCL. If it fails, die.
> 
> 2. Try not to leave files like this behind, perhaps installing an
> on_proc_exit callback or similar, but accept that crashes and unlink()
> failures will make it inevitable in some cases.
> 
> 3. Document that crashes or other strange failure cases may leave
> archive_temp.* files behind in the archive directory, and recommend
> that users remove them before restarting the database after a crash
> (or, with caution, removing them while the database is running if the
> user is sure that the files are old and unrelated to any archiving
> still in progress).
> 
> I'm not arguing that this is exactly the right idea. But I am arguing
> that it shouldn't take a ton of engineering to come up with something
> reasonable here.

This is roughly what I had in mind.  I'll give it a try.

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