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.