Date: Thu, 7 Jul 2022 13:44:10 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Bruce Momjian <bruce@momjian.us>, pgsql-hackers@postgresql.org,
	Shruthi Gowda <gowdashru@gmail.com>
Subject: Re: pg15b2: large objects lost on upgrade
Message-ID: <20220707184410.GB13040@telsasoft.com>
References: <20220702154917.GM13040@telsasoft.com>
 <CA+Tgmobg=5hAVCcTD2277AiuqCL0Vp38ETepuR2hAxeqSyR9kg@mail.gmail.com>
 <20220705165605.GR13040@telsasoft.com>
 <CA+TgmoY0bREBNKWFkOzpg7is3DQK7gsnMoA+bBzE9+y9kdjhng@mail.gmail.com>
 <20220706115600.GU13040@telsasoft.com>
 <CA+Tgmoavavffv2GTtZ-1=QUxZh020xfUUAOS6mXd9u_zFQ=9Qw@mail.gmail.com>
 <20220707171019.GA13040@telsasoft.com>
 <CA+TgmoZDg-osUEV5FkXiL7zuVT2=m_ABVP5W9uhaAdBRnhgJPQ@mail.gmail.com>
 <YsckxPjKlGcTB50j@momjian.us>
 <CA+TgmoaT_OMUjehQP8BvChV+5XJe3KUjiCLcad5NnvnQA5OkCA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoaT_OMUjehQP8BvChV+5XJe3KUjiCLcad5NnvnQA5OkCA@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220707184410.GB13040%40telsasoft.com>
Precedence: bulk

On Thu, Jul 07, 2022 at 02:38:44PM -0400, Robert Haas wrote:
> On Thu, Jul 7, 2022 at 2:24 PM Bruce Momjian <bruce@momjian.us> wrote:
> > On Thu, Jul  7, 2022 at 01:38:44PM -0400, Robert Haas wrote:
> > > On Thu, Jul 7, 2022 at 1:10 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > Maybe it's a good idea to check that the file is empty before unlinking...
> > >
> > > If we want to verify that there are no large objects in the cluster,
> > > we could do that in check_new_cluster_is_empty(). However, even if
> > > there aren't, the length of the file could still be more than 0, if
> > > there were some large objects previously and then they were removed.
> > > So it's not entirely obvious to me that we should refuse to remove a
> > > non-empty file.
> >
> > Uh, that initdb-created pg_largeobject file should not have any data in
> > it ever, as far as I know at that point in pg_upgrade.  How would values
> > have gotten in there?  Via pg_dump?
> 
> I was thinking if the user had done it manually before running pg_upgrade.

We're referring to the new cluster which should have been initdb'd more or less
immediately before running pg_upgrade [0].

It'd be weird to me if someone were to initdb a new cluster, then create some
large objects, and then maybe delete them, and then run pg_upgrade.  Why
wouldn't they do their work on the old cluster before upgrading, or on the new
cluster afterwards ?

Does anybody actually do anything significant between initdb and pg_upgrade ?
Is that considered to be supported ?  It seems like pg_upgrade could itself run
initdb, with the correct options for locale, checksum, block size, etc
(although it probably has to support the existing way to handle "compatible
encodings").

Actually, I think check_new_cluster_is_empty() ought to prohibit doing work
between initdb and pg_upgrade by checking that no objects have *ever* been
created in the new cluster, by checking that NextOid == 16384.  But I have a
separate thread about "pg-upgrade allows itself to be re-run", and this has
more to do with that than about whether to check that the file is empty before
removing it.

-- 
Justin