Message-ID: <4193dcbc-591e-44bb-816c-43b4ae70d31c@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Return pg_control from pg_backup_stop().
From: David Steele <david@pgbackrest.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Haibo Yan <tristan.yim@gmail.com>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 Heikki Linnakangas <hlinnaka@iki.fi>, Robert Haas <robertmhaas@gmail.com>,
 Andres Freund <andres@anarazel.de>, Fujii Masao <masao.fujii@gmail.com>
References: <8b8aa673-fcef-4e14-a05d-0885283ef1b8@pgbackrest.org>
 <d21f3c7c-563c-49db-8983-faa73ed4369f@pgbackrest.org>
 <f74cab02-5ebb-439c-975b-29d019eed787@pgbackrest.org>
 <17DC1346-0CDE-4E39-B110-3D6FB0797AC6@gmail.com>
 <feb68313-3257-4d9d-8b69-6468e506a61a@pgbackrest.org>
 <7F7B289B-F94F-42C2-9E54-6A689C0D64BB@gmail.com>
 <aboDQSoywyKGGNCr@paquier.xyz>
 <1800c83c-264a-4183-9da5-ac78e25849a8@pgbackrest.org>
 <abov3-Kre8ntiL-S@paquier.xyz>
 <3b23e3b7-53d2-4784-b482-05cca3327acb@pgbackrest.org>
 <abphw4g2TwuPP0Cv@paquier.xyz>
 <df9e4c1f-4178-47fb-abdd-9829f7079b44@pgbackrest.org>
Content-Language: en-US
In-Reply-To: <df9e4c1f-4178-47fb-abdd-9829f7079b44@pgbackrest.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Date: Mon, 13 Apr 2026 14:55:10 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/4193dcbc-591e-44bb-816c-43b4ae70d31c%40pgbackrest.org>
Precedence: bulk

On 3/18/26 19:26, David Steele wrote:
> On 3/18/26 15:26, Michael Paquier wrote:
>> On Wed, Mar 18, 2026 at 07:35:47AM +0000, David Steele wrote:
>>> You are correct -- the copy of pg_control needs to happen before
>>> do_pg_backup_stop(). An older version of this patch saved pg_control in
>>> backup_state which made the prior location safe. However, I missed 
>>> moving
>>> this code when I moved pg_control out of backup_state. Code review to 
>>> the
>>> rescue.
>>
>> Right.  I am wondering also if the final result would not be better
>> without 0002, actually, focusing only on the "simpler" base backup
>> case through the replication protocol, and you are making a good case
>> in mentioning it as not absolutely mandatory for base backups that are
>> taken through the SQL functions.  One could always tweak the flag
>> manually in the control file based on the contents taken from the data
>> folder.  That's more hairy than writing the entire file, for sure,
>> still possible.
> 
> Getting even 01 into PG19 would be a great outcome. This would solve the 
> problem of torn pg_control and deleted backup labels for any backups 
> made with pg_basebackup and that's going to cover a *lot* of cases.
> 
> Established third-party backup solutions that are not based on 
> pg_basebackup are generally able to manipulate pg_control so that's not 
> as much of a concern, perhaps. It does raise the barrier of entry for 
> new backup software if they need to learn to read and validate 
> pg_control to avoid a torn copy and set the flag. Patch 02 solves that 
> problem in a general way so I still think it adds value for the 
> ecosystem -- but we could always discuss that in the PG20 cycle.
> 
> Whatever gets committed for PG19 I'll write a followup patch to describe 
> the hazards of reading pg_control and generally how to get a good copy. 
> However, this will be complicated enough that the best answer will 
> likely be to use pg_basebackup or some other reputable backup software. 
> I don't love this -- I feel like the low-level interface should be 
> usable with such hazards.

I have withdrawn this patch. If anybody wants to pick it up in the 
future I'll be happy to rebase it but I think two years is long enough 
to maintain a patch that is not getting traction.

We are left with the issue that pg_basebackup backups may contain a torn 
copy of pg_control. At the least this should be documented.

It would also be a good idea to document that utilizing the low-level 
backup interface requires validating the checksum in pg_control to avoid 
a torn copy. This is non-trivial but certainly doable.

Regards,
-David