Message-ID: <7b9af864-bdf8-e65b-2ffa-41f4a17ed9ad@pgmasters.net>
Date: Wed, 18 Oct 2023 10:38:39 -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: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: thomas.munro@gmail.com, michael@paquier.xyz,
 pgsql-hackers@postgresql.org, sfrost@snowman.net
References: <65825be1-e79a-46f4-9d9f-4ff95a10e378@pgmasters.net>
 <e05f20f9-6f91-9a70-efab-9a2ae472e65d@pgmasters.net>
 <0f948866-7caf-0759-d53c-93c3e266ec3f@pgmasters.net>
 <20231018.111301.602565389803838288.horikyota.ntt@gmail.com>
From: David Steele <david@pgmasters.net>
In-Reply-To: <20231018.111301.602565389803838288.horikyota.ntt@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/7b9af864-bdf8-e65b-2ffa-41f4a17ed9ad%40pgmasters.net>
Precedence: bulk

On 10/17/23 22:13, Kyotaro Horiguchi wrote:
> At Tue, 17 Oct 2023 16:16:42 -0400, David Steele <david@pgmasters.net> wrote in
>> Given that the above can't be back patched, I'm thinking we don't need
>> backup_label at all going forward. We just write the values we need
>> for recovery into pg_control and return *that* from pg_backup_stop()
>> and tell the user to store it with their backup. We already have
>> "These files are vital to the backup working and must be written byte
>> for byte without modification, which may require opening the file in
>> binary mode." in the documentation so dealing with pg_control should
>> not be a problem. pg_control also has a CRC so we will know if it gets
>> munged.
> 
> I'm somewhat perplexed regarding the objective of this thread.
> 
> This thread began with the intent of preventing users from removing
> the backup_label from a backup. At the beginning, the proposal aimed
> to achieve this by injecting an invalid value to pg_control file
> located in the generated backup. However, this (and previous) proposal
> seems to deviate from that initial objective. It now eliminates the
> need to be concerned about the pg_control version that is coped into
> the generated backup. However, if someone removes the backup_label
> from a backup, the initial concerns could still surface.

Yeah, the discussion has moved around quite a bit, but the goal remains 
the same, to make Postgres error when it does not have the information 
it needs to proceed with recovery. Right now if you delete backup_label 
recovery appears to complete successfully, silently corrupting the database.

In the proposal as it stands now there would be no backup_label at all, 
so no danger of removing it.

We have also gotten a bit sidetracked by our hope to use this proposal 
to address torn reads of pg_control during the backup, at least in HEAD.

Regards,
-David