Thanks for taking a look.
On Wed, Jan 11, 2023 at 04:53:39PM +0900, Michael Paquier wrote:
> Hmm. Is passing down the file name used as a cutoff point the best
> interface for the modules? Perhaps passing down the redo LSN and its
> TLI would be a cleaner approach in terms of flexibility? I agree with
> letting the startup enforce these numbers as that can be easy to mess
> up for plugin authors, leading to critical problems.
I'm having trouble thinking of any practical advantage of providing the
redo LSN and TLI. If the main use-case is removing older archives as the
documentation indicates, it seems better to provide the file name so that
you can plug it straight into strcmp() to determine whether the file can be
removed (i.e., what pg_archivecleanup does). If we provided the LSN and
TLI instead, you'd either need to convert that into a WAL file name for
strcmp(), or you'd need to convert the candidate file name into an LSN and
TLI and compare against those.
> "basic_archive" does not reflect what this module does. Using one
> library simplifies the whole configuration picture and the tests, so
> perhaps something like basic_wal_module, or something like that, would
> be better long-term?
I initially created a separate basic_restore module, but decided to fold it
into basic_archive to simplify the patch and tests. I hesitated to rename
it because it already exists in v15, and since it deals with creating and
restoring archive files, the name still seemed somewhat accurate. That
being said, I don't mind renaming it if that's what folks want.
I've attached a new patch set that is rebased over c96de2c. There are no
other changes.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
