public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected]
Subject: Re: recovery modules
Date: Thu, 12 Jan 2023 15:30:40 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <20230111192901.GA1955161@nathanxps13>
References: <20221227192449.GA3672473@nathanxps13>
<[email protected]>
<[email protected]>
<20221229194326.GA296099@nathanxps13>
<20230103175917.GA204418@nathanxps13>
<20230103190538.GA287415@nathanxps13>
<[email protected]>
<20230111192901.GA1955161@nathanxps13>
On Wed, Jan 11, 2023 at 11:29:01AM -0800, Nathan Bossart wrote:
> 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.
Logging was one thing that came immediately in mind, to let the module
know the redo LSN and TLI the segment name was built from without
having to recalculate it back. If you don't feel strongly about that,
I am fine to discard this remark. It is not like this hook should be
set in stone across major releases, in any case.
> 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).
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()..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (5+ messages)
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: recovery modules
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox