Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 11 Jan 2023 16:53:39 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@postgresql.org
Subject: Re: recovery modules
Message-ID: <Y75rA+YNIWnJ1zsH@paquier.xyz>
References: <20221227192449.GA3672473@nathanxps13>
 <20221227221111.xjw5hc3fqzt773gu@awork3.anarazel.de>
 <Y6uNJVNP29B4egDx@paquier.xyz>
 <20221229194326.GA296099@nathanxps13>
 <20230103175917.GA204418@nathanxps13>
 <20230103190538.GA287415@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="FkttpnByu3iWk2Q8"
Content-Disposition: inline
In-Reply-To: <20230103190538.GA287415@nathanxps13>
Archived-At: <https://www.postgresql.org/message-id/Y75rA%2BYNIWnJ1zsH%40paquier.xyz>
Precedence: bulk


--FkttpnByu3iWk2Q8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Jan 03, 2023 at 11:05:38AM -0800, Nathan Bossart wrote:
> I noticed that cfbot's Windows tests are failing because the backslashes in
> the archive directory path are causing escaping problems.  Here is an
> attempt to fix that by converting all backslashes to forward slashes, which
> is what other tests (e.g., 025_stuck_on_old_timeline.pl) do.

+       GetOldestRestartPoint(&restartRedoPtr, &restartTli);
+       XLByteToSeg(restartRedoPtr, restartSegNo, wal_segment_size);
+       XLogFileName(lastRestartPointFname, restartTli, restartSegNo,
+                    wal_segment_size);
+
+       shell_archive_cleanup(lastRestartPointFname);

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.  The same code
pattern is repeated twice for the end-of-recovery callback and the
cleanup commands when it comes to building the file name.  Not
critical, still not really nice.

 MODULES = basic_archive
-PGFILEDESC = "basic_archive - basic archive module"
+PGFILEDESC = "basic_archive - basic archive and recovery module"
"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?
--
Michael

--FkttpnByu3iWk2Q8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmO+awMACgkQnvQgOdby
QH3BEA//UGDEPGaZfnKFg6+Ai+Y+Uo1xI9BPBP1yY+XdnV5NPZkkpJoAcBMK8/rG
qzLrbhxSo2JFWVOGbNspVIYFDEbVTo/mMHq/QQ0b0g5IGEPnaGzlS3SQjM1hFijF
SvT4QgqLJCB3YHKOM0sueJq12amA7ZN46dSIWQ0vLUzQlnENsxQ+NebSr+TPIuKG
S4InVMVJh6hOktXx8712Sk1fEhEyuOh911zA/6vNhHL2qRjhrlZ0TfXxNQZ+rkbY
idH7hXcTPKXYA2SYBUmzbF10QI6REgFxQ0e9aymN+h9lmGKF0TLKF4rG5VAAYSPZ
QWeYxOtnGfFAZwk5nrf1z2ICOrOL9Cfpi7FPAVz1SQfYh/zdz5SyvIQWsqZ/xQoP
agxDUVrSVj8NM5/peq4giE0rl73EZ+nW9zdicpjsr4ptJiBP219dNJRN1zl9Prai
1TaJj3PwwY3aXVKcfc+01Hujn/DFNVvxj/pRCR7IA+vs0VsUzcGmSmAU36vJNAdm
vgqKDnuBUhUgm9LXsOLwyMeeZAGqsDDYC+BTv5h+3WQQfhkSKlIwgYDVxuDUddzI
8H2I2ULP6cEbUSZ/FXUMOrGajSJ/0EOPgVOn4ynLZhcEPOBQR8HOUsKD9dCoCZqE
AJgiuMlJcoWiHCe9ZFaBKHfkFBNFbQbULIgAMW/jr0ohagUZNP8=
=U+y9
-----END PGP SIGNATURE-----

--FkttpnByu3iWk2Q8--