Re: Add basic tests for the low-level backup method.

From: David Steele <david(at)pgmasters(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add basic tests for the low-level backup method.
Date: 2024-03-13 00:12:28
Message-ID: 4980aba2-9cd4-4bd6-b2e1-e873639ab7ee@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2/29/24 16:55, Michael Paquier wrote:
> On Thu, Feb 29, 2024 at 10:30:52AM +1300, David Steele wrote:
>> On 11/12/23 08:21, David Steele wrote:
>>> As promised in [1], attached are some basic tests for the low-level
>>> backup method.
>>
>> Added to the 2024-03 CF.
>
> There is already 040_standby_failover_slots_sync.pl in recovery/ that
> uses the number of your test script. You may want to bump it, that's
> a nit.

Renamed to 042_low_level_backup.pl.

> +unlink("$backup_dir/postmaster.pid")
> + or BAIL_OUT("unable to unlink $backup_dir/postmaster.pid");
> +unlink("$backup_dir/postmaster.opts")
> + or BAIL_OUT("unable to unlink $backup_dir/postmaster.opts");
> +unlink("$backup_dir/global/pg_control")
> + or BAIL_OUT("unable to unlink $backup_dir/global/pg_control");
>
> RELCACHE_INIT_FILENAME as well?

I'm not trying to implement the full exclusion list here, just enough to
get the test working. Since exclusions are optional according to the
docs I don't think we need them for a valid tests.

> +# Rather than writing out backup_label, try to recover the backup without
> +# backup_label to demonstrate that recovery will not work correctly without it,
> +# i.e. the canary table will be missing and the cluster will be corrupt. Provide
> +# only the WAL segment that recovery will think it needs.
>
> Okay, why not. No objections to this addition. I am a bit surprised
> that this is not scanning some of the logs produced by the startup
> process for particular patterns.

Not sure what to look for here. There are no distinct messages for crash
recovery. Perhaps there should be?

> +# Save backup_label into the backup directory and recover using the primary's
> +# archive. This time recovery will succeed and the canary table will be
> +# present.
>
> Here are well, I think that we should add some log_contains() with
> pre-defined patterns to show that recovery has completed the way we
> want it with a backup_label up to the end-of-backup record.

Sure, I added a check for the new log message when recovering with a
backup_label.

Regards,
-David

Attachment Content-Type Size
low-level-backup-test-v2.patch text/plain 5.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2024-03-13 00:23:27 Re: Transaction timeout
Previous Message Xing Guo 2024-03-13 00:01:31 Re: Disable LLVM bitcode generation with pgxs.mk framework.