Re: [patch] Fix checksum verification in base backups for zero page headers
Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Michael Banck <michael.banck@credativ.de>
Cc: PostgreSQL Development <pgsql-hackers@postgresql.org>
Date: 2020-09-24T14:24:27Z
Lists: pgsql-hackers
On 22.09.2020 17:30, Michael Banck wrote: > Hi, > > Am Dienstag, den 22.09.2020, 16:26 +0200 schrieb Michael Banck: >> Am Mittwoch, den 02.09.2020, 16:50 +0300 schrieb Anastasia Lubennikova: >>> I've looked through the previous discussion. As far as I got it, most of >>> the controversy was about online checksums improvements. >>> >>> The warning about pd_upper inconsistency that you've added is a good >>> addition. The patch is a bit messy, though, because a huge code block >>> was shifted. >>> >>> Will it be different, if you just leave >>> "if (!PageIsNew(page) && PageGetLSN(page) < startptr)" >>> block as it was, and add >>> "else if (PageIsNew(page) && !PageIsZero(page))" ? >> Thanks, that does indeed look better as a patch and I think it's fine >> as-is for the code as well, I've attached a v2. > Sorry, forgot to add you as reviewer in the proposed commit message, > I've fixed that up now in V3. > > > Michael > Great. This version looks good to me. Thank you for answering my questions, I agree, that we can work on them in separate threads. So I mark this one as ReadyForCommitter. The only minor problem is a typo (?) in the proposed commit message. "If a page is all zero, consider that a checksum failure." It should be "If a page is NOT all zero...". -- Anastasia Lubennikova Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Commits
-
Extend PageIsVerified() to handle more custom options
- 0a593bc4674f 11.10 landed
- a8795445bc1b 12.5 landed
- 017e78a3edc2 13.1 landed
- d401c5769ef6 14.0 landed