Re: Add system identifier to backup manifest

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amul Sul <sulamul@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-03-07T04:07:28Z
Lists: pgsql-hackers
On Wed, Mar 06, 2024 at 11:05:36AM -0500, Robert Haas wrote:
> So with that in mind, here's my proposal. This is an adjustment of
> Amit's previous refactoring patch. He renamed the existing
> get_controlfile() to get_dir_controlfile() and made a new
> get_controlfile() that accepted the path to the control file itself. I
> chose to instead leave the existing get_controlfile() alone and add a
> new get_controlfile_by_exact_path(). I think this is better, because
> most of the existing callers find it more convenient to pass the path
> to the data directory rather than the path to the controlfile, so the
> patch is smaller this way, and less prone to cause headaches for
> people back-patching or maintaining out-of-core code. But it still
> gives us a way to avoid repeatedly constructing the same pathname.

Yes, that was my primary concern with the previous versions of the
patch.

> If nobody objects, I plan to commit this version.

You are not changing silently the internals of get_controlfile(), so
no objections here.  The name of the new routine could be shorter, but
being short of ideas what you are proposing looks fine by me.
--
Michael

Commits

  1. Add the system identifier to backup manifests.

  2. pg_verifybackup: Refactor parse_manifest_file.

  3. Add option force_initdb to PostgreSQL::Test::Cluster:init()