Re: More issues with pg_verify_checksums and checksum verification in base backups
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Stephen Frost <sfrost@snowman.net>
Cc: Andres Freund <andres@anarazel.de>, David Steele <david@pgmasters.net>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, pgsql-hackers@postgresql.org, andrew@dunslane.net, daniel@yesql.se, magnus@hagander.net, tgl@sss.pgh.pa.us
Date: 2018-11-28T00:57:27Z
Lists: pgsql-hackers
Attachments
- 0001-Switch-pg_verify_checksums-back-to-a-blacklist.patch (text/x-diff) patch 0001
- 0002-Fix-set-of-issues-with-pg_verify_checksums-and-base-.patch (text/x-diff) patch 0002
On Tue, Nov 27, 2018 at 06:27:57PM -0500, Stephen Frost wrote: > * Michael Paquier (michael@paquier.xyz) wrote: >> Believe me or not, but we have spent so much energy on this stuff that I >> am ready to give up on the whitelist patch and focus on other business. > > I would have hoped that you'd see why I was concerned from the start > about this now that we have a released version of pg_verify_checksums > in 11.1 that is paper-bag-worthy. That's unrelated. Let's be clear, I still don't like the fact that we don't use a whitelist approach, per the arguments raised upthread. The tool also clearly lacked testing and coverage from the beginning and has never been able to work on Windows, which was a very bad decision. Still we need to live with that and I take my share of the blame, willing to make this stuff work better for all. >> - skipfile should be called after making sure that we work on a file. > > It's not clear to me what this is referring to, exactly...? Can you > clarify? Please see 0002 attached, which moves the call to skipfile() where I think it should go. >> - it is necessary to exclude EXEC_BACKEND files. > > Agreed, they should be added to the exclude list. Base backups are impacted as well as this causes spurious warnings. Attached are two patches to fix all the mess: - 0001 is a revert of the whitelist, minus the set of regression tests checking after corrupted files and empty files. - 0002 is a fix for all the issues reported on this thread, with tests added (including the tablespace test from Michael Banck): -- Base backups gain EXEC_BACKEND files in their warning filters. -- pg_verify_checksums gains the same files. -- temporary files are filtered out. -- pg_verify_checksums performs filtering checks only on regular files, not on paths. 0001 and 0002 need to be merged as 0001 would cause the buildfarm to turn red on Windows if applied alone. Can you know see my point? -- Michael
Commits
-
Fix various checksum check problems for pg_verify_checksums and base backups
- 19516afdf167 11.2 landed
- 5c995139752f 12.0 landed
-
Switch pg_verify_checksums back to a blacklist
- 85036308dc58 11.2 landed
- a1c91dd1108c 12.0 landed