RE: Stronger safeguard for archive recovery not to miss data

Takamichi Osumi (Fujitsu) <osumi.takamichi@fujitsu.com>

From: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>
To: 'Laurenz Albe' <laurenz.albe@cybertec.at>, 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-01-18T07:34:44Z
Lists: pgsql-hackers

Attachments

Hi, Laurenz


On Friday, January 15, 2021 12:56 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> On Tue, 2020-12-08 at 03:08 +0000, osumi.takamichi@fujitsu.com wrote:
> > On Thursday, November 26, 2020 4:29 PM Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote:
> > > At Thu, 26 Nov 2020 07:18:39 +0000, "osumi.takamichi@fujitsu.com"
> > > <osumi.takamichi@fujitsu.com> wrote in
> > > > The attached patch is intended to prevent a scenario that archive
> > > > recovery hits WALs which come from wal_level=minimal and the
> > > > server continues to work, which was discussed in the thread of [1].
> > >
> > > Perhaps we need the TAP test that conducts the above steps.
> >
> > I added the TAP tests to reproduce and share the result, using the
> > case of 6-(1) described above.
> > Here, I created a new file for it because the purposes of other files
> > in src/recovery didn't match the purpose of my TAP tests perfectly.
> > If you are dubious about this idea, please have a look at the comments
> > in each file.
> >
> > When the attached patch is applied,
> > my TAP tests are executed like other ones like below.
> >
> > t/018_wal_optimize.pl ................ ok t/019_replslot_limit.pl
> > .............. ok t/020_archive_status.pl .............. ok
> > t/021_row_visibility.pl .............. ok t/022_archive_recovery.pl
> > ............ ok All tests successful.
> >
> > Also, I confirmed that there's no regression by make check-world.
> > Any comments ?
> 
> The patch applies and passes regression tests, as well as the new TAP test.
Thank you for checking.

> I think this should be backpatched, since it fixes a bug.
Agreed.

> I am not quite happy with the message:
> 
> FATAL:  WAL was generated with wal_level=minimal, data may be missing
> HINT:  This happens if you temporarily set wal_level=minimal without taking a
> new base backup.
> 
> This sounds too harmless to me and doesn't give the user a clue what would be
> the best way to proceed.
> 
> Suggestion:
> 
> FATAL:  WAL was generated with wal_level=minimal, cannot continue
> recovering
Adopted.

> DETAIL:  This happens if you temporarily set wal_level=minimal on the primary
> server.
> HINT:  Create a new standby from a new base backup after setting
> wal_level=replica.
Thanks for your suggestion.
I noticed that this message should cover both archive recovery modes,
which means in recovery mode and standby mode. Then, I combined your
suggestion above with this point of view. Have a look at the updated patch.
I also enriched the new tap tests to show this perspective.

Best Regards,
	Takamichi Osumi

Commits

  1. Fix test added by commit 9de9294b0c.

  2. Stop archive recovery if WAL generated with wal_level=minimal is found.