On 11/10/21 1:22 PM, Bossart, Nathan wrote:
> On 11/10/21, 8:10 AM, "David Steele" <david@pgmasters.net> wrote:
>
>> I would prefer this module to be in core as our standard implementation
>> and load by default in a vanilla install.
>
> Hm. I think I disagree with putting it in contrib and with making it
> the default archive library. The first reason is backward
> compatibility. There has already been quite a bit of discussion about
> this, and I don't see how we can get away with anything except for
> maintaining the existing behavior for now. Maybe we could move to a
> better default down the road, but I'm hesitant to press that issue too
> much at the moment.
OK, I haven't had to go over the patch in detail so I didn't realize the
module was not backwards compatible. I'll have a closer look soon.
> The second reason is that the basic_archive module has a couple of
> deficiencies. For example, it doesn't handle inconvenient server
> crashes well (e.g., after archiving but before we've renamed the
> .ready file). A way to fix this might be to compare the archive file
> with the to-be-archived file and to succeed if they are exactly the
> same. Another problem is that there is no handling for multiple
> servers using basic_archive to write WAL to the same location. This
> is because basic_archive first copies data to a temporary file that is
> always named "archtemp." This might be fixed by appending a random
> string to the temporary file or by locking it somehow, but there are
> still a few things left to figure out.
Honestly, I'm not sure to what extent it makes sense to delve into these
problems for an archiver that basically just copies to another
directory. This is a not a very realistic solution for the common
storage requirements we are seeing these days.
> I think it'd be awesome to eventually fix all these issues in
> basic_archive and to recommend it as a proper archiving strategy, but
> I'm worried that this will introduce a significant amount of
> complexity to this patch. I really only intended for basic_archive to
> be used for testing and to demonstrate that it's possible use the
> archive module infrastructure to do something useful. If folks really
> want it in contrib, I'd at least add a big warning about the
> aforementioned problems in its docs.
I'll have more to say once I've had a closer look, but in general I
agree with what you have said here. Keeping it in test for now is likely
to be the best approach.
Regards,
--
-David
david@pgmasters.net