Re: The danger of deleting backup_label

From: David Steele <david(at)pgmasters(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>
Subject: Re: The danger of deleting backup_label
Date: 2023-10-16 17:00:11
Message-ID: 12bf9438-f229-5048-dd91-440f3b1880e9@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 10/16/23 12:25, Robert Haas wrote:
> On Mon, Oct 16, 2023 at 11:45 AM David Steele <david(at)pgmasters(dot)net> wrote:
>> Hmmm, the reason to back patch this is that it would fix [1], which sure
>> looks like a problem to me even if it is not a "bug". We can certainly
>> require backup software to retry pg_control until the checksum is valid
>> but that seems like a pretty big ask, even considering how complicated
>> backup is.
>
> That seems like a problem with pg_control not being written atomically
> when the standby server is updating it during recovery, rather than a
> problem with backup_label not being used at the start of recovery.
> Unless I'm confused.

You are not confused. But the fact that it practically can be read as
torn at all on the standby is a function of how rapidly it is being
written to update min recovery point. Writing it atomically via a temp
file may affect performance in this area, but only during the backup,
which may cause recovery to run more slowly during a backup.

I don't have proof of this because I don't have any hosts large enough
to test the theory.

>> Right now the user can remove backup_label and get a "successful"
>> restore and not realize that they have just corrupted their cluster.
>> This is independent of the backup/restore tool doing all the right things.
>
> I don't think it's independent of that at all.

I think it is. Imagine the user does backup/recovery using their
favorite tool and everything is fine. But due to some misconfiguration
or a problem in the WAL archive, they get this error:

FATAL: could not locate required checkpoint record
2023-10-16 16:42:50.132 UTC HINT: If you are restoring from a backup,
touch "/home/dev/test/data/recovery.signal" or
"/home/dev/test/data/standby.signal" and add required recovery options.
If you are not restoring from a backup, try removing the file
"/home/dev/test/data/backup_label".
Be careful: removing "/home/dev/test/data/backup_label" will
result in a corrupt cluster if restoring from a backup.

I did this by setting restore_command=false, but it could just as easily
be the actual restore command that returns false due to a variety of
reasons. The user has no idea what "could not locate required checkpoint
record" means and if there is enough automation they may not even
realize they just restored from a backup.

After some agonizing (we hope) they decide to delete backup_label and,
wow, it just works! So now they merrily go on their way with a corrupted
cluster. They also remember for the next time that deleting backup_label
is definitely a good procedure.

The idea behind this patch is that deleting backup_label would produce a
hard error because pg_control would be missing as well (if the backup
software did its job). If both pg_control and backup_label are present
(but pg_control has not been loaded with the contents of backup_label,
i.e. it is the running copy from the backup cluster) we can also error.

It's not perfect, because they could backup (or restore) pg_control but
not backup_label, but we are narrowing the cases where something can go
wrong and they have silent corruption, especially if their
backup/restore software follows the directions.

Regards,
-David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2023-10-16 17:16:19 Re: Rename backup_label to recovery_control
Previous Message Michael Christofides 2023-10-16 16:29:37 Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE