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

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.

> > 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.

> > > 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.
 
> > > 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 am not saying this change is wrong, but I think the reasons need to be
stated in this thread, rather than just moving forward.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.