Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:
> On Thu, Aug 26, 2021 at 12:34:56PM -0400, Stephen Frost wrote:
> > * Bruce Momjian (bruce@momjian.us) wrote:
> > > On Thu, Aug 26, 2021 at 11:36:51AM -0400, Stephen Frost wrote:
> > > > * Bruce Momjian (bruce@momjian.us) wrote:
> > > > > On Thu, Aug 26, 2021 at 11:00:47AM -0400, Robert Haas wrote:
> > > > > > Anyone see a flaw in that analysis?
> > > > >
> > > > > I am still waiting to hear the purpose of this preservation.  As long as
> > > > > you don't apply the patch, I guess I will just stop asking.
> > > >
> > > > I'm a bit confused why this question keeps coming up as we've discussed
> > > > multiple reasons (incremental backups, possible use for TDE which would
> > >
> > > I have not seen much explaination on pgBackRest, except me mentioning
> > > it.  Is this something really useful?
> >
> > Being able to quickly perform a backup on a newly upgraded cluster would
> > certainly be valuable and that's definitely not possible today due to
> > all of the filenames changing.
>
> You mean incremental backup, right?  I was told this by the pgBackRest
> developers during PGCon, but I have not heard that stated publicly, so I
> hate to go just on what I heard rather than seeing that stated publicly.

Yes, we're talking about either incremental (or perhaps differential)
backup where only the files which are actually different would be backed
up.  Just like with PG, I can't provide any complete guarantees that
we'd be able to actually make this possible after a major version with
pgBackRest with this change, but it definitely isn't possible *without*
this change.  I can't see any reason why we wouldn't be able to do a
checksum-based incremental backup though (which would be *much* faster
than a regular backup) once this change is made and have that be a
reliable and trustworthy backup.  I'd want to think about it more and
discuss it with David in some detail before saying if we could maybe
perform a timestamp-based incremental backup (without checksum'ing the
files, as we do in normal situations), but that would really just be a
bonus.

> > > As far as TDE, I haven't seen any concrete plan for that, so why add
> > > this code for that reason?
> >
> > That this would help with TDE (of which there seems little doubt...) is
> > an additional benefit to this.  Specifically, taking the existing work
> > that's already been done to allow block-by-block encryption and
> > adjusting it for AES-XTS and then using the db-dir+relfileno+block
> > number as the IV, just like many disk encryption systems do, avoids the
> > concerns that were brought up about using LSN for the IV with CTR and
> > it's certainly not difficult to do, but it does depend on this change.
> > This was all discussed previously and it sure looks like a sensible
> > approach to use that mirrors what many other systems already do
> > successfully.
>
> Well, I would think we would not add this for TDE until we were sure
> someone was working on adding TDE.

That this would help with TDE is what I'd consider an added bonus.

> > > > make this required, general improved sanity when working with pg_upgrade
> > > > is frankly a benefit in its own right too...).  If the additional code
> > >
> > > How?  I am not aware of any advantage except cosmetic.
> >
> > Having to resort to matching up inode numbers between the two clusters
> > after a pg_upgrade to figure out what files are actually the same
> > underneath is a pain that goes beyond just cosmetics imv.  Removing that
> > additional level that admins, and developers for that matter, have to go
> > through would be a nice improvement on its own.
>
> OK, I was just not aware anyone did that, since I have never hard anyone
> complain about it before.

I've certainly done it and I'd be kind of surprised if others haven't,
but I've also played a lot with pg_dump in various modes, so perhaps
that's not a great representation.  I've definitely had to explain to
clients why there's a whole different set of filenames after a
pg_upgrade and why that is the case for an 'in place' upgrade before
too.

> > > > was a huge burden or even a moderate one then that might be an argument
> > > > against, but it hardly sounds like it will be given Robert's thorough
> > > > analysis so far and the (admittedly not complete, but not that far from
> > > > it based on the DB OID review) proposed patch.
> > >
> > > I am find to add it if it is minor, but I want to see the calculus of
> > > its value vs complexity, which I have not seen spelled out.
> >
> > I feel that this, along with the prior discussions, spells it out
> > sufficiently given the patch's complexity looks to be reasonably minor
> > and very similar to the existing things that pg_upgrade already does.
> > Had pg_upgrade done this in the first place, I don't think there would
> > have been nearly this amount of discussion about it.
>
> Well, there is a reason pg_upgrade didn't initially do this --- because
> it adds complexity, and potentially makes future changes to pg_upgrade
> necessary if the server behavior changes.

I have a very hard time seeing what changes might happen in the server
in this space that wouldn't have an impact on pg_upgrade, with or
without this.

> I am not saying this change is wrong, but I think the reasons need to be
> stated in this thread, rather than just moving forward.

Ok, they've been stated and it seems to at least Robert and myself that
this is worthwhile to at least continue through to a concluded patch,
after which we can contemplate that patch's complexity against these
reasons.

Thanks,

Stephen