Re: pg_upgrade's interaction with pg_resetwal seems confusing

On Fri, Oct 13, 2023 at 9:29 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Oct 13, 2023 at 12:00 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > On Thu, Oct 12, 2023 at 7:17 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > Now, as mentioned in the first paragraph, it seems we anyway don't
> > > need to reset the WAL at the end when setting the next OID for the new
> > > cluster with the -o option. If that is true, then I think even without
> > > slots work it will be helpful to have such an option in pg_resetwal.
> > >
> > > Thoughts?
> >
> > I wonder if we should instead provide a way to reset the OID counter
> > with a function call inside the database, gated by IsBinaryUpgrade.
> >
>
> I think the challenge in doing so would be that when the server is
> running, a concurrent checkpoint can also update the OID counter value
> in the control file. See below code:
>
> CreateCheckPoint()
> {
> ...
> LWLockAcquire(OidGenLock, LW_SHARED);
> checkPoint.nextOid = ShmemVariableCache->nextOid;
> if (!shutdown)
> checkPoint.nextOid += ShmemVariableCache->oidCount;
> LWLockRelease(OidGenLock);
> ...
> UpdateControlFile()
> ...
> }
>

But is this a problem? basically, we will set the
ShmemVariableCache->nextOid counter to the value that we want in the
IsBinaryUpgrade-specific function. And then the shutdown checkpoint
will flush that value to the control file and that is what we want no?
I mean instead of resetwal directly modifying the control file we
will modify that value in the server using the binary_upgrade function
and then have that value flush to the disk by shutdown checkpoint.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com