Re: recovery modules

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