public inbox for [email protected]  

help / color / mirror / Atom feed

From: Shruthi Gowda <[email protected]>
To: Robert Haas <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Tom Kincaid <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: PostgreSQL Development <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)
Date: Thu, 7 Oct 2021 12:54:34 +0530
Message-ID: <CAASxf_P+EFr++8Vmfjb88HMEcCRS6NbXXQhFath0oXLnoHwJSA@mail.gmail.com> (raw)
In-Reply-To: <CA+TgmoZhgeXfrT5zOUkVisPQ-cuYRg2M9MBG7WrE0sgcUKQVWg@mail.gmail.com>
References: <CA+TgmobMvbfx53Ruyh+zzgNOhmfhBLPVy4iqbZYT080jO888xA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAASxf_NthmRLjzwyciUekCi4SLzYf=BrwbiJtepeKJYYzbGmxQ@mail.gmail.com>
	<CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com>
	<CAASxf_N2EsZk8wU7wgWBzXpRZMC5dNxsZN21O5m3R3XTb5P9OQ@mail.gmail.com>
	<CA+TgmoYHCzQChf2K4BfGcczXA6DNy=fwih+NcaidsWdDUkvN_A@mail.gmail.com>
	<CAASxf_OjBEVrkWCvwKkoT8nyYQ6Uk1MHDhhVSyZ_EXCQG7J8Dg@mail.gmail.com>
	<CA+TgmoZtbAnybMwni-u-9LArqYhsvpeF48itsJP=otrvE_ZzSQ@mail.gmail.com>
	<CAASxf_Mj3SSXFmtd9O6sf-+y4PJz+oZ57RAi0bRKKLAOBDcorA@mail.gmail.com>
	<CA+TgmoZhgeXfrT5zOUkVisPQ-cuYRg2M9MBG7WrE0sgcUKQVWg@mail.gmail.com>

On Thu, Oct 7, 2021 at 2:05 AM Robert Haas <[email protected]> wrote:
>
> On Mon, Oct 4, 2021 at 12:44 PM Shruthi Gowda <[email protected]> wrote:
> > Thanks for the inputs, Robert. In the v4 patch, an unused OID (i.e, 4)
> > is fixed for the template0 and the same is removed from unused oid
> > list.
> >
> > In addition to the review comment fixes, I have removed some code that
> > is no longer needed/doesn't make sense since we preserve the OIDs.
>
> This is not a full review, but I'm wondering about this bit of code:
>
> -       if (!RELKIND_HAS_STORAGE(relkind) || OidIsValid(relfilenode))
> +       if (!RELKIND_HAS_STORAGE(relkind) || (OidIsValid(relfilenode)
> && !create_storage))
>                 create_storage = false;
>         else
>         {
>                 create_storage = true;
> -               relfilenode = relid;
> +
> +               /*
> +                * Create the storage with oid same as relid if relfilenode is
> +                * unspecified by the caller
> +                */
> +               if (!OidIsValid(relfilenode))
> +                       relfilenode = relid;
>         }
>
> This seems hard to understand, and I wonder if perhaps it can be
> simplified. If !RELKIND_HAS_STORAGE(relkind), then we're going to set
> create_storage to false if it was previously true, and otherwise just
> do nothing. Otherwise, if !create_storage, we'll enter the
> create_storage = false branch which effectively does nothing.
> Otherwise, if !OidIsValid(relfilenode), we'll set relfilenode = relid.
> So couldn't we express that like this?
>
> if (!RELKIND_HAS_STORAGE(relkind))
>     create_storage = false;
> else if (create_storage && !OidIsValid(relfilenode))
>     relfilenode = relid;
>
> If so, that seems more clear.

'create_storage' flag says whether or not to create the storage when a
valid relfilenode is passed. 'create_storage' flag alone cannot make
the storage creation decision in heap_create().
Only binary upgrade flow sets the 'create_storage' flag to true and
expects storage gets created with specified relfilenode. Every other
caller/flow passes false for 'create_storage' and we still need to
create storage in heap_create() if relkind has storage. That's why I
have explicitly set 'create_storage = true' in the else flow and
initialize relfilenode on need basis.

Regards,
Shruthi KC
EnterpriseDB: http://www.enterprisedb.com

view thread (69+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)
  In-Reply-To: <CAASxf_P+EFr++8Vmfjb88HMEcCRS6NbXXQhFath0oXLnoHwJSA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox