Re: recovery modules

On Thu, Jan 12, 2023 at 03:30:40PM +0900, Michael Paquier wrote:
> On Wed, Jan 11, 2023 at 11:29:01AM -0800, Nathan Bossart wrote:
>> 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 done that in the past for pg_verify_checksums -> pg_checksums, so
> I would not mind renaming it so as it reflects better its role.
> (Being outvoted is fine for me if this suggestion sounds bad).

IMHO I don't think there's an urgent need to rename it, but if there's a
better name that people like, I'm happy to do so.

> Saying that, 0001 seems fine on its own (minus the redo LSN/TLI with
> the duplication for the segment name build), so I would be tempted to
> get this one done.  My gut tells me that we'd better remove the
> duplication and just pass down the two fields to
> shell_archive_cleanup() and shell_recovery_end(), with the segment
> name given to ExecuteRecoveryCommand()..

I moved the duplicated logic to its own function in v6.

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