Greetings,

On Sat, Oct 20, 2018 at 00:58 Michael Paquier <mich...@paquier.xyz> wrote:

> On Sat, Oct 20, 2018 at 12:41:04AM -0400, Stephen Frost wrote:
> > I’d also like to give David Steele a chance to comment on the specific
> API,
> > and any other backup tools authors, which I don’t think we should be
> > rushing into anyway and I would think we’d only put into master..
>
> By the way, we need to do something for the checksum verification code
> in base backups for v11 as well.  If you enable checksums and take a
> base backup of a build with EXEC_BACKEND, then this creates spurious
> checksums failures.  That's a bug.  So while I agree that having a
> larger robust API is fine for HEAD, I would most likely not back-patch
> it.  This is why I would suggest as a first step for HEAD and v11 to use
> a whitelist for base backups, to check for temporary tablespaces in
> pg_verify_checksums, to move isRelFileName into src/common/ and to keep
> the change minimalistic.


I’m all for keeping the changes which are backpatched minimal, which
updating the blacklist as your original patch on this thread did would
certainly be.  Even adding in the logic to skip temp files as pg_basebackup
has would be simpler and based on existing well-tested and extensively used
code, unlike this new pattern-based whitelist of files approach.

I have to say that I can’t recall hearing much in the way of complaints
about pg_basebackup copying all the random cstore files, or the new
checksum validation logic complaining about them, and such when doing
backups and I wonder if that is because people simply don’t use the two
together much, making me wonder how much of an issue this really is or
would be with the account-for-everything approach I’ve been advocating for.

Thanks!

Stephen

>

Reply via email to