Message-ID: <dabdce7d-89ec-8bd4-44d7-da57bbd000fd@pgmasters.net>
Date: Thu, 12 Oct 2023 10:25:45 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.15.1
Subject: Re: The danger of deleting backup_label
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
References: <1330cb48-4e47-03ca-f2fb-b144b49514d8@pgmasters.net>
 <ZRY2vdlKJqhuZqGs@paquier.xyz>
 <0e088834-aa39-ed09-3ec7-31c13c629a20@pgmasters.net>
 <ZScgEc826eICWwd5@paquier.xyz>
From: David Steele <david@pgmasters.net>
In-Reply-To: <ZScgEc826eICWwd5@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/dabdce7d-89ec-8bd4-44d7-da57bbd000fd%40pgmasters.net>
Precedence: bulk


On 10/11/23 18:22, Michael Paquier wrote:
> On Tue, Oct 10, 2023 at 05:06:45PM -0400, David Steele wrote:
>> That fails because there is a check to make sure the checkpoint is valid
>> when pg_control is loaded. Another possibility is to use a special LSN like
>> we use for unlogged tables. Anything >= 24 and < WAL segment size will work
>> fine.
> 
> Do we have any reason to do that in the presence of a backup_label
> file anyway?  We'll know the LSN of the checkpoint based on what the
> base backup wants us to use.  Using a fake-still-rather-valid value
> for the LSN in the control file to bypass this check does not address
> the issue you are pointing at: it is just avoiding this check.  A
> reasonable answer would be, IMO, to just not do this check at all
> based on the control file in this case.

Yeah, that's fair. And it looks like we are leaning towards excluding 
pg_control from the backup entirely, so the point is probably moot.

>>> If the contents of the control file are tweaked before sending it
>>> through a BASE_BACKUP, it would cover more than just pg_basebackup.
>>> Switching the way the control file is sent with new contents in
>>> sendFileWithContent() rather than sendFile() would be one way, for
>>> instance..
>>
>> Good point, and that makes this even more compelling. If we include
>> pg_control into backup_label then there is no need to modify pg_control (as
>> above) -- we can just exclude it from the backup entirely. That will
>> certainly require some rejigging in recovery but seems worth it for backup
>> solutions that can't easily modify pg_control. The C-based solutions can do
>> this pretty easily but it is a pretty high bar for anyone else.
> 
> I have little idea about that, but I guess that you are referring to
> backrest here.

Sure, pgBackRest, but there are other backup solutions written in C. My 
point is really that we should not depend on backup solutions being able 
to manipulate C structs. It looks the the solution we are working 
towards would not require that.

Regards,
-David