Re: More issues with pg_verify_checksums and checksum verification in base backups

Greetings,

* David Steele (david(at)pgmasters(dot)net) wrote:
> On 10/30/18 11:59 AM, Stephen Frost wrote:
> > * Kyotaro HORIGUCHI (horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp) wrote:
> >> So I'm +1 for the Michael's current patch as (I think) we can't
> >> make visible or large changes.
> >>
> >> That said, I agree with Stephen's concern on the point we could
> >> omit requried files in future, but on the other hand I don't want
> >> random files are simply rejected.
> >
> > They aren't rejected- there's a warning thrown about them.
>
> pgBackRest has been using a whitelist/blacklist method for identifying
> checksummable files for almost 2 years we haven't seen any issues. The
> few times a "random" file appeared in the logs with checksum warnings it
> was later identified as having been mistakenly copied into $PGDATA. The
> backup still completed successfully in these cases.
>
> So to be clear, we whitelist the global, base, and pg_tblspc dirs and
> blacklist PG_VERSION, pg_filenode.map, pg_internal.init, and pg_control
> (just for global) when deciding which files to checksum. Recently we
> added logic to exclude unlogged and temporary relations as well, though
> that's not required.
>
> For PG11 I would recommend just adding the param file generated by exec
> backend to the black list for both pg_basebackup and pg_verifychecksums,
> then create a common facility for blacklisting for PG12.

Michael, this obviously didn't happen and instead we ended up releasing
11.1 with your changes, but I don't feel like this issue is closed and
I'm a bit disappointed that there hasn't been any further responses or
discussions on this.

I discussed this at length with David and Amit, both of whom have now
also commented on this issue, at PGConf.Eu, but still there hasn't been
a response from you. Is your thought here that your lack of response
should be taken as meaning I should simply revert your commit and then
commit your earlier patch to just add the param file? While we
typically take silence as acceptance, it's a bit different when it comes
to reverting someone else's change, at least to my mind.

I'm happy to go ahead and make those changes if there's no disagreement
regarding it.

Also, just to be clear, I don't intend this with any animosity and I
certainly understand if it just has fallen through the cracks or been
lost in the shuffle but I really don't like the implication put forward
that we're happy to not throw any kind of warning or notice about random
files showing up in the PG data directories.

Thanks!

Stephen