Re: Add system identifier to backup manifest
Amul Sul <sulamul@gmail.com>
From: Amul Sul <sulamul@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-03-07T04:21:52Z
Lists: pgsql-hackers
On Thu, Mar 7, 2024 at 9:37 AM Michael Paquier <michael@paquier.xyz> wrote: > 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. > Could be get_controlfile_by_path() ? Regards, Amul
Commits
-
Add the system identifier to backup manifests.
- 2041bc4276c9 17.0 landed
-
pg_verifybackup: Refactor parse_manifest_file.
- dc8f2d7c064a 17.0 landed
-
Add option force_initdb to PostgreSQL::Test::Cluster:init()
- ff9e1e764fcc 17.0 landed